refactor: Replace react-bootstrap tabs with Antd tabs #11118

kgabryje · 2020-09-30T14:42:41Z

SUMMARY

Replace react-bootstrap tabs with Antd tabs in Welcome, BuilderComponentPane, ControlPanelsContainer, AdhocMetricEditPopover, DatasourceEditor, AdhocFilterEditPopover, DateFilterControl. Screenshots below show a few examples of changes.

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Welcome screen before/after:

BuilderComponentPane before/after:

ControlPanelsContainer before/after:

TEST PLAN

ADDITIONAL INFORMATION

kgabryje · 2020-10-05T12:04:19Z

@rusackas Can you please take a look?

superset-frontend/spec/javascripts/datasource/DatasourceEditor_spec.jsx

kgabryje · 2020-10-15T16:14:37Z

As was discussed in PR #11160, I changed most of the Tabs styles to "line". The only component that still uses the "card" style is ControlsPanelContainer, because in the case of only 1 tab displayed it looks rather weird with line style (as on the screenshot).
@rusackas @ktmud What do you think?

codecov-io · 2020-10-15T16:24:34Z

Codecov Report

Merging #11118 into master will decrease coverage by 10.74%.
The diff coverage is 81.25%.

@@             Coverage Diff             @@
##           master   #11118       +/-   ##
===========================================
- Coverage   66.55%   55.81%   -10.75%     
===========================================
  Files         869      410      -459     
  Lines       41587    13976    -27611     
  Branches     3797     3549      -248     
===========================================
- Hits        27680     7801    -19879     
+ Misses      13805     6007     -7798     
- Partials      102      168       +66

Flag	Coverage Δ
#cypress	`55.81% <81.25%> (+0.03%)`	⬆️
#javascript	`?`
#python	`?`

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
.../src/dashboard/components/BuilderComponentPane.jsx	`100.00% <ø> (ø)`
...erset-frontend/src/datasource/DatasourceEditor.jsx	`51.69% <25.00%> (-19.94%)`	⬇️
...erset-frontend/src/common/components/Tabs/Tabs.tsx	`95.45% <100.00%> (ø)`
.../src/explore/components/AdhocFilterEditPopover.jsx	`78.72% <100.00%> (ø)`
.../src/explore/components/AdhocMetricEditPopover.jsx	`56.16% <100.00%> (-13.70%)`	⬇️
.../src/explore/components/ControlPanelsContainer.jsx	`91.42% <100.00%> (-1.00%)`	⬇️
.../explore/components/controls/DateFilterControl.jsx	`60.11% <100.00%> (-5.96%)`	⬇️
...uperset-frontend/src/dashboard/util/dnd-reorder.js	`0.00% <0.00%> (-100.00%)`	⬇️
...rset-frontend/src/dashboard/util/getEmptyLayout.js	`0.00% <0.00%> (-100.00%)`	⬇️
...et-frontend/src/components/Menu/LanguagePicker.tsx	`0.00% <0.00%> (-100.00%)`	⬇️
... and 701 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66fc267...c14ac96. Read the comment docs.

ktmud · 2020-10-16T23:23:41Z

What does it look like if we make it less wide?

kgabryje · 2020-10-19T09:34:44Z

@ktmud I tried with full width when there are 2 tabs and 30% when only Data tab is shown. What do you think?

rusackas · 2020-10-19T17:53:11Z

@ktmud I tried with full width when there are 2 tabs and 30% when only Data tab is shown. What do you think?

I think this makes enough sense for now, as I agree, the single super-wide tab didn't look good. Although I think 50% makes more sense, since that's what it the tab would be when the (only) other tab appears. This would make it more consistent from viz to viz.

Making further adjustments should be easy/fast styling PRs, so I'm not losing too much sleep over this. Also, the "customize" tab's content will be moving to a whole new area "soon" when we revamp Explore, so this is all a temporary issue.

As an alternative idea, we could show the second tab anyway, and just leave it grayed out/disabeld, but I fear that may lead to more user confusion.

ktmud · 2020-10-20T06:09:19Z

Agree 50% makes more sense. I think what would be the best of both world is to have fixed padding and dynamic width by default, then automatically expand to take full width when their total width is close to the width of the wrapping container. Might be a little tricky to implement, but it could be worth it.

kgabryje · 2020-10-20T12:47:21Z

I changed the width to 50%. @ktmud I see your point, but I think that in many cases full width is what we want to go for, even though the tabs don't need that much space, e.g. Welcome view. In my opinion it looks better that way and I'd rather handle special cases individually. However, if there are more voices for "dynamic" approach I'm happy to refactor it that way 🙂

ktmud · 2020-10-20T22:41:33Z

If you look at design mockups in SIP-34, there are multiple pages with dynamic width . Not sure what should be the default, but my guess is we will eventually need to implement a "width mode" option one way or another.

That said, if we don't need to implement those pages right now, I'm OK with current solution.

superset-frontend/cypress-base/cypress/integration/chart_list/card_view.test.ts

kgabryje · 2020-10-29T09:29:50Z

@rusackas Rebase done

rusackas · 2020-10-30T05:27:50Z

Hey @kgabryje - sorry this PR took another hit. I forgot this had changes for the Welcome page, which just had a big revamp merged in. You can probably just remove changes to the Welcome page from this PR, and I promise I'll get it merged in. Apologies for the extra effort, and thanks for your patience.

kgabryje · 2020-10-30T11:17:59Z

@rusackas Rebase done. I like the new look of Welcome page by the way!

* Replace tabs in BuilderComponentPane * Replace tabs in ControlPanelsContainer * Replace tabs in AdhocMetricEditPopover * Replace Tabs in DatasourceEditor * Replace tabs in AdhocFilterEditPopover * Replace tabs in DateFilterControl * Bug fix * Change Tab styles * Fix tests * Fix cypress tests * Lint fix * Fix tests * Change Tabs style in ControlPanelsContainer * Change tabs content height * Lint fix * Add data test * Fix e2e test * Move Tabs file to separate dir * Fix after rebase * Fix e2e tests * Fix after rebase

superset-github-bot bot added the polidea label Sep 30, 2020

pull-request-size bot added the size/XXL label Sep 30, 2020

kgabryje force-pushed the antd/tabs2 branch 5 times, most recently from ed9e756 to 6f03492 Compare October 5, 2020 09:22

pull-request-size bot added size/XL and removed size/XXL labels Oct 5, 2020

kgabryje changed the title ~~refactor: Replace react-bootstrap tabs with Antd tabs [WIP] [depends on #11090]~~ refactor: Replace react-bootstrap tabs with Antd tabs Oct 5, 2020

willbarrett requested a review from nytai October 5, 2020 22:30

mistercrunch reviewed Oct 7, 2020

View reviewed changes

superset-frontend/spec/javascripts/datasource/DatasourceEditor_spec.jsx Show resolved Hide resolved

kgabryje force-pushed the antd/tabs2 branch from e199942 to 48cfb90 Compare October 15, 2020 16:10

kgabryje force-pushed the antd/tabs2 branch from 726b153 to 85991d7 Compare October 15, 2020 20:55

kgabryje force-pushed the antd/tabs2 branch from 85991d7 to 816e6d3 Compare October 20, 2020 12:39

pull-request-size bot added size/L and removed size/XL labels Oct 20, 2020

mistercrunch added the risk:many-files Change has too many files label Oct 21, 2020

rusackas reviewed Oct 26, 2020

View reviewed changes

superset-frontend/cypress-base/cypress/integration/chart_list/card_view.test.ts Outdated Show resolved Hide resolved

kgabryje force-pushed the antd/tabs2 branch 2 times, most recently from cb5a555 to bb6ec67 Compare October 27, 2020 14:18

kgabryje force-pushed the antd/tabs2 branch from 737b1ba to 63884d1 Compare October 29, 2020 08:34

kgabryje added 21 commits October 30, 2020 10:20

Replace tabs in BuilderComponentPane

f058e75

Replace tabs in ControlPanelsContainer

7d4d5f9

Replace tabs in AdhocMetricEditPopover

b3cf06a

Replace Tabs in DatasourceEditor

c7f55bd

Replace tabs in AdhocFilterEditPopover

93af468

Replace tabs in DateFilterControl

7dd4f86

Bug fix

414fe10

Change Tab styles

a9284da

Fix tests

0829097

Fix cypress tests

a27bd1a

Lint fix

ee10062

Fix tests

da46a92

Change Tabs style in ControlPanelsContainer

e99ba9d

Change tabs content height

ae27b95

Lint fix

7f416bb

Add data test

87d7313

Fix e2e test

99295f7

Move Tabs file to separate dir

3300668

Fix after rebase

72187a3

Fix e2e tests

c7ef0e7

Fix after rebase

c14ac96

kgabryje force-pushed the antd/tabs2 branch from 63884d1 to c14ac96 Compare October 30, 2020 09:41

rusackas merged commit 55a3404 into apache:master Oct 31, 2020

mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.0.0 labels Mar 12, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: Replace react-bootstrap tabs with Antd tabs #11118

refactor: Replace react-bootstrap tabs with Antd tabs #11118

kgabryje commented Sep 30, 2020 •

edited

Loading

kgabryje commented Oct 5, 2020

kgabryje commented Oct 15, 2020 •

edited

Loading

codecov-io commented Oct 15, 2020 •

edited

Loading

ktmud commented Oct 16, 2020

kgabryje commented Oct 19, 2020

rusackas commented Oct 19, 2020

ktmud commented Oct 20, 2020

kgabryje commented Oct 20, 2020

ktmud commented Oct 20, 2020 •

edited

Loading

kgabryje commented Oct 29, 2020

rusackas commented Oct 30, 2020

kgabryje commented Oct 30, 2020

refactor: Replace react-bootstrap tabs with Antd tabs #11118

refactor: Replace react-bootstrap tabs with Antd tabs #11118

Conversation

kgabryje commented Sep 30, 2020 • edited Loading

SUMMARY

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

TEST PLAN

ADDITIONAL INFORMATION

kgabryje commented Oct 5, 2020

kgabryje commented Oct 15, 2020 • edited Loading

codecov-io commented Oct 15, 2020 • edited Loading

Codecov Report

ktmud commented Oct 16, 2020

kgabryje commented Oct 19, 2020

rusackas commented Oct 19, 2020

ktmud commented Oct 20, 2020

kgabryje commented Oct 20, 2020

ktmud commented Oct 20, 2020 • edited Loading

kgabryje commented Oct 29, 2020

rusackas commented Oct 30, 2020

kgabryje commented Oct 30, 2020

kgabryje commented Sep 30, 2020 •

edited

Loading

kgabryje commented Oct 15, 2020 •

edited

Loading

codecov-io commented Oct 15, 2020 •

edited

Loading

ktmud commented Oct 20, 2020 •

edited

Loading