-
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
[WIP] Styles: Remove the ! important tag from user predefined style selections #40121
Conversation
…duce specificity of table element selector to prevent it overriding user applied styles
…rwritting user applied presets
FYI - this is very much still a work in progress - quite a bit of testing is needed to check that reducing the specificity of the theme.json selectors isn't causing block defaults to be incorrectly applied. Also, this is only in the 5.9 compat code for ease of quick setup and test - if this approach moves forward it would need to be migrated to 6.0+ compat code |
So initial testing indicates that the |
See also #40159. |
Size Change: +78 B (0%) Total Size: 1.22 MB
ℹ️ View Unchanged
|
FYI, due to the questions about the support of |
Just an additional note for clarification. This approach would by no means be a silver bullet solution to this problem, and has the following limitations:
If this approach is used it probably should be seen as a temporary solution until the application of styles can be handled in a much more fine-tuned way server-side by the style engine. |
Co-authored-by: Ramon <ramonjd@users.noreply.github.com>
I've been testing this in twentytwentytwo and empty theme and it works as described: my presets are applied to blocks and elements in theme.json, but are overwritten by global styles > user defined styles. Personally I like your approach. Removing
This is true. My initial thought is that the benefits of having a predictable cascading hierarchy, without the brute force approach of
I think search block might be an example. It adds color classes to the button
Specificity is a good thing to keep in mind for the style engine, though it's going to be interesting to work out how to instruct it to output the right classes in the right scenario. It's still up in the air, but I imagine that the style engine, at least initially, would blindly follow input instructions, e.g., we'd have to make sure the arguments we pass to the style engine correspond with the specificity we want for each level of classnames (global, block etc). Having things centralized however will definitely make the work easier I hope! 🤞
👍 |
@oandregal, @youknowriad - This PR put in place what you suggested here André, ie. to go through and reduce global style and block style specificity in order to allow the Do you think it is worth exploring this further as a temporary solution to the issues caused by the addition of the |
@markhowellsmead, @cbirdsong I would be interested to know if you had any thoughts on how best to ensure that user preset classes that a user applies in the editor can be given priority over any styles set in theme.json or via global styles in the site editor if the This PR is experimenting with As an example of the current potential hierarchy of block styles, an
Cory, you suggested the use of As mentioned in the above comments the ideal situation is server-side rendering of all these styles in a way that can be smarter about when and in which order they are applied, but we are still a way off from being able to implement that, so if you can think of any alternative solutions in the meantime let me know. |
That makes things easier, then! The cleanest solution would be making sure these styles are added to the
Poking at a stock install, seems like the easiest solution might be just moving the |
Thanks for the response @cbirdsong - sorry, I left out the critical detail in my summary that some of the styles have a higher specificity further up the cascade. The css is already loaded in the correct order which is why adding the I will take another look and see if there is some way to just handle the exceptions like the table block, rather than a blanket application of |
There is another approach here which avoids the need for |
Yeah, I think it's just going to be a lot of figuring out how to best handle each one of these edge cases. In the case of the table block, that inconsistency with other blocks is very odd1. Is the issue you're seeing Footnotes
|
The problem is the fact that if a color is assigned via global styles it is added to the blocks custom selector of The theme.json styles.elements and styles.blocks.elements cause the same issue, eg. an h2 element setting in a column block in theme.json gets added via a Will keep thinking about options for getting around this that don't involve the extra 50kb of CSS that this solution creates.
oh, good, it wasn't just me that thought this was a little strange 😄 |
Have you considered custom properties? The way I handled this in my non-theme.json theme was setting a custom property in the .has-magenta-background-color {
--background-color: magenta;
} Then elsewhere that custom property can be used however it's needed for that particular element. The table example would work like this: .wp-block-table {
background-color: var(--background-color);
&.is-style-stripes {
$default-color: $gray-100;
background-color: transparent;
tbody tr:nth-child(odd) {
background-color: $default-color;
background-color: var(--background-color, #{$default-color});
}
}
} This method could also flatten out the selector generated by custom colors applied in global styles. However, I'm not sure about this:
Can you tell me how to get the editor into this state? I don't see this functionality in the site editor. |
Thanks for that example, I will have a think about what might be possible with this approach.
I don't think it is possible in the editor at the moment, you would need to have a theme.json and add the following under the styles.blocks section and then add a heading in a column block in the editor to see the effect: "core/columns": {
"elements": {
"h2": {
"color": {
"text": "red",
"background": "black"
},
}
}
} |
@cbirdsong I have a draft PR up here using this concept, seems to work at a basic level, but will do more testing over the next few days and see if there are any gotchas. Thanks again for the suggestion. |
Closing in favour of #40335 |
What?
Removes the
! important
that is being applied to user applied style presets. This was added here in order to fix an issue with theme style settings overriding user selected settings.Why?
The application of the
! important
across all the predefined classes has been a breaking change across a range of themes (#38252, #34575, #37590) as it overrides the valid use of some of these presets, and in some cases in a way which can't be worked around at the theme level, particularly with responsive typography.While it is important to ensure that the styles applied by the user in the editor take priority over global styles, and theme styles, I think it is worth reconsidering the use of
! important
to achieve this for the following reasons:! important
applied it will still override the user selection! important
in order to work due to the removal of a wrapper around the Post Title block. While it will still be an issue with any blocks applying these styles with a selector of specificity higher than (0,1,0) this can be solved on a case by case basis with the use of:where()
or:not()
While this change may make it easier for theme styles to override, either accidentally or intentionally, user applied styles, clear documentation for theme authors on how to prevent this with the use of lower specificity selectors may be a better long term solution than trying to enforce it with
! important
.How?
!important
from the preset classes.wp-block-table > table
experimental selectorN.B I have done a quick test across all the core blocks using TT2 theme and the table block was the only one I found that didn't have users settings applied as expected with the
! important
removed, but with reducing the specificity of the theme.json element selectors there is a risk that in some cases block defaults could take priority over theme styles so an detailed audit of all blocks needs to be undertaken to see if this causes any issues in this regard.Testing Instructions
Go to the editor, add three post title blocks:
Publish the post and go to the front end.
Add some theme.json element settings:
The expected behavior would be that the user choices are respected, both in the editor and front-end.