Try: Reduce specificity of reset & classic styles. #32659
Conversation
jasmussen
added
the
CSS Styling
jasmussen
requested review from
youknowriad,
scruffian,
MaggieCabrera and
carolinan
|
Note to self, need to push a fix for the Gallery block:
This rule is now winning:
Easy fix, but will let you all add some thoughts first. |
Interesting use-case, potentially we could consider doing this PR for just "resets" for now maybe? I think there might be other cases where themes could inadvertantly override the default centering margins (like figure maybe) |
Agreed, even if we don't land this for the classic styles, it seems a win to land it for the reset. That said, this PR does solve an issue with navigation block justification in classic themes, which isn't trivial to fix otherwise: if I increase the specificity in the navigation block, I break global styles. |
|
Size Change: +54 B (0%) Total Size: 1.08 MB
ℹ️ View Unchanged
|
|
🤔 What happens if we do this to the editor styles as well (the ones injected dynamically)? |
|
Blowing my mind with that idea. Do you happen to recall where we rewrite I wanted to also add the thought that we could make the changes suggested here to everything except the left/right auto margins. While that breaks the nav block issue I noted (I can look for other fixes), it would still let us lower the specificity of the top/bottom margin rules, which would be a nice win. |
|
@jasmussen it happens here https://github.com/WordPress/gutenberg/blob/trunk/packages/block-editor/src/utils/transform-styles/transforms/wrap.js basically, the ideal there would be to wrap without altering the specificity, not sure that's possible though |
|
I gave it a shot, but I think that lowers the wrong specificity:
So this margin rule from the theme:
Now it loses to the block style:
In other words, it does work as intended. But we need to apply this to everything we want to be overridden. So in the case of the left aligned gallery, I can add the
We could potentially use |
TIL, that's pretty neat! ✨ Zooming out a bit, did we have specificity targets for different parts of the editor? Using a comma delimited notation (style, id, class, element) with resets we'd probably stick with might have scores from (0,0,0,X), while blocks might be aiming for something like (0,0,1,X) to (0,0,2,X)? As a reminder for folks 👀 CSS specificity is mostly counting. And no matter the number of items in one category it can't beat the next one. Using a comma delimited notation, if I say had 15 element rules in a selector a single class selector would still beat it. In Joen's example in #32659 (comment) This would be (0,0,1,1) vs (0,0,1,0) (with the :where pseudo selector). |
It's been adhoc so far, but it might be good to start thinking about this in a more systematic way, in part because we have this new Do you have any good suggestions on how to start systematizing that? Are there any principles we can extract in documentation? Any linters we can add? |
|
Curious, though, how did you calculate the specificity of |
Manually counting, checkout also https://www.w3.org/TR/selectors/#specificity. From the spec :where and it's argument are always zero
So with that in mind
So probably the hierarchy of styling will probably be something like: Reset / Base Styles -> Block Styles -> [ Custom Theme / Editor Styles ] -> Global Styles -> Specific Block User Overrides If we can provide consistent specificity targets at the first two layers, then it should be easier to provide guidelines for folks building themes, and the necessary overrides for anything above that. Note that we don't necessarily have to use strict specificity guidelines if we're sandboxing styling in other ways (say isolating rules via iframes and such). |
|
Thank you as always for teaching me, I very much appreciate that. Your hierarchy reminds me of #20331, which seems like healthy serendipity worth building on. The tricky bit is the theme, as it requires some care on part of the author to not mess up the global style user overrides. However as those same properties are provided by theme.json directly, hopefully that helps manage the complexity. |
This hierarchy makes sense but let me clarify the difficulty a bit here. In the frontend, the equivalent hierarchy is: So our goal is to match both hierarchies as closely as possible, there are thought two main differences: 1- in the editor we need a that's just an example, the important bit here is that we're trying to reset the styles of the base elements to the browser's defaults by overriding the WP-Admin common.css styles. But by doing so we increased the specificity of 2- To solve the issue above what we did (a long time ago) is to add The problem remains though for 1- Try to wrap all the styles that can affect blocks with 2- Make sure the "reset" and the editor styles don't affect the specificity of their rules when they wrap the CSS with. |
|
Thanks @youknowriad and @gwwar for the reviews. Before we go deeper on the path of a hierarchy, is this PR okay to land as an interim step? |
|
Thanks for looking. I have this feeling those are all issues best addressed in the theme itself, does that seem right to you? If so, I'd be happy to create tickets for those and/or look at patches. Or did you get a sense that this was fixable in the block editor, without raising specificity again? |
|
I'm not sure. I think some should be fixed in Gutenberg and some in themes. In the case of the Gallery with no alignment, 2020 theme adds the |
|
Excellent catch on the gallery. It appears to not be a problem with block themes as the left/right auto margins are enforced as !important. But it is indeed a problem in classic themes, as the margin: 0 rule now wins out:
While I still think lowering that specificity, and actually updating the rule of the Gallery block to not have such a blanket zero margin output, it does make me question whether it's actually a good idea to update those classic rules at all. We might want to do it for the reset styles only after all 🤔 Block themes will still benefit from the new lowered specificity of the reset style. What do you think? And thanks again for looking. |
|
Though it is worth noting, that if we don't include the lowered specificity, then improvements from #33021, just as an example, can't benefit classic themes. This will likely expand more widely as more properties like margins and paddings are absorbed into global styles. |
|
The Gallery block issue would most visible, so it might be good to fix that as a follow-up. I think it would be easier to gather more feedback if we merged this PR. |
|
I'm waffling on merging it, as I'm going on vacation next week, so I won't have time to follow up on this. So at this moment I'm leaning towards pausing this one until I get back. |
jasmussen
force-pushed
the
try/reduced-specificity-resets
branch
from
641ec16 to
f2cb2e8
Compare
|
I'm back from AFK and hope to land this soon. Since it also appears to improve a 2019 theme issue, it would be good to land soon. I've rebased to test that it's still valid, and will be investigating the gallery block issue and how that can be fixed, and then either push a fix to this branch, or have a followup ready. |
|
Alright, after investigating a bit further, it appears that the issue, as so well reviewed by @Mamaduka in #32659 (comment), is primarily an issue in classic themes. Here's what happens in 2020 that causes the gallery to be left aligned. The theme outputs this rule:
This rule is common for themes, as the
So there are two things we can do here. 1. Restore specificity for those auto margins. This would be a bit of a bummer as I recall those margins being frustrating when I mostly worked on classic themes. But it might be a pragmatic choice for backward compatibility. Block themes won't have to worry about them, at least. Keep something like this in classic.scss, in other words: 2. Expand the list of blocks with specific auto margins. This PR already had the following in place, to center lists: We could expand that definition to include the figure element, which fixes the 2020 issue and still keeps the generic .wp-block class low-specificity. Something like this: It fixes it for 2020:
The downside to that approach is that it very slightly increases the specificity of those margin rules for classic themes. 0,2,1 for Personally I'm slightly in favor of option 2, because those margins are annoying. But it's not a strong preference, and I'd be happy to go the potentially more pragmatic route. Before I push anything, I'd love your thoughts. Thanks. |
Personally I think this is a mistake and should be removed from all blocks, it shouldn't be the responsibility of the block to define its margins but should be the responsibility of its container ideally. Of course there might be special cases to clear out margins... but margin auto is very specific to the "flow" layout. |
|
Going forward with block themes, there won't be any auto margin applied. But given how legacy themes have assumed those auto margins for a while, would you be okay with option 1 then? |
|
Yes, I think option 1 is better. These "classic" styles are not "reset" styles meaning It's not clear whether they should have less specificity or more than editor styles. So I think keeping that rule as is is a good approach. |
|
Awesome. Things still work well here:
|
| .wp-block { | ||
| // Deprecated style needed for the block widths and alignments. | ||
| // for themes that don't support the new layout (theme.json). | ||
| html :where(.wp-block) { | ||
| max-width: $content-width; | ||
|
|
||
| // Provide every block with a default base margin. This margin provides a consistent spacing |
There was a problem hiding this comment.
Noting that these margins can also get removed by figure { margin: 0; } do you think we should also move them up?
There was a problem hiding this comment.
I'd personally prefer to keep them there, as they as I recall were the most disruptive to work with, and the reduced specificity of those margins were the biggest win of this approach. It should also be less disruptive as it's vertical margins.
However I'm happy to move them up either in this PR or as a followup if it turns out to be necessary?
There was a problem hiding this comment.
the biggest win of this approach.
I'm curious, in which situation the smaller specificity benefit classic themes here. What kind of styles classic themes apply here?
There was a problem hiding this comment.
I'm thinking mostly of developing new classic themes, or refactoring older ones.
There was a problem hiding this comment.
I agree with @jasmussen that it should be fine to keep these where they are. I tested in a few themes and didn't notice this causing any issues. If they did interfere with that figure rule, it'll just be the vertical margin (and may actually have no effect if there's margin on the items above + below the figure anyway).
There was a problem hiding this comment.
Thanks for looking Kjell. Cool, in any case it's an easy follow up which I will be keeping my eyes peeled on and ready to change if need be. I'd like to land this PR tomorrow or Monday, so there's still time to chime in here.
|
Alright, it's Monday and I'll be here for any followups. This one after recent iterations feels very safe to me, so I expect when I merge it in a moment it will be a non issue. Nevertheless, let's keep an eye out on theme styles in the editing canvas! |
Description
We have two stylesheets that are created to normalize the editing canvas:
reset.scssandclassic.scss.classic.scssis loaded for classic themes, and primarily serves to provide left/right/wide/full-wide alignment styles. Block themes do not need these, as those alignments are served by alayoutproperty and handled by the system.In both cases, while the styles are necessary, they should have as low specificity as is possible. The lower the specificity, the easier it is for a theme to style things in a nice way.
There is a new selector, called
:where, which is able to provide default styles with lowered specificity. Some of the details on how that works are outlined in this article, but you can also check out this codepen for an example. That selector is theoretically ideal for these resets, because it allows us to reduce the specificity quite a bit, letting both theme and block styles win any specificity fight they get into. In this PR, I tried employing that selector for those resets.Overall, it worked very well, especially for the reset which just feels perfect. Here's before and after using "empty theme" — and they should look identical, only with the new version having lower specificity:
After:
For other themes, here I tested TT1, it solved this problem surfaced by @carolinan, where
margin-left: auto; margin-right: auto;overrides the default navigation item margin, causing them to collapse. Before:(don't mind the padding, that's a separate issue)
After:
In these screenshots, the navigation menu is justified right.
The primary "gotcha" I encountered is that themes commonly apply a
margin-left: 0;onolandulelements, which seems an innocent and sensible change over the defaults. The problem is that for classic themes, themargin-left: auto; margin-right: auto;rules exist (along with amax-width) to center a block inside a post content column that supports wide and fullwide images. When left margins are zeroed out, it effectively means a left-aligned list block. In trunk, the auto-margins win due to their intrinsic specificity. But with the lowered specificity of this branch, the zero margin now wins.For that reason, I included a targeted rule for
olandulelements, to keep their specificity. It's not an inclusion I love, but it feels necessary as the change feels very common. What do you think?How has this been tested?
Checklist:
*.native.jsfiles for terms that need renaming or removal).