Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: Bootstrap to AntD - DropdownButton #13002

Merged

Conversation

michael-s-molina
Copy link
Member

SUMMARY

  • Migrates Collapse component from Bootstrap to AntD.
  • Aligns dropdown caret with text

See: #10254

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Screen Shot 2021-02-05 at 2 50 34 PM

Screen Shot 2021-02-05 at 2 50 34 PM

Screen Shot 2021-02-05 at 2 51 08 PM

Screen Shot 2021-02-08 at 10 22 04 AM

Screen Shot 2021-02-08 at 10 20 29 AM

Screen Shot 2021-02-08 at 10 21 16 AM

Screen Shot 2021-02-08 at 10 52 39 AM

Screen Shot 2021-02-08 at 10 53 11 AM

@rusackas @junlincc

TEST PLAN

1 - Open any screen that contains a DropdownButton (explore, dashboard -> edit, etc.)
2 - All DropdownButton components should have the same theme and behavior

ADDITIONAL INFORMATION

  • Has associated issue:
  • Changes UI
  • Requires DB Migration.
  • Confirm DB Migration upgrade and downgrade tested.
  • Introduces new feature or API
  • Removes existing feature or API

@junlincc junlincc added the explore:refactor Related to refactoring Explore label Feb 8, 2021
@michael-s-molina michael-s-molina force-pushed the bootstrap-to-antd-dropdown-button branch from 131003b to b6ee14a Compare February 8, 2021 16:43
@codecov-io
Copy link

codecov-io commented Feb 8, 2021

Codecov Report

Merging #13002 (5034253) into master (2ce7982) will increase coverage by 5.36%.
The diff coverage is 70.58%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #13002      +/-   ##
==========================================
+ Coverage   53.06%   58.42%   +5.36%     
==========================================
  Files         489      468      -21     
  Lines       17314    15873    -1441     
  Branches     4482     4080     -402     
==========================================
+ Hits         9187     9274      +87     
+ Misses       8127     6599    -1528     
Flag Coverage Δ
cypress 58.42% <70.58%> (+5.36%) ⬆️

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

Impacted Files Coverage Δ
...erset-frontend/src/SqlLab/components/ResultSet.tsx 4.16% <0.00%> (-0.09%) ⬇️
...end/src/SqlLab/components/RunQueryActionButton.tsx 52.77% <ø> (ø)
superset-frontend/src/chart/ChartContainer.jsx 100.00% <ø> (ø)
superset-frontend/src/chart/ChartRenderer.jsx 75.67% <0.00%> (-1.04%) ⬇️
...perset-frontend/src/common/components/Dropdown.tsx 50.00% <ø> (ø)
...t-frontend/src/common/components/Tooltip/index.tsx 100.00% <ø> (ø)
...perset-frontend/src/components/AlteredSliceTag.jsx 92.00% <ø> (ø)
superset-frontend/src/components/CachedLabel.jsx 42.10% <ø> (ø)
...ontend/src/components/CertifiedIconWithTooltip.tsx 0.00% <ø> (ø)
...perset-frontend/src/components/CopyToClipboard.jsx 57.14% <0.00%> (ø)
... and 147 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 f85497e...5034253. Read the comment docs.

@junlincc junlincc requested review from rusackas and junlincc February 9, 2021 02:54
Copy link
Member

@junlincc junlincc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approving as product sign-off. ✅ thanks for the PR!
now i know where the previous mysterious drop-shadow in Explore came from lol
Screen Shot 2021-02-08 at 10 24 08 PM
Screen Shot 2021-02-08 at 10 24 53 PM

@junlincc
Copy link
Member

junlincc commented Feb 9, 2021

not related, @michael-s-molina Michael, could you double check if we covered the casing change in the main menu dropdown? thanks!
Screen Shot 2021-02-08 at 10 29 28 PM

@michael-s-molina
Copy link
Member Author

not related, @michael-s-molina Michael, could you double check if we covered the casing change in the main menu dropdown? thanks!
Screen Shot 2021-02-08 at 10 29 28 PM

@junlincc This text is translated on the server side (Python). This is the part that Kasia was working on. We should discuss how to continue this today in our meeting.

@rusackas rusackas added hold! On hold hold:testing! On hold for testing and removed hold! On hold labels Feb 16, 2021
Copy link
Member

@rusackas rusackas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few nits/comments, and places we can expand, but in general this approach looks great to me! Thanks for your patience with this. @junlincc (and @eschutho?) let's find some time to QA-bash this and get it in!

@rusackas rusackas removed the hold:testing! On hold for testing label Feb 18, 2021
@michael-s-molina michael-s-molina force-pushed the bootstrap-to-antd-dropdown-button branch from b6ee14a to 5034253 Compare February 18, 2021 13:35
@rusackas rusackas merged commit 9335b9c into apache:master Feb 18, 2021
@mistercrunch mistercrunch added 🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels 🚢 1.2.0 labels Mar 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏷️ bot A label used by `supersetbot` to keep track of which PR where auto-tagged with release labels explore:refactor Related to refactoring Explore size/L 🚢 1.2.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants