-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Button Elements: Allow element styles in classic themes #42012
Conversation
@@ -246,6 +246,9 @@ | |||
"text": "#fff", | |||
"background": "#32373c" | |||
}, | |||
"spacing": { | |||
"padding": "calc(0.667em + 2px) calc(1.333em + 2px)" |
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.
I'm not sure what to do about this. In the theme I tested, the theme reset the padding for buttons. This padding is the one that used to be provided by the button block, but the search block uses a different padding. If we don't provide this then themes with heavy handed resets will lose button padding, but if we do provide this then the search block buttons will end up using this padding instead of the default specified in the block CSS. IMO this is a better compromise, but it will impact on the search button padding slightly.
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.
It is likely one of the smaller problems and I too agree that the second evil is lesser.
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.
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.
This looks like a fix for an oversight: we do want the css of blocks be it in JSON or in CSS to work for all themes.
I added a commit which moves the padding definition for the Search block button to the opinionated styles. This means that themes using this option won't be impacted. This follows on from the ideas discussed in #42054 |
Size Change: +566 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
This is actually an issue for classic themes that we're rediscussing at #43981 It seems that we haven't ported these changes to core as per https://github.com/WordPress/wordpress-develop/blob/trunk/src/wp-includes/global-styles-and-settings.php#L108 It's worth reevaluating this PR in light of the conversation at #43981 and revert if necessary to make sure classic themes do not get styles they don't want. |
What?
In #34180, we moved block and element styles into JSON as well as migrating the styles of the
core/buttons
block away from CSS and into these new formats.Classic themes don't load styles provided in JSON. This changes that, so that we now also include core only block and element styles from theme.json.
Why?
We need to ensure that the correct element styles are provided to all themes, not just block themes.
How?
Previously we only output variables and presets from theme.json, not the styles section. IMO it's safe to use this section as we restrict it to only the rules coming from the core styles, not from other origins:
Testing Instructions
Screenshots or screencast
The search block looks like this:
@WordPress/block-themers