-
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
Tidy up button vertical align styles #17601
Conversation
Do you know why primary buttons had that vertical-align style? |
@youknowriad It seems to have been there from #6562, so from pretty early on. It's not clear why it was added, but I imagine it was lifted from the styles in core ... it does look like core buttons have It seems unnecessary with modern css and a little over-opinionated. The fewer styles in these base components the better.
Just had a look. That only seems to be only happening on the Button block. For some reason the button block has I wonder if a lint rule to stop |
|
Maybe it's just me, but I feel like If theres a way to apply that rule just for previews, that might be a solution. |
I've used Can't we just absolute align the warning Also, I feel this is something that can be explored separately from this PR. |
@youknowriad I haven't been able to find a way to fix it yet. I think it also speaks to a different problem—that block styles shouldn't have such an effect on general editor styles. Will merge this PR as I'm working on updating the button styles now. |
* Remove vertical-align:top from buttons and tweak block-editor-warning message * Styling tweaks for block warnings to account for removed `vertical-align: top`
* Remove vertical-align:top from buttons and tweak block-editor-warning message * Styling tweaks for block warnings to account for removed `vertical-align: top`
NOTE - I don't think should be merged for WP5.3 as it affects every
Button
. Having said that, it looks like buttons maybe updated anyway - #17585.Description
This is a follow-up to #17572 (comment)
Primary buttons had
vertical-align: top
, but other types of buttons didn't, which made laying them out alongside one another difficult. This PR removes thatvertical-align: top
from all buttons to make them consistent. It affects all buttons so it might have some far-reaching consequences (but I couldn't see any particular issues).I've removed the slightly hacky fixes from #17572 and switched to flexbox's
align-items: baseline
to properly align the buttons in the block warnings.There were also some other minor tweaks required
20px
definition was only used for the editor warning message, and its defined in the error boundary anyway: https://github.com/WordPress/gutenberg/blob/master/packages/editor/src/components/error-boundary/style.scss#L4How has this been tested?
Manual testing
div
in a block)edit
) (oh, turns out block error boundaries are not working!).Screenshots
Types of changes
Task
Checklist: