-
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
Manage CSS for Global Styles #20290
Manage CSS for Global Styles #20290
Conversation
@nosolosw Haiii!!! 👋 Apologies for the late reply! My day was pretty packed yesterday and didn't get me many opportunities for code things. I did review this update early and let it marinate throughout the day. These are my thoughts :) I think we need a
|
Certainly, we need to scope the styles within the editor. How about using the existing
A few things here!
body { /* container */
color: black;
}
p { /* inner block */
color: red;
} The example you shared also highlights the importance of focused selectors. My current thinking is that core should come with pre-defined selectors for each block and that selectors for third-party blocks should also be created when a block is registered. Theme authors would have them ready to use. Beyond those, it's on the authors to register the selectors they need at convenience -- I'm thinking of highly specific selectors like "target cite within a blockquote". Finally, and perhaps the biggest issue I see with this proposal is how would we surface this to users? They are a few open questions here: how do we detect and expose in the UI the custom areas that are modifiable? Should users only be able to tweak the custom areas themes have declared or all the ones available? etc. I've seen the expectation that the system is able to do this (here, here, and here), so I guess is something that needs figuring out at the design level? |
e3b98b9
to
bc34183
Compare
Updated this to work with the global-style theme changes at WordPress/theme-experiments#22 and to use the scoped class |
Size Change: +278 kB (25%) 🚨 Total Size: 1.11 MB
ℹ️ View Unchanged
|
bc34183
to
86e956b
Compare
03f7867
to
73fbff2
Compare
73fbff2
to
c6ec6b2
Compare
Pushed the changes that adapted this to the theme.json schema discussed in this thread. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @nosolosw,
What if we include in this PR mechanism setting the default lineHeight from the paragraph (a direct property) and link color in a paragraph (a CSS var). Currently, this PR is focusing on colors/gradients/font sizes/settings; we already have a PR focusing on colors #21490 (and can be easily extended for font sizes and gradients) and a PR focusing on settings #22291. It would be nice if this one included the end to end cycle for setting lineHeight and including the theme.json mechanism for link color.
I'd like to keep the scope of this PR under control, but I'm happy to work on the link color thing if nobody has owned them yet when I get to it. Note that, when this PR lands, theme authors can already set any property that has been declared via implicit atts at the block level (color, font-size, and line-height). I didn't use line-height in the demo theme but you already can! The next step I'd like to prepare is how to we make this automatic: so when a block declares an implicit property at the block level, it can already be used at the global level removing the need for maintaining this list. It also touches the block registration bit I've mentioned here (see tasks section). |
We cannot use the full preset schema, including names, because our tooling is not able to internationalize the names from the theme.json file, we can use the final format for them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @nosolosw nice work here the engine seems to be in a good shape 👍
I think we may be able to discard the global property.
Currently, we can change the styles of a group block using:
"core/group": {
"styles": {
"color": "blue",
"font-size": "14"
}
"presets": {
color:
gradient:
}
}
In the future if we add nested selectors I imagine the shape would look like this:
"core/group": {
"styles": {
"color": "blue",
"font-size": "14"
},
"presets": {
color:
gradient:
},
"core/paragraph": {
"styles": {
"color": "blue",
"font-size": "14"
}
}
}
I imagine the global styles should mirror that shape for the group and just assume a :root selector.
So the shape of global styles would be:
{
"styles": {
"color": "blue",
"font-size": "14"
},
"presets": {
color:
gradient:
},
"core/paragraph": {
"styles": {
"color": "blue",
"font-size": "14"
}
}
}
While currently, it is:
{
global: {
"styles": {
"color": "blue",
"font-size": "14"
},
"presets": {
color:
gradient:
}
},
"core/paragraph": {
"styles": {
"color": "blue",
"font-size": "14"
}
}
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am not able to output styles for the blocks e.g:
p {
color: red;
}
But I am not being able to output global styles like:
:root {
color: red;
}
Using:
{
"global": {
"styles": {
"color": "red"
}
}
}
How can I output the global styles?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems we may have a bug in the merging mechanism.
If I add the following styles as core defaults:
"core/paragraph": {
"styles": {
"color": "blue",
"font-size": "14"
}
}
And the following styles in theme.json:
"core/paragraph": {
"styles": {
"color": "red"
}
}
The result is:
p {
color: red;
}
I think the result should be:
p {
color: red;
"font-size": "14"
}
@jorgefilipecosta that's really good feedback that I'm processing right now and I'll ping you when this is ready for the next round. Wanted to comment on a few general things to keep the conversation going while I act on feedback:
I'd like to keep this conversation in the thread about theme.json structure. I've already responded there.
I'd say this is a bug on the expectations for this feature! What I'm saying is that it works as I made it to. However, my expectations may need re-adjustment, I was actually on the fence about how that should work. My rationale for the current behavior was that it works like the presets: if the theme provides a preset/style/etc, core defaults are discarded. Your comment made me realize that the expectations for styles and presets are different:
|
- presets: each preset category (color, font-size, gradients) override each other - styles: leaf items override each other - features: leaf items override each other
@jorgefilipecosta I've implemented your suggestions, this is ready for the next round. There's this bit of feedback about the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, the problems I identified were addressed and things seem to be working as expected.
I don't see any blocker.
I think we as follow up we should see how can we remove the hardcoded map of blocks and how we can output truly global styles(in this PR can only output styles to a specific block).
There is also a question of how can themes target multiple blocks? In CSS one would do p, h1 {...} Should we also accept comma-separated selectors?
Thanks all for the reviews, patience, and ideas to improve this PR. To recap, this advanced the use of experimental-theme.json by themes at the framework level. It added:
Note that this is still experimental and needs iteration in many areas. The fact that people can play now with it will help us to gather more feedback and adjust accordingly. |
Initial docs for this (and related PRs) at #22518 |
They are since a couple of hours ago (since #22520 landed). You may have an older master version, perhaps? |
Ah, I now see that your comment is from 2h ago as well! Definitely update master :) |
I tried this out and in general it's working as described! 👏 Some comments / questions specifically on how this is formatted today, please let me know if there's a better way to direct this: Presets
Styles
Features
|
First, let me say that I've just merged initial documentation for this. Unfortunately, it isn't visible in the handbook just yet because it is only updated in every WordPress release now. In case it needs some clarification, etc, perhaps this would be a good place to comment. Presets
Styles: 👍 Features:
|
Hi @nosolosw I noticed that when we set a text color the style shape on the block is |
@jorgefilipecosta we certainly need to consolidate all the things (block.json, block atts, theme.json)! :) As a general rule, I think it'd be good for velocity to have focused PRs that do one thing. The link color property (which has a lot to discuss about) could be perhaps split in two: add it to the block level + add it to the theme.json. We could have another to consolidate the shape of different things, etc. |
Continues work from: #19883 #20047
See larger picture at #22296
This PR explores a way in which we can "manage CSS" for global styles. Instead of writing CSS directly, themes would provide their design choices for blocks. Gutenberg will generate a CSS that merges core, themes, and user preferences into one.
How this works?
Let's take an existing pattern such as "Hero Two Columns" and try to convert it to "global styles format":
Via
theme.json
, themes would register the design tokens they want for blocks:Note the use of CSS variables to set the values of the properties. These come from the color and font-size presets that the theme has defined in
functions.php
. They'll come from thetheme.json
at some point, but at the moment we pull them from the existing presets #22076The output will be:
Note that, at the moment, the mapping from keys to selectors (
core
=>:root
,core/paragraph
=>p
, etc) is hardcoded inglobal-styles.php
. This needs evolving in a different PR and be taken from the block registry.How to test
functions.php
and modify the preset values. As an example, copy the these ones for the color palette:You should see instead:
theme.json
. For example, add a text-shadow to the h1:#global-styles-inline-css
embedded stylesheet doesn't contain any text-shadow property.