Increase font-size selector specificity #22671
Conversation
oandregal
requested review from
ajitbohra,
jorgefilipecosta,
Soean and
talldan
as code owners
|
Size Change: +4 B (0%) Total Size: 1.12 MB
ℹ️ View Unchanged
|
|
From what I can tell at a glance, this is nice, simple, and does not negatively affect themes as far as I can tell. So if it benefits the work you're doing, this seems okay. I'll keep thinking about this, and would love to also hear what Kjell thinks. but overall, 👍 👍 |
|
The added specificity here appears to break theme font sizes for Twenty Twenty. It also makes it tougher for themes to override these custom sizes — if their editor stylesheet declares something like
|
|
Thanks for the insight, Kjell! A meta-question is: what is the right path forward, and do editor styles need to change to accommodate the right path? This PR is part of the global styles efforts, and to some extent these efforts need to override some theme styles. As part of that, the theme itself will also get new tools to help adjust and accommodate these overrides. It may be a chicken and egg situation, where until global styles can be configured by the theme itself. But it all has to be balanced against the breakage along the way, of course. I personally definitely prefer the One other option is to leverage the fact that Final question: would this conversation be different if the editor was in an iframe? |
|
Thanks for the feedback! For context, this PR was created to have this conversation and not because it was really necessary for any of the work we are doing for the Block Style System. Primarily, the conversation we had pivoted on why colors increase its selector specificity with It seems that increased specificity is necessary for colors & pseudo-classes (hover, visited, active statuses), which doesn't look like something fonts need in practice. I've looked at GitHub issues and I couldn't find any issue that would be resolved by this change. Although I like how the code becomes simpler, I wonder if that warrants a breaking change, so I'm inclined to close this PR as for now. The situation Kjell mentions (themes using the same preset names that core so having specificity issues due to the enqueued classes) is something that should be resolved by a different approach: either conditionally enqueue core presets only if the theme doesn't provide any and/or by using theme.json (by managing the theme CSS we can do "smart things" easily without the theme author having to worry about this). |
Yes and I think that's an important point. I would love to see us, if possible, land on the same end result for both font sizes and colors. The inconsistency is likely to create confusion for future developers.
This sounds good. |
This sounds excellent to me. |
There was a problem hiding this comment.
With #25768 we are now automatically generating the preset classes. I guess we may discuss what specificity we should generate, but I think by default we should just use a simple class. Themes can always use custom CSS with higher specificity if needed.
|
Hi @nosolosw, given that now we are handling selectors with global styles I guess we can close this PR? |
|
Right, let's clean this up and close, it wasn't a good fit anyway. |
This PR is a follow-up to this conversation.
While adding the editor styles wrapper to the preset colors we discussed why we used the
:rootpseudo-class there while we didn't for font-sizes. It looks like the:rootfor colors was added to cover for hover/focus statuses (PR, conversation).The question is: is there a similar need for font-sizes? Is this PR useful?
cc @kjellr @jasmussen