-
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
Styles: try wrapping with :root to fix reset styles #61638
Conversation
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ lib/class-wp-theme-json-gutenberg.php ❔ lib/theme.json ❔ phpunit/class-wp-theme-json-test.php |
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 PR!
In terms of specificity, :root
is exactly the same as a classname, so this change effectively undoes the reduction in #60106. I think that if that's what we want to do, reverting #60106 is a better solution as it will keep the simpler, more semantic classname selector.
The other thing to bear in mind is that, if we revert the global styles specificity, we'll need to revert #60228 too, because the reduction in the layout selectors was dependent on the global styles one.
I don't have enough context on why this change is needed, so a fuller description on this PR, or link to an issue, would be much appreciated!
Size Change: -1.22 kB (-0.07%) Total Size: 1.74 MB
ℹ️ View Unchanged
|
We don't want to undo any reduction in specificity, we just want to put it at a level that is the same or higher as the reset styles. We don't want to revert global styles specificity reduction.
I'm not sure how much fuller I can make it, but I'll try. When there are reset styles at the element specify level, they will overrule any zero specificity global styles? |
My understanding of the proposal is that:
So if that's correct, I don't think a revert is required but there might be some additional tweaks required for the block library styles for core blocks. This does still provide a general reduction in specificity, unfortunately, not to zero as we would in an ideal world starting from scratch. |
Flaky tests detected in f1b413f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9221646544
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
@@ -1,9 +1,7 @@ | |||
// Same as the group block styles. | |||
:where(.wp-block-template-part) { | |||
&.has-background { |
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 seems we previously did have some specificity left here. We should make sure it tests well now.
// Adding `figure` to the selector increases the specificity above the global | ||
// styles specificity, which is levelled at 0-1-0. We should figure out why | ||
// `figure` is needed. | ||
:root :where(figure.wp-block-gallery) { |
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 just added :root
as a precaution here to make sure this overrides any reset styles.
0ba0494
to
c5b2fa5
Compare
I've rebased this one to address the CSS linting error regarding On the choice to use |
The restoration of cc/ @tellthemachines and @andrewserong for your expertise on what other layout styles may also need updating Also, I have the following list of recent PRs around reducing specificity. Are there others I've missed?
|
c5b2fa5
to
885a70b
Compare
I've pushed a commit that I think might be the minimum set of changes required to make layout styles work and maintain some more consistent specificity. To be honest, I had a really hard time working out what should and should not be updated for the layout style selectors. Especially, when running into bugs that turned out to also be on trunk (#61849, #61846 etc) Moving forward I think we definitely need some test coverage to ensure the correct layout styles are being applied when and where they should. These would also serve as internal documentation for anyone needing to change some aspect of our layout supports. It's also worth noting here that this PR is being discussed in regards to what CSS specificity we should settle on for WP 6.6. |
After some discussion, and taking into account the limited time before the 6.6 beta, the option to use a This PR should be ready to be refined, reviewed, etc with a view to landing as soon as possible. |
@@ -2,6 +2,6 @@ | |||
@include caption-style-theme(); | |||
} | |||
|
|||
:where(.wp-block-embed) { | |||
.wp-block-embed { |
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.
Should the .wp-block-embed figcaption
rule at the top of this file have lowered specificity?
Rather than adding :root
to each individual rule or removing :where
, I thought it might be better to do something like this:
:root {
:where(.wp-block-embed) {
// ...
}
}
This way everything is consistent.
I realize it's the same specificity as just a classname, but it makes it easier for anyone modifying the css to maintain the same specificity.
In the future if we ever need to implement layers, we can replace the :root
with @layer name
, or if we want to reduce specificity to zero we remove the :root {}
wrapper.
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.
Should the .wp-block-embed figcaption rule at the top of this file have lowered specificity?
I'd say so. A rough rule could be "anything that is configurable via global styles should be".
At a glance that mixin touches color
and font-size
rules and captions are now a configurable element in Global Styles.
Rather than adding :root to each individual rule or removing :where, I thought it might be better to do something like this:
Sounds good to me. I'm still working through the impacts this PR will have on #61032 and haven't gone through all the block styles yet.
My understanding is this PR has only tweaked what was changed in the original zero-specificity PRs which has proven to be somewhat incomplete. There was some discussion that we needed a complete audit of all the block styles once everything settled.
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.
One possible issue with the suggested approach to nesting :root
and :where
like that is it might encourage people to think they can still just nest the next selector within the :where
when they really have to define the entire selector or make the nested selector chain another :where()
e.g.
:root {
:where(.wp-block-button) {
// ...
&:where(> .wp-block-button__link) {
// ...
}
}
:where(.wp-block-button > .wp-block-button__link) {
// ...
}
}
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.
Yeah, it gets a bit much doesn't it. 😬
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.
:root {
:where(.wp-block-embed) {
// ...
}
}
I had a look at this suggestion, but it becomes convoluted with some of the more complex selectors, so probably best to keep it simple right now.
I've updated the button block styles in 5a6a6f6. It's worth noting that the styles relating to fallback border display have been removed. Since #61556 landed, we have access to the merged global styles data in the editor settings. A follow-up is planned to update the border panel in the editor to leverage this data and apply fallbacks only when required. An alternative, looking to move the fallback border styles into the |
It appears the Separator block's global styles aren't working with themes that define more than just a single background color for the block in theme.json, including TT4. This is the current behaviour on trunk so I'm not 100% sure if there has been a regression or it was intended this way for some reason. Improving this can be added to the list of follow-ups. To progress this PR the Separator block's styles were updated in 2f70fdd, just note the odd behaviour when testing. |
I've made further tweaks for the Social Links, Table, and Tag Cloud blocks. These block style variation style updates need another good test but that will have to wait until tomorrow. |
(belatedly) gave this a test and all seems to be working as expected!
That sounds sensible, I went ahead and closed my PR. |
Backport is ready for review over in WordPress/wordpress-develop#6633 |
…61638) Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org>
Dev Note: WordPress 6.6 CSS SpecificityOne of the goals of WordPress 6.6 is to simplify the process for theme authors to override core styles while also maintaining support for Global Styles. Historically, high CSS specificity in core styles has made customization challenging and unpredictable, often requiring complex CSS rules to achieve desired outcomes. Development of the new section styles feature also highlighted a need for uniform CSS specificity to support nesting such styles, facilitating the creation of sophisticated, layered designs. Uniform 0-1-0 SpecificityWordPress 6.6 introduces several changes aimed at broadly reducing CSS specificity and making it more uniform. These changes generally fall into two categories:
Where adjustments to CSS specificity were required, they were achieved by wrapping the existing selector within Core Block StylesThe choice of Any blocks with Global Styles support using higher specificity selectors had those selectors wrapped in Theme.json / Global Styles:All block styles, including block style variation styles, output by Theme.json and Global Styles are now limited to UsageThe alignment of Custom BlocksAuthors of custom blocks that opt into global styles and apply default styling via a selector with greater than An example could be a custom list block that opts into padding block support but defines default padding via: ul.wp-block-custom-list {
padding: 0;
} Without adjusting the specificity of this rule, any customizations of the block type's padding in Global Styles would be overridden. Wrapping the selector in // Block's stylesheet
:root :where(ul.wp-block-custom-list) { // This is a contrived example and could simply be `.wp-block-custom-list`
padding: 0;
}
// Global Styles - Loaded after the block styles
:root :where(.wp-block-custom-list) {
padding: 1em;
} Block Styles (aka Block Style Variations)Theme authors customizing Block Styles for a core block will need to limit their style's specificity so the block style continues to be configurable via Global Styles. For example, take a theme that tweaks the border radius for the Image block's "rounded" block style: .wp-block-image.is-style-rounded img {
border-radius: 2em;
} Without adjustment, this style would override any customizations made to the Rounded block style within Global Styles. In this case, the theme can tweak its rounded image style to the following: //. Theme style
:root :where(.wp-block-image.is-style-rounded img) {
border-radius: 2em;
}
// Global Styles - Loaded after the block styles
:root :where(.wp-block-image.is-style-rounded img) {
border-radius: 4px;
} Zero-Specificity, CSS Layers, and The FutureReducing all core styles to zero specificity was explored before settling on CSS Layers were also evaluated but fell short due to not being able to enforce all styles belonged to a layer. This will change in the future at which point a combination of CSS Layers and zero-specificity can be revisited to further the benefits gained in this initial reduction of CSS specificity. More history and context can be found in this detailed discussion. Useful Links |
Addition of `:root` specificity in Core/Gutenberg seems to be overwriting this. See: WordPress/gutenberg#61638
…61638) Co-authored-by: ellatrix <ellatrix@git.wordpress.org> Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org> Co-authored-by: tellthemachines <isabel_brison@git.wordpress.org> Co-authored-by: talldan <talldanwp@git.wordpress.org> Co-authored-by: andrewserong <andrewserong@git.wordpress.org> Co-authored-by: kevin940726 <kevin940726@git.wordpress.org>
I'd be very careful saying this in the Dev Note. So far, I've seen quite a few reports from people building themes where these changes have altered their designs in various ways. I'd say that every theme and block author should check their designs. It doesn't have to be OMG, the sky is falling, but it should be proactive in telling them that the reality is that this can and will affect designs where custom CSS comes into play. |
Thanks for the feedback @justintadlock, appreciate it 👍 While numerous deep dives into the directory showed a majority of themes and plugins shouldn't need an update, that doesn't mean the amount needing tweaks is insignificant. It wasn't my intention to suggest otherwise or create a false sense of security so I've updated that sentence with the following.
I hope that strikes a better balance 🤞 |
What?
In order to apply global styles after any theme reset styles, e.g.
ol, ul { ... }
, increase the specificity slightly, while keeping CSS overall specificity levelling and specificity decreases.To do: figure out where else exactly we need
:root
, I'm not super familiar with this code.Note: Block style variations and their selectors will be handled via a separate follow-up as they haven't yet had their specificity reduction merged in Gutenberg (see #61032).
Why?
To make sure reset styles not wrapped in
:where
do not break anything.Why
:root
and nothtml
orbody
? Because:root
has the same specificity of attributes and would catch all kinds of reset styles including those with attributes (example).How?
Wrapping all our specificity normalised/levelled rules in
:root
, so that they run after these reset styles.Testing Instructions
The only unit tests we have are covering the generation of styles rather than their application, so we'll have to rely on some manual testing or adding new tests. Any help here is appreciated! :)
Manual Testing
Testing Instructions for Keyboard
Screenshots or screencast