Prototype: merge block CSS with theme.json styles
#34180
Conversation
oandregal
added
[Status] In Progress
|
Size Change: +64 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
|
I like this principle. It does open up a bit of challenges, though, perhaps best illustrated here:
The most important properties of the button are extracted from the style and put into the JSON file for easy parsing, tweaking and disabling at a theme's whim. This is cool, and probably sufficient. But it's obviously also not all of it, as some styles do not have JSON counterparts. It's probably fine as in most cases these are properties that are structurally sensible, such as the Then there are the curious bits, for example those padding values,
The padding challenge remains, though. If we were to absorb those styles, the outline would still need to have a |
|
This padding issue has been a headache for us. We've had a use case that complicates it even more, and it's a very common one for themes to implement, that is related to not having any hover states built in on the block. The case is simple: a button block that on hover looks like the outline variation. For a theme to do that it has to build in this padding trickery and then remove it when the hover happens, because then there's a border that didn't exist before. Ideally, I would like the solid button style to have a border width set to the same color as the button background but I'm guessing that won't work well for backward compatibility, will it? @jasmussen your codepen is exactly what I would do in this case too, but the theme.json can actually define a border-width, how would that be handled then? /cc @pbking @jffng @scruffian |
|
I'm trying this with the empty theme and the padding still behaves as expected:
The reduced padding for the outline still applies, although the default padding is now part of |
|
Adding this to emptytheme's json: I get, on trunk:
And with this PR:
I don't know why border-radius is not picking up on trunk :S |
That's exactly the headache I was hoping to find a better solution to. My custom CSS solution is usually to use an inner box-shadow instead of the outset border — The alternate solutions I was pondering, adding an explicit height or using pseudo elements, have their own sets of downsides. |
I would say it's fine if it only happened when the user sets it, but not so much when it's the theme.json doing so. You are just passing the headache to the themers :D |
|
The solid/outlined button problem is a bit tangential to the conversation here, the principle of moving these styles over still feels valid to me. This was primarily an exploration of one aspect that will start to surface as we start to do this. It's probably a good thing, as we would've encountered this same issue with the button eventually anyway. |
|
Yeah, I believe these issues are inherent to the block but shouldn't be a blocker for this particular change |
|
I'm looking at the issue with border-radius Maggie shared. For the challenges of maintaining height: isn't the issue that the user can't modify the underlying values of the block style variations? If they could, when they change padding in the default state, it is also the user's responsibility to update it in the outline variation. Not a final solution but #33232 should get us moving. |
|
This is what happens: the outline variation uses the selectors The global styles rule for the button:
I can repro the same thing in |
|
As for the border-radius, I didn't see any report, so filed an issue at #34240 |
The challenge is that showing a solid background button next to a bordered outlined button with the same height, is remarkably harder to accomplish than you'd think. You'd think the solid button just has a background and no border, while the outlined version has a border and no background, right? Yes, but then the border is outset from the fill, causing the height differential, shown here with a 4px border:
The way we've done it so far is to apply
But Another option would be to apply the outline version using A third option is to also draw a black border on the solid button, so both are the same height. But then you'd have to change both the background and the border color in order to change the color of the solid button. Which might be okay, but wouldn't work very well with gradient color buttons. I found a third option here: https://codepen.io/joen/pen/yLXLEgW?editors=1100 — always drawing a transparent border on the solid button, that seems to get pretty close to accomplishing what we need. The challenge here would then be if you change the border width on the outline button, you'd also have to change the border width on the solid version, even if it was transparent. Not sure if that would be a good experience 🤔 @MaggieCabrera any thoughts on that last option above? ☝️ |
As I was reading what you wrote I was thinking about transparent borders. This solution helps accomplish the hover case (which ofc right now is not something we need to worry about in this context, but it's heavy on my mind since I've been struggling with that lately) where we would want the solid button to appear as the outline version while hovering without having to tinker with the padding. I like it very much from a themer's perspective. As for the user experience, I guess it's not ideal. It would make sense to me that border width of the button would stay consistent regardless of the variation we are editing, I don't see why the value would be different between them. |
It would certainly start out the same, but I imagine it would still make sense for the border width to be a per-button property. Of the options on deck, and since you seem mostly on board with the transparent border idea (thanks for looking), to me it seems like the best path forward so we can replace that padding calc-value. What do you think, @kjellr? I know we discussed this in the past. |
I like this too. This PR also highlights the need for a way to add comments to style configurations. The 9999px radius is a great example of this. IIRC this feedback was also recently relayed by Anne from the FSE testing group. |
mburridge
added
the
Needs Dev Note
|
Added the Needs Dev Note label in case this needs a dev note (either individual or as part of a "misc" dev note) for WP 6.1 release. |
|
We posted about this here: https://make.wordpress.org/core/2022/08/01/moving-core-block-styling-to-json/ |
|
Thanks @scruffian, might it be worth adding the |
|
Done, thanks |
mburridge
removed
the
Needs Dev Note
After block CSS was merged with `theme.json` styles in [WordPress/gutenberg#34180 Gutenberg PR #34180], this removed some existing, default styling for some elements, including buttons. This change re-adds the removed styles by enqueueing `classic.css` for classic themes. Merges [WordPress/gutenberg#44334 Gutenberg PR #44334] into trunk. Follow-up to [54257]. Props scruffian, oandregal, ramonopoly, aristath, andrewserong, get_dave, bernhard-reiter. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54358 602fd350-edb4-49c9-b593-d223f7449a82
After block CSS was merged with `theme.json` styles in [WordPress/gutenberg#34180 Gutenberg PR #34180], this removed some existing, default styling for some elements, including buttons. This change re-adds the removed styles by enqueueing `classic.css` for classic themes. Merges [WordPress/gutenberg#44334 Gutenberg PR #44334] into trunk. Follow-up to [54257]. Props scruffian, oandregal, ramonopoly, aristath, andrewserong, get_dave, bernhard-reiter. See #56467. Built from https://develop.svn.wordpress.org/trunk@54358 git-svn-id: http://core.svn.wordpress.org/trunk@53917 1a063a9b-81f0-0310-95a4-ce76da25c4cd
After block CSS was merged with `theme.json` styles in [WordPress/gutenberg#34180 Gutenberg PR #34180], this removed some existing, default styling for some elements, including buttons. This change re-adds the removed styles by enqueueing `classic.css` for classic themes. Merges [WordPress/gutenberg#44334 Gutenberg PR #44334] into trunk. Follow-up to [54257]. Props scruffian, oandregal, ramonopoly, aristath, andrewserong, get_dave, bernhard-reiter. See #56467. Built from https://develop.svn.wordpress.org/trunk@54358 git-svn-id: https://core.svn.wordpress.org/trunk@53917 1a063a9b-81f0-0310-95a4-ce76da25c4cd
|
Draft dev note: WhyPreviously blocks styles have be expressed in CSS. When themes want to override these styles using theme.json, it can be difficult to ensure that the specificity of the CSS that Global Styles generates overrides the default block CSS. This struggle has resulted in the use of some convoluted selectors in block CSS and also some issues where Global Styles settings weren't working as expected. WhatTo overcome this it is now possible to express styles for a block in JSON, using the same properties as a theme.json file. These styles are merged with the Global Styles settings, with theme and user style settings taking priority over block styles. This means that blocks which express their styles in JSON will no longer have to deal with specificity battles between their CSS and Global Styles. In addition, when themes and users override these rules then no extra CSS will be output, which has a performance benefit. HowBlocks can add an This will output the following CSS in the frontend: |
|
There is now #45198 to track the next steps for this experimental API. |
After block CSS was merged with `theme.json` styles in [WordPress/gutenberg#34180 Gutenberg PR #34180], this removed some existing, default styling for some elements, including buttons. This change re-adds the removed styles by enqueueing `classic.css` for classic themes. Merges [WordPress/gutenberg#44334 Gutenberg PR #44334] into trunk. Follow-up to [54257]. Props scruffian, oandregal, ramonopoly, aristath, andrewserong, get_dave, bernhard-reiter. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54358 602fd350-edb4-49c9-b593-d223f7449a82
Implements #33977
I'd like feedback on the direction. Should we want to pursue this, I'd improve the implementation later.
This PR experiments with moving parts of the CSS of the block to its
block.jsonso it can be used to be merged withtheme.jsonstyles. The result of the merging will be (later in the chain overwrites the previous):There're a few advantages to this approach:
theme.json).I've noticed that many of the block styles are attached to classes (has-background, style variations, etc) so I presume, initially, we won't see a big reduction of CSS. It'll be a good first step to absorb more later (e.g.: block variations).
How to test
global-styles-inline-cssembedded stylesheet in the frontend contains the following CSS coming from theblock.json:theme.jsonof the theme:global-styles-inline-cssembedded stylesheet in the frontend contains the following CSS (colors come from the theme now):