-
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
fix: fix submenu header double line #14631
Conversation
Codecov Report
@@ Coverage Diff @@
## master #14631 +/- ##
==========================================
+ Coverage 77.20% 77.47% +0.27%
==========================================
Files 958 958
Lines 48492 48489 -3
Branches 5691 5690 -1
==========================================
+ Hits 37437 37567 +130
+ Misses 10855 10722 -133
Partials 200 200
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
/testenv up |
@rusackas Ephemeral environment spinning up at http://34.209.38.58:8080. Credentials are |
/testenv up |
@@ -42,6 +42,8 @@ const StyledHeader = styled.div` | |||
padding: 14px 0; | |||
margin-right: ${({ theme }) => theme.gridUnit * 3}px; |
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.
You can replace all instances of ${({ theme }) => by moving this piece of code to the top like this https://github.com/apache/superset/pull/14448/files#diff-0856ab0ce511fac9256389d5ed3635f15d77ec907de602ce32d28f072de76753
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.
Nice tip!! We should sweep through and do this all over the place, on some rainy day (probably not now).
…ix-submenu-views
@junlincc Ephemeral environment spinning up at http://34.217.89.200:8080. Credentials are |
@pkdotson Ephemeral environment spinning up at http://18.236.78.240: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!
LGTM!! Thanks for the fix 😊 |
Ephemeral environment shutdown and build artifacts deleted. |
* fix submenu header * remove unused css * lint * address comment * address more comments (cherry picked from commit 60f903f)
* fix submenu header * remove unused css * lint * address comment * address more comments
* fix submenu header * remove unused css * lint * address comment * address more comments
* fix submenu header * remove unused css * lint * address comment * address more comments
SUMMARY
This fixes issue where title of submenu is too long for column and would make text break to next line.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
before
after
Screen.Recording.2021-05-13.at.6.27.53.PM.mov
TEST PLAN
Go to alert & reports and annotationslist to view the fixed menus.
ADDITIONAL INFORMATION