-
Notifications
You must be signed in to change notification settings - Fork 13.9k
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
feat(dashboard): Rearrange items in chart header controls dropdown #20049
feat(dashboard): Rearrange items in chart header controls dropdown #20049
Conversation
Codecov Report
@@ Coverage Diff @@
## master #20049 +/- ##
==========================================
- Coverage 66.35% 66.35% -0.01%
==========================================
Files 1713 1713
Lines 64102 64105 +3
Branches 6746 6748 +2
==========================================
Hits 42538 42538
- Misses 19849 19851 +2
- Partials 1715 1716 +1
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up |
@kgabryje Ephemeral environment spinning up at http://34.220.138.72:8080. Credentials are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR @kgabryje. I'm assuming that we will have another one coming for the dashboard options as well (to make them consistent).
I'm not sure if it's related but clicking on Download as image is raising the following exception:
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/SliceHeaderControls/index.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
Outdated
Show resolved
Hide resolved
superset-frontend/src/dashboard/components/SliceHeaderControls/SliceHeaderControls.test.tsx
Outdated
Show resolved
Hide resolved
LGTM thank you 🙏 |
2d688c8
to
2e80a3f
Compare
Well spotted. The problem was that the menu renders in the chart holder in DOM tree, but submenu is attached to the body and there's no way to specify popup container. I changed the screenshot selector from relative to absolute (each chart holder has a class specific to chart id, so it's easy to use exact selector to grab correct chart) |
Yep! That will happen when new Dashboard header (consistent with header in Explore) gets implemented |
/testenv up |
@michael-s-molina Ephemeral environment spinning up at http://34.222.140.234:8080. Credentials are |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Ephemeral environment shutdown and build artifacts deleted. |
…pache#20049) * feat(dashboard): Rearrange items in chart header controls dropdown * Fix download as image and tests
SUMMARY
This PR rearranges and renames menu items in chart's controls dropdown on Dashboard.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
Before:
After:
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION
CC @kasiazjc