-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Try: Simplify & polish heading levels. #34378
Conversation
Size Change: -293 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
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.
Tests well for me! ✨ for code removal as well
Just carrying over the conversation from #34406, it looks like this PR introduced a regression for a number of components that add a negative margin to offset the Scrollbars introduced:
@jasmussen do we want to work through those instances that need their negative margins updated (I'm happy to open a couple of PRs, I already updated the drop down menu component in #34406). Alternately, did we want to update the Heading dropdown to use its own negative margins instead of globally updating padding for the children of the popover content? Happy to help out! |
Thanks for the help tracking that down. I've created #34435 to fix it. |
Description
Followup to #32926, this one tries to polish the heading level dropdown a little bit and simplify some code. The min width is no longer necessary, and the padding was too tight. Before:
This PR removes, and therefore unifies, some of the rules, and reduces the padding a bit to compensate:
At first glance the vertical toolbar looks like it has too much padding at this point. But give it a moment, I think it works alright.
Checklist:
*.native.js
files for terms that need renaming or removal).