-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Try absorbing padding for cover #23032
Conversation
Size Change: -13 B (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
"core/cover": { | ||
"styles": { | ||
"spacing": { | ||
"padding": 16 |
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.
This is how we'd absorb the existing CSS properties of the blocks. A few thoughts on this:
First, I think I'd wait post-5.5 to do this kind of change, so we have a full cycle to test in the plugin, as this requires open-up theme.json to production. Related conversation: for the link color feature to work, we need to expose the presets declared by the theme via add_theme_support
as CSS variables. We may be able to do this by only shipping to WordPress core the function that transforms the theme support declarations into CSS variables, without the need of shipping theme.json just yet. I thought I'd connect this two convos for perspective.
A second point to talk about is that, ideally, we can reference this sort of variables ($grid-unit-20
) via theme.json. We can also leave as it is, hardcoded values, but I think it'll be easier if we can use "named variables". Two ways we can do this are:
- We add the grid unit values as if they were any other presets (font-size, color) in theme.json, and theme authors will reference them as they do with any other preset:
{
"global": {
"presets": {
"padding": [
{
"slug": "unit",
"value": 8
},
{
"slug": "unit-20"
"value": 16
}
]
}
},
"core/cover": {
"styles": {
"spacing": {
"padding": "var(--wp--preset--padding--unit-20)"
}
}
}
}
- We create a mechanism to resolve the variable at render time. We need to mark the values to be resolved as well as having those variables available somewhere. Let's pretend that all values that are enclosed by
<>
should be resolved to the variable and that we expose them via avariables
subkey of the global scope (I'm not attached to this specific format). Theme authors could then do:
{
"global": {
"variables": {
"grid-unit": 8,
"grid-unit-20": 16
}
},
"core/cover": {
"styles": {
"spacing": {
"padding": "<global.variables.grid-unit-20>"
}
}
}
}
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.
We create a mechanism to resolve the variable at render time.
Is the intent to ensure compat for browsers with no var()
support? Or is it something else?
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.
That's something that crossed my mind, yes, although we're already using CSS vars in other places (presets), so we need to provide fallbacks for this sort of situation anyway.
One advantage the second approach has it that it doesn't pollute the global space with all the variables that are present in the base-styles. However, I don't know how much a problem that would be in practice. On the other hand, the second approach requires a specific syntax, which I consider a disadvantage. I like how the first approach requires fewer concepts to learn: leaf values just use regular CSS syntax.
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.
Yes, it's not an easy decision.
it doesn't pollute the global space with all the variables that are present in the base-styles.
Can this be mitigated by properly namespacing them, even if they are globally available?
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.
More brain-dumping: we'll need to expose the theme.json via some REST endpoint for mobile to be able to use/expose this data in the apps. One consideration we should make is how to balance theme authors' needs (use already-known CSS syntax at the leaf nodes) vs mobile code's needs (use parseable syntax at the leaf nodes).
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.
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.
not sure how to evaluate the css functions on mobile side
Can't say I have good insight on the matter, but I wonder, what CSS functions are we perhaps talking about? Talking about the var()
specifically or others too?
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.
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 prefer the usage of CSS syntax "var(--wp--preset--padding--unit-20)". We would need to parse things anyway no matter if it is "var(--wp--preset--padding--unit-20)" or "<global.variables.grid-unit-20>". And in that case, I would prefer if we used what developers already know instead of needing to specify a new syntax and a parser for it.
So I guess things could look like this:
{
"global": {
"variables": {
"grid-unit": 8,
"grid-unit-20": 16
}
},
"core/cover": {
"styles": {
"spacing": {
"padding": "var(--wp--global--variables--grid-unit-20)"
}
}
}
}
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 wouldn't worry if it was just about var()
actually, as you said we need to parse things anyway. But the possibility of using any kind of css function(like calc()
) is the main blocker for native mobile. Convo going on here: #24165
We'll be starting to actively work on GSS for native mobile in a couple of weeks so hopefully we'll have more progress on that issue. ✌️
Going to close this for now. We can always re-open. While I like the idea of absorbing more of the block's CSS into the GS engine, this doesn't seem the direction to go. In a conversation with @youknowriad he argued that the styles of the block should live with the block. I agree with that. I'm not sure what form should it take this "absorbing the block's css into the GS engine" idea: can we set block attributes via theme.json? do we generate a dynamic rule for the block depending on theme.json values (not part of the block's style.css)? Perhaps we can revisit once theme.json lands into core. |
Follow-up on #21492
Work In Progress
This is an in-progress experiment to absorb style declarations that were previously included in the block's CSS via theme.json. By managing the block's CSS we can consolidate the style needs of different origins (block defaults, theme, and user), ship less CSS to the browser (more performant), and adapt the CSS to the different contexts (editor, front) without any theme intervention.