-
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
CSS Custom Properties on BODY lead to DOM inheritance issues #35840
Comments
(Aside: I've looked it up, and we're talking about |
Related: #27478 |
I think that "classically" is misleading here. The transformation of usage of CSS Custom Properties has matured (rightly so) in the past few years. There is no classical usage in my mind, and merely an awareness and attempt at defining and shifting common practices. I actually think it is a great idea to target and keep |
I agree that “classically” wasn't the right term to use. Progressive improvement is certainly the right way to go. Educating theme and plugin authors is certainly laudable, but they need to be educated to use CSS as it was intended, and as it's defined by the standards at the W3C. WordPress needs to follow the standards, and not divert from them without an essential reason. It's definitely important to set the site-wide values as high up the cascade as they can go, in order to provide the maximum level of support. In this case, and according to the W3C definitions, that's the document root object This is especially important when, as in the case I've shown, the current WordPress Core logic leads to errors and usability problems which individual developers - myself included - have to work around by mutating core logic in every project. |
Additional: the problem is that if developers use the |
Correct, and in that case they would want to use the
There are no standards, but only recommendations from W3C. |
The problem can be worked around, but it would involve every front end developer who ever works on theme and plugin development for WordPress having to recognise a special case. Where they may have to diverge from standard CSS practice to accommodate a system-specific decision.
I know. But deliberately going against the W3C recommendations was how we arrived at the various browser prefixing and cross-browser conflicts which have plagued the industry for so many years. This is a simple drop in the ocean, but I'm still not seeing the need to diverge from a procedure which is recommended and used by so many developers, both within and without the WordPress ecosystem. Can you clarify which decisions were taken to arrive at the reason to use |
If anyone's following along (or finds this thread), then the code in this Gist will parse the contents of |
Additional note: one result of adding the rules to |
I agree with @markhowellsmead here. It looks like this change was made here: #31302 to allow...
It would be great to know what this means and whether there is a way to use
@colorful-tones are you able to expand on this? How does using
Can you start by educating me, please? 😂 |
Hey guys. I was thinking that maybe the selector specificity for declaring CSS variables in global styles could be even lowered to But if IE11 support with |
I would discourage usage of :where() for such a fundamental thing, as it’s not that well supported: #39621 |
Bumping this again as pull request #42084 which addressed the issue was marked as “not a priority”. I'd like to know why this isn't a priority, and how the development team came to this conclusion. |
1 similar comment
Bumping this again as pull request #42084 which addressed the issue was marked as “not a priority”. I'd like to know why this isn't a priority, and how the development team came to this conclusion. |
Here is another issue that is caused by this: #39131 |
Bumping this -- it's still a problem for theme and plugin developers. |
I'm sorry. I do not intend to increase frustration and/or exhaust your patience @markhowellsmead. I want to understand the use case and challenge. I can be dense at times and ask for extra clarification 🙃
Yes, understood. The cascade does not ascend but descends for inheritance. 👍 I think clarifying your expectations around where these definitions are coming from might help give context to the friction in the use-case. For example, in your original code snippet:
I would ask: why is the theme author assigning their variables ( For the |
These are all examples and apply to custom apps built on a WordPress basis, as well as in themes and plugins. W3C recommendations are to define top-level CSS Custom Properties on the document root element
Again: check the documentation for CSS Custom Properties. Top-level rules can be overridden at any level in the DOM, provided that the basis rule is applied as high up the DOM as possible.
Once again: this is just a simple example for maximal clarity. |
There are also other more subtle issues which may not be immediately evident, which are linked in other issues above. |
Just trying to revisit and gather further context here...
There is nothing in the referenced W3C Candidate Recommendation Snapshot that states anything to the effect: CSS variables should be assigned to the Furthermore, the referenced document is a W3C Candidate Recommendation Snapshot, which represents the following as defined by W3C:
Again, I'm not trying to exhaust the issue and merely trying to clarify for all parties. ❤️ |
Here's MDN:
Additionally, as mentioned in a comment by @Inwerpsel on the PR that merged in this change (#31302):
Simply put, custom properties being output on The reasons given for this change made in that PR and the accompanying issue do not really make sense, and no further explanation has been articulated, despite someone asking very explicitly about that in this issue and in the original issue. This feels like a case of Wordpress going its own way for arcane internal reasons and offloading the cost of that onto theme/plugin developers. |
I still have not heard or seen any compelling evidence to validate this beyond it is a "standard" (reference to a W3C Candidate Recommendation Snapshot, which never indicates it as a standard and even explicitly states the contrary) and "a common best practice." (Reference MDN) I think some stubborn opinions are held onto regarding these third-party resources. Yes, MDN and W3C are both valid, long-standing, and sound resources, but neither should be recognized as binding laws. These resources are governed by stakeholders, just like WordPress, and standards are continuously being discussed and molded. |
@jorgefilipecosta @aristath @youknowriad do either of you folks have insight into history of how Noting that a lot of folks are in transit for WordCamp US right now and response times may be delayed. |
To be clear, you're right that it is just a convention, and I'm not adamantly opposed to these staying on Footnotes
|
Oh, good tip! I wasn't aware of that one.
This might become easier when the editor is loaded in its own iframe, wouldn't it? |
I primarily wanted to highlight the practical problems resulting from using I agree there's no completely sound argument, based on the spec, to use either. You have to realize that if you use both together randomly, you'll create some very hard to understand bugs for a substantial amount of people. If you add custom properties to the mix, answering the question "why is my style not applied?" can get quite hairy. So it's best to make some choice and stick to it. Now if you choose |
I tried to do a simple thing – add a color to the page's scrollbar using variable colors, but due to the current CSS specificity I can't do it. What is the reason for the variables being in the body element? |
Still a problem. |
I did not read through the reasoning, I saw something about making it less complex. Can someone give me a summary why this exits at all? They want to overwrite some theme base styles with the theme.json? Why not just enqueue a base style sheet earlier than the inline styles are put out? It's common practice to use You can not target scrollbars ... I just read about that you can target SVG with root so it makes a lot of sense why it's common practice, but GB messes it all up with this decision. |
Pinging @WordPress/outreach, I managed to get a little traction on this last year but it hasn't been discussed any further since then. |
Actually, I want to correct myself, you can easily overwrite Bootstraps CSS vars. In fact, WP putting custom vars out on the body makes even more sure vars get overwritten, even if themes styles come later. And I guess that is actually the reasoning for this. So if you set the SCSS variable
So, not as bad as I thought. In fact, because by default the theme style comes after the inline styles from GB, if I get this right this works specifically because the vars are put out on the My suggestion if it is hard to change now would be to introduce a new Quote from the OP:
The order is wrong for the typical case where you enqueue your theme's stylesheet on
But why not do it like this:
And then use |
I can see the point you're trying to make, @nextgenthemes, but the order of this specific CSS is irrelevant. The value
That is a use case for small projects and ones with few custom definitions. The projects I build for clients commonly have dozens—if not hundreds—of CSS custom properties for all manner of rules, so it's impractical to leave behind a solution which was introduced to CSS itself, which allows this to work without needing to implement system-specific workarounds. Some might believe that WordPress can provide a solution which replaces the need for such custom development. That is not a realistic goal. Given that on big projects, there are always requirements for complex animation, layout shifts using CSS Grid across myriad breakpoints, and dynamic script-based interaction, sticking to years-long-established best practices will stop WordPress from being a hindrance. WordPress encourages developers to use the theme.json solution it provides instead of using commonly-used best practices which have been in place for many years. The Web Standards Project began in 1998 and pushed developers and browser makers into working towards a common goal instead of working on proprietary solutions. It remains essential that WordPress doesn't actively stop W3C Recommendations from working, which it does at the moment by insisting that the top-level element is the |
The pull request #42084 was working towards a solution for this, but was closed in 2023 because it was “old”. 🤔 |
This comment #31302 (comment) also points out that people using JavaScript to interact with the custom properties will encounter problems because of the divergence from established practices and documentation. |
@markhowellsmead I said
So you do not need to tell me
I am aware, and I understand everything about this thing now, this was the reason for my long comment explaining it. I have no clue what you are on about W3C again, @colorful-tones already told you that you are wrong.
WP devs never did such thing! I am basically on your side, but this made up stuff is not helping your argument. Using a different element for CSS vars is for sure not a "proprietary solution". Given 5K issues and how WP and GB works dev works, I would not get my hopes up into this changing any time soon, @ramonjd with the PR self closed in a kind of hilarious way.
Who is "we", the GB dev team? He killed his own work on it because it's "old". This looks like the end of this to me. Now everything is build on top of this bad idea and WP has an insane backwards compat policy, so they will probably just say they can't change it now because there is already so much build on top of it, if they care at all. It could an opt-in switch ... But if it's just about "many CSS" vars, what is the actual difference besides the forced naming that can probably even changed with code if you declare them in a (S)CSS or a JSON file? Well I know one thing, it's not cached because it's inline in the HTML on every page again and again and that sucks for performance optimization. But WP is going the road of putting out all these chunks of inline stuff, it has positives and negatives ... |
@tellthemachines I'm curious whether you have any insights here with your recent work or lowering the specificity/ looking at css layers :) |
@nextgenthemes I'm sorry to read that you're so negative about the potential for improvement. I respect other people's opinions. I've referenced the W3C Recommendations throughout because of their long-standing place at the centre of platform-independent HTML and CSS development since the 1990s. My comment about WordPress using |
@nextgenthemes, there is no wrong or correct, and we’re discussing this communally to reach a consensus by stating and understanding the nuances. I appreciate @markhowellsmead opening this issue and diligently providing clarity and details to push the conversation forward. As well as the many other folks that have chimed in. I worry that the proposed changes may have a dramatic impact, but I have little understanding of what the changes would look like. I would be happy to champion thorough solution testing and solicit community feedback if someone were to reopen or put forward a pull request. Perhaps we could target this for the next WordPress 6.6 release, as a proposed item. 🤔 |
If there is a change made here, then it will probably have a notable impact, which stakeholders need to assess. It's an important decision which needs to be taken by consensus, not just based on my opinion or by the individual opinions pro and contra. Perhaps @ramonjd can add his opinions to the discussion, and consider re-opening his pull request if he feels that it remains relevant (and, specifically, technically viable based on the current codebase). |
TLTR:
I told you that I am for this change, here you go making stuff up again. You act as if I am against this improvement, never did I say that. You're calling this "proprietary" is a stretch if I ever heard one. And I made perfectly clear that I do understand this issue multiple times, there is no point for repeating yourself and explaining it to me like I do not understand what your reasons are why you opened this. My guess spamming around here won't help either.
Based on what I read above, he claimed that putting it on Quoting you quoting their stuff:
So you are in fact right and he is wrong! Sorry, I am not playing this bullshit "understanding nuances" game. You know you are right, and I know it. But this forced niceness and pretending things that are not true is trendy these days. Fits perfectly, who puts his pronouns on his profile. ROFL. And after he was corrected, he brought up W3C again, completely ignoring the fact you stated. That was my issue with it. People hate facts so much on the internet, you are not supposed to point them out, you are deemed "negative" for stating them and calling people out for being wrong. That even people who did the calling out come up with pretentious bullshit, it's hilarious. It's the absolute worst of Reddit, Hacker News also suffers from this, and now It's even in GitHub issues. It's so 2024 Are there filters for inline CSS, because at least we should have those. With a filter, we should just be able to |
Hey folks. Let's please center the discussion on the issue at hand. A lot of folks were tagged in this when the As for the PR being closed out, to add some context, it's encouraged that contributors working on the project regularly review their open PRs to ensure that those who might want to contribute with code (either with reviews, testing, or code) have a higher chance of jumping into active, relevant work. This is also so that those relying on a solution can understand whether something is in progress. This is tricky to get right as noted by the 1.1K pull requests and I appreciate the proactiveness there to close it out. |
I recently revisited the body/:root conundrum in an unrelated PR. I originally wanted to flip "body" to ":root" across the codebase, but quickly discovered that CSS declarations in the body tag are there in order that theme.json styles can overwrite browser defaults such as margin. As confirmed here, I believe that was the intention behind #31302 CSS custom properties, on the other hand, may still be safely contained under I'm happy to reopen and rebase that PR if folks are willing to help test it?
To add context to the citation and comments, #42084 was "old" in Github PR terms, since the codebase, in particular the theme JSON class, had changed a lot. More relevant was that the PR went many months without interest from testers/reviewers. Furthermore, other priorities arose. It was closed to indicate that that specific PR was no longer "in progress", not because the topic was irrelevant or defunct. Thanks to @annezazu, who described it best above.
I recognize and appreciate @markhowellsmead's persistence and support on this issue. It costs nothing to reopen a PR besides a bit of rebase tomfoolery 😄 I'm still convinced it's worth a shot. It'd be great to get it widely tested as well. I'll make sure to promote it as much as I can among other core devs. Cheers! |
There shouldn't be any specificity-related consequences to this change either way. |
Thanks for taking the time to look at this @ramonjd and @tellthemachines. I'm appreciative of the work you're doing @ramonjd and thanks to the linked issues and pull requests, I have a better understanding that the topic of applying CSS selector rules—not custom properties—to the document root in order to resolve other issues has become particularly complex. I'll be honest and say that I can't appreciate the finer details without a deep dive, which I'd be happy to schedule in if you think it'd be appropriate. I can now understand the use case of applying the top-level rules (background, margins etc.) from theme.json to the However, the specific point in this particular issue relates to the assignment of CSS custom properties to what I believe is the “correct” root element. I don't feel that the custom properties and the selector-based rules necessarily need to be applied to the same element, though. |
Description
Classically, the assignment of CSS Custom Properties begins at the top-level (or "root") element through the use of
:root
. This is common usage and allows the values to be inherited down through the DOM tree.(It is also one of the methods by which the popular IE11 polyfill
css-vars-ponyfill
works. I realise that IE11 support is being dropped from WordPress Core, but solving the inheritance issue would also allow this ponyfill to work again with no additional effort.)Gutenberg adds the generated CSS Custom Properties to the
body
element, and not:root
, through applyingglobal-styles
using thewp_add_inline_style
function. This leads to issues where third-party CSS authors connect the generated properties to their own Custom Properties on:root
. The generated properties are not available at the top root level, and so the standard CSS cascade (inheritance) doesn't work.Recommendation (and request!): apply the global CSS Custom Properties to
:root
, notbody
.Step-by-step reproduction instructions
Screenshots, screen recording, code snippet
Here's a Codepen example of the code above: https://codepen.io/permanenttourist/pen/oNeYgVP?editors=1100
Environment info
Please confirm that you have searched existing issues in the repo.
Yes
Please confirm that you have tested with all plugins deactivated except Gutenberg.
Yes
The text was updated successfully, but these errors were encountered: