Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Explore targeting global styles to the body element instead of :root #27478

Closed
jorgefilipecosta opened this issue Dec 3, 2020 · 9 comments · Fixed by #31302
Closed

Explore targeting global styles to the body element instead of :root #27478

jorgefilipecosta opened this issue Dec 3, 2020 · 9 comments · Fixed by #31302
Assignees
@jorgefilipecosta
Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress

Comments

@jorgefilipecosta
Copy link
Member

In order to have global styles padding, it may make sense to target the global styles to the body element.

We may have some issues with this e.g: if there are elements outside the body, like overlays, etc. So we may need to target presets using :root and styles like padding using the body which adds more complexity.

This exploration may also show that padding should not be allowed globally but just for a container block. Or it may show the need for a site block, so themes have total control of where the root block is.
@jorgefilipecosta jorgefilipecosta added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Dec 3, 2020
@aristath
Copy link
Member

if there are elements outside the body, like overlays, etc. So we may need to target presets using :root and styles like padding using the body which adds more complexity.

Everything is inside the <body> element... Nothing can be outside it besides the <head> of the document. Unless by body you were referring to the actual content and not necessarily the <body> element? What am I missing?

This exploration may also show that padding should not be allowed globally but just for a container block

As a concept that makes perfect sense to me... Adding padding controls to all blocks - though possible - would result in too much noise. +1!

Or it may show the need for a site block, so themes have total control of where the root block is

I would love for us to build a "site" block which would implement a css-grid a-la-#18600, but if there is a "root"/"site" block, how would themes have total control of where it is? It would - by necessity - wrap everything, possibly being the first - and only - element inside <body>. At least that's how I understand a "site" block

@jorgefilipecosta
Copy link
Member Author

Everything is inside the element... Nothing can be outside it besides the of the document. Unless by body you were referring to the actual content and not necessarily the element? What am I missing?

Yes, every HTML content should be inside the body, but although not correct browsers still render elements placed outside the body. I guess that's not a case we should worry about given that if that's the case third party code is doing something unexpected.

@mtias, @youknowriad, @nosolosw is this issue still something we should give a try? Make global styles target body instead of:root?

@youknowriad
Copy link
Contributor

yeah, I think this makes sense.

@jorgefilipecosta jorgefilipecosta mentioned this issue Apr 28, 2021
Merged
@github-actions github-actions bot added the [Status] In Progress Tracking issues with work in progress label Apr 28, 2021
@aristath aristath mentioned this issue Nov 1, 2021
Closed
@markhowellsmead
Copy link

markhowellsmead commented Nov 19, 2021 •
edited
Loading

Many existing solutions inside and outside the WordPress ecosystem use :root as the base element, as intended by the W3C Recommendation. I believe it's been a mistake to move away from this standards-based recommendation and assign the custom properties to the body element with no real reason or benefit. The argument is correct, that there are no elements outside the body, but CSS works from the root (html or :root) element down and always has. For example, the rem unit, which is based on the font size set on html.

cc @youknowriad @keithdevon.

@youknowriad
Copy link
Contributor

Hi there @markhowellsmead I'm curious to know what kind of ecosystem usage is relying not the :root as base element here?

Personally, I don't mind moving some CSS variables back to the :root but wanted to clarify that I think styles should probably apply to .wp-site-blocks instead of body because that's the actual root for blocks. For instance if we do add a "layout" style to the root element, applying it to body or :root won't have any effect on the actual layout because it needs to be the direct parent. Also some other styles like padding and margin might have a slightly different behavior in this case.

For the CSS variables it's another story but I don't see it as a big argument for it either. What's the reason for this since everything is inside wp-site-blocks and we do need to generate styles with that selector. Meaning there's the argument of consistency here.

@markhowellsmead
Copy link

As I've mentioned above, the W3C recommendation is that CSS custom properties adhere to the same principles as all other CSS rules: that definitions begin on the html element and then cascade down from there. I mentioned in #35840 that the decision to place the generated rules on the body breaks the cascade for developers who place their definitions in line with the W3C recommendation. (There's an example linked in that issue.) Here, I'm specifically referring to the definition of the custom properties (like colours), which should be available to the entire document, not just the block container. Not layout rules.

Regarding layout rules: applying these should certainly begin at the block container, not the body. Until now, various solutions have used .editor-styles-wrapper as the base class for the editor. Changing this would potentially break a lot of pre-existing themes and I don't personally see any need for this change.

I think it's a bad idea for core to enforce a class name for the block container in the frontend, as this will add to the existing issues where core CSS is too specific already. (Sometimes too specific to override easily.)

@markhowellsmead
Copy link

One specific case of the need for custom properties to be applied to :root is CSS Vars Ponyfill, which is in use on thousands of sites to allow custom properties to work in e.g. IE11. I appreciate completely that WordPress has dropped support for IE11, but sticking to W3C recommendations would allow a side-effect of this third-party solution to continue working, without any additional work on the part of WordPress developers.

@markhowellsmead
Copy link

markhowellsmead commented Nov 22, 2021 •
edited
Loading

Just as a quick clarification: having re-read your comment, I think you're primarily looking at this with regards to the editor. This issue is possibly more relevant to the rules which are auto-generated for the frontend of the website. I can see that applying the custom properties to the block root container in WordPress Admin could make sense, but it's important to note that some themes contain CSS rules for elements which are outside the block root element (e.g. the block sidebar panels), which lie outside .wp-site-blocks and .editor-styles-wrapper. This would be another good reason to apply the custom properties as high up the DOM structure as possible.

@aristath aristath mentioned this issue Jul 1, 2022
Merged
1 task
@ramonjd
Copy link
Member

ramonjd commented Jul 7, 2022

I have a PR going in #42084 that proposes to change body to :root for the preset CSS declaration block only.

Are the reasons above still valid?

Is this a bad idea? If so, what makes the case different to other instances of where Gutenberg uses :root? Example

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@jorgefilipecosta jorgefilipecosta

Labels
Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Status] In Progress Tracking issues with work in progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Update: Target global styles to the body element instead of :root WordPress/gutenberg
5 participants
@youknowriad @aristath @markhowellsmead @ramonjd @jorgefilipecosta