fix: Empty tab component in Dashboard cannot be deleted #14878
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14878 +/- ##
=======================================
Coverage 77.57% 77.57%
=======================================
Files 963 963
Lines 49290 49291 +1
Branches 6205 6205
=======================================
+ Hits 38235 38236 +1
Misses 10854 10854
Partials 201 201
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
/testenv up |
|
@pkdotson Ephemeral environment spinning up at http://34.219.157.222:8080. Credentials are |
| @@ -89,6 +89,10 @@ const StyledTabsContainer = styled.div` | |||
| .ant-tabs { | |||
| overflow: visible; | |||
|
|
|||
| .ant-tabs-nav-wrap { | |||
| min-height: ${({ theme }) => theme.gridUnit * 12.5}px; | |||
There was a problem hiding this comment.
Nit, non-blocking: Why this 12.5 value here? Could we round it to a whole value or else add a comment noting the reason?
There was a problem hiding this comment.
Hello @suddjian. Thanks for your feedback. This is to make it exactly 50px which should be the default height of the tabs component.
There was a problem hiding this comment.
If this is meant to match the tabs component, it would be nice to have them both reference the same constant value instead of magic numbers that happen to match up.
|
/testenv up |
|
@junlincc Ephemeral environment spinning up at http://54.70.95.188:8080. Credentials are |
There was a problem hiding this comment.
Agree with @suddjian's comments... I'm not a fan of fractional gridUnits or "magic numbers" so I'm supportive of a follow-up PR t either add a constant that both refer to. It could potentially be exported by the tab component (open to suggestions there). It could export the pixel height, but it would be better, I think, to export a number of gridUnits, which could then be used here, and by the Tab component itself. 12 or 13 gridUnits for both should do nicely, whichever looks better. Tagging this PR for a revisit.
|
Ephemeral environment shutdown and build artifacts deleted. |
* upstream/master: fix(explore): Datepicker glitch on hover outside the modal (apache#15033) Add ming-height to empty tab (apache#14878) Remove nowrap (apache#14954) display all metric results in editor (apache#15031) feat: Add "is_select_query" method to base engine spec to make it possible to override it (apache#15013) fix(dashboard): custom css should be removed on unmount (apache#15025) feat(filter-box): hide druid options if druid not enabled (apache#14921) fix: adding additional configs and colors for queryHistory (apache#14995) chore: rename 'Source' to 'Database' for consistency (apache#15021) chore(ci): fix ci conflict (apache#15016) fix(native-filters): avoid double load on initialization (apache#15012) feat(native-filters): Support default to first value in select filter (apache#14869) docs: required information for OAuth2 configuration (apache#15010) Update index.mdx (apache#14990)
mistercrunch
added
🏷️ bot
SUMMARY
Adds a min-height to the tabs component so that when empty the user can still reach the trash icon to delete the component from the dashboard
BEFORE
Video.Game.Sales.mp4
AFTER
Video.Game.Sales.1.mp4
TESTING INSTRUCTIONS
ADDITIONAL INFORMATION