-
Notifications
You must be signed in to change notification settings - Fork 14.4k
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: Make spacings in Settings menu more consistent #11709
Conversation
Codecov Report
@@ Coverage Diff @@
## master #11709 +/- ##
==========================================
- Coverage 62.85% 60.47% -2.39%
==========================================
Files 889 843 -46
Lines 43053 41310 -1743
Branches 4015 3716 -299
==========================================
- Hits 27063 24981 -2082
- Misses 15812 16329 +517
+ Partials 178 0 -178
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
.about-section { | ||
margin: ${({ theme }) => theme.gridUnit}px 0 | ||
${({ theme }) => theme.gridUnit * 2}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.
Can we use :last-child
so this can be more generic?
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.
Those styles are needed not for any last child but specifically for "About us" section. That section is a bit "hacked" as "Version" and "SHA" are not defined as menu items - we don't want them to be clickable or highlighted on hover. Because of that, default Antd styles from Menu
were not applied there and I added those styles to make it look nicer.
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.
In that case, we may still generalize it something like MetaMenuItem
in case this pattern is used elsewhere. But let's not over-optimize for now.
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 fix!
SUMMARY
Decreases margin for item group title, increases margin for the last element of item group, increases margin for "About" section.
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TEST PLAN
ADDITIONAL INFORMATION
CC: @ktmud