Add custom CSS support to elements and block style variations #49396
Conversation
This comment was marked as resolved.
This comment was marked as resolved.
|
Size Change: -21.5 kB (-2%) Total Size: 1.39 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 25db93f. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5090482727
|
There was a problem hiding this comment.
Thanks for working on this @carolinan !
Going to Site Editor, Styles, color, buttons, text, and changing the color should NOT change the text color: The CSS should override this: To do: Add elements Custom CSS to the global CSS field?
I can't reproduce this on Empty theme but it might be due to stylesheet order as both the declarations have the exact same specificity:
(Having an element selector target the inner class of a specific block like .wp-block-button__link feels a bit off, though I suspect the reason is that our "buttons" aren't all actual button elements)
Were you planning on adding custom CSS fields for block style variations in the UI as a follow-up? If we have the possibility to set them from theme.json it would make sense for the UI to reflect that too.
|
Changing Styles> color> buttons> text should not override the theme.json Elements > button > CSS. That the CSS overrides this setting, is one of the reasons why we need the CSS field in the UI, so that users can remove the CSS. I can work on the UI, yes, but I'd need some UX decisions first; perhaps the elements CSS should be in the global CSS panel. |
This comment was marked as outdated.
This comment was marked as outdated.
carolinan
changed the title
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
There was a problem hiding this comment.
Thanks for persisting with this, it's working pretty well! Just a couple comments below and a thought:
I don't love how adding custom CSS to the default variation works, because there's no way of overriding it from the UI. For any non-default variation, the added CSS appears in the custom CSS box for that variation, and can be removed, but for the default variation the custom CSS can't be overridden from the block's custom CSS field.
I wonder if we could either wire up the custom CSS added in theme.json for the default style to appear (and be overridable from) the block's custom CSS, or if perhaps we should disable adding styles for the default variation in theme.json altogether?
| // We need to add a ':not' selector to the selector to target the default variation. | ||
| $not_selector = ''; | ||
| foreach ( static::$blocks_metadata[ $name ]['styleVariations'] as $not_style_variation_name => $node ) { | ||
| $not_selector .= ':not(.is-style-' . $not_style_variation_name . ')'; |
There was a problem hiding this comment.
We could simplify this and remove the loop altogether by making the rule :not([class*="is-style-"])
| if ( variation[ 1 ].css ) { | ||
| const variationSelector = | ||
| blockSelectors[ blockType.name ] | ||
| .styleVariationSelectors[ variation[ 0 ] ]; |
There was a problem hiding this comment.
We should probably add a version of the "not" selector in here, otherwise styles applied to the default variation won't appear in the editor.
|
I don't understand what "wire up CSS" means 😆 |
|
If it wasn't for the separator block default variation, I think the default variation CSS could be removed, yes. |
Sorry! That wasn't very clear. I meant would it be possible connect the "default" style variation with the block's custom CSS input (the one outside of the variations), so that any custom CSS added to the default variation in theme.json appears in the input and can be overridden from there?
Hmm, if that's the main issue, I think we could try a different solution for it. Maybe deprecating the "wide" variation if it's redundant? Or making it the default and adding a "narrow" variation instead? It doesn't feel right to structure the behaviour of the custom CSS API around a styling issue with a particular block 😅 |
|
I would like the wide to be deprecated and only the dots kept as a variation. But the last time a style variation was deprecated it caused problems for developers because they had added their own CSS to the core styles, reusing the class names instead of creating their own. Now, that would not only affect developers but also affect users directly, because they can customize the variations too., right? |
I'm thinking we'd have to deprecate in a way that preserves the classnames for existing uses of the style variation, so as not to break any websites out there.
Once this PR is merged we will have that problem, yeah. I guess we'd have to still show any variations with custom styles attached in the UI so they could be reset/removed. |
|
If we go with "disable adding styles for the default variation in theme.json", how do we make that known for people editing theme.json? I don't think it is possible to use the schema to warn that it is invalid? |
Yeah, that's what makes it hard, the default style can have any name. Plus it could be changed or removed via a filter. Perhaps it's better to take the other approach, and try to at least make sure any additions to the default variation are editable. |
|
When a style is selected and then reverted back to the default, the class is Well, it can't be styled there unless we change that. :) |
True 😄 and there doesn't seem to otherwise be a good way forward here. I'm mostly concerned that theme authors can add "default" styles that are impossible to override in the UI. |
|
What I am trying to do with the last 4 commits is ignore anything that is added to theme.json variations.default.css: Styling the default variation via theme.json |
There was a problem hiding this comment.
I think we might have dug ourselves into a really deep hole with the current implementation of default style variations 😅 I've been thinking about how to untangle this and there's no obvious way forward.
isDefault is a property that can be added to any style variation, and once the ability to set new variations in theme.json (as described in #49827) ships, a theme will be able to add and set as default a new variation for any block.
The problem is that there's a mismatch between how this works and the UI in the block settings: the variation set to isDefault is always named "Default" in the UI, and also, the "Default" variation appears selected if a block has no variation explicitly set (I'm equating a variation being "set" here with its classname being present on the block, which I think is a reasonable assumption to make)
This could potentially be less confusing if we always explicitly set the default variation for new blocks when they have a default variation configured. That would be something to explore separately from this PR though!
I guess the approach of adding the default style selector to all the non-variation specific block custom CSS will solve the problem in the short term, but we have to make sure to add the correct classname.
Another thing that would be nice is if we could get any styles added to the default variation from theme.json to show in value for custom CSS here, so we could disable them by emptying the input if needed. I'm not sure how doable that is though, because that input is already controlling the non-variation block custom CSS.
| // Only use the class for the default style variation if the block has style variations. | ||
| if ( isset( static::$blocks_metadata[ $name ]['styleVariations'] ) ) { | ||
| // Add the block selector and the .is-style-default style variation class. | ||
| $selector = static::$blocks_metadata[ $name ]['selector'] . ', ' . static::$blocks_metadata[ $name ]['styleVariations']['default']; |
There was a problem hiding this comment.
The default variation isn't always named "default". This won't work for the Button block, for example, and we also don't have any way of enforcing the naming for third party blocks. The only way to make sure a style variation is default is by checking for the "isDefault" property.
|
Could we remove the "default" selection from the interface and replace it with a "reset" "no variation" or "unstyled" option? Or, not indicate that one of these options has to be selected. And then in the interface, list all variations by their actual name and find a way to indicate which variation is set as the default: |
|
I mean that we could try to separate unstyled from default and not consider this to be a valid variation name: |
I like these ideas, but it might be good to get design feedback on them. When adding style variations to the site editor, I initially had all the variations displaying and removed them after subsequent discussion, as it was found confusing. Perhaps it's a good time to revisit that discussion cc @WordPress/gutenberg-design |
|
Hey guys! I appreciate very much your efforts on Custom CSS, since the Global Custom CSS has been implemented. If I was god, I would prioritize this request to be approved. Sometimes it's good to get an outside look at our work for feedback, and I want to warn you that I read through the entire current discussion, and when I finished it, it sounded more like you were discussing deprecating styles instead of implementing custom CSS to elements and block style variations. 😂 The discussion ""got lost"" along the way, in the sense that it considered several different issues and not always blockers for the current PR. I suggest that you define the next steps that need to be taken, and to what extent one step really is making the other step impossible. I'm sure your caution will remain. The hard part is over, with the implementation proposed by @carolinan which is already done. |
What?
Adds custom CSS support to theme.json
styles.elementsandstyles.blocks.blockname.variations.Outputs the CSS from theme.json in the editors and the front.
-The theme.json schema is not updated in this PR because it already allowed CSS for these sections.
Why?
This is an enhancement that increases the flexibility of CSS in theme.json:
-Allows adding CSS to all headings at once (and other elements)
-Allows adding CSS to style variations. Example: Change the width of the separator "wide" block style variation.
Testing Instructions
Activate Twenty Twenty-Three
In theme.json, add some custom CSS to a variation and an element. Example:
Do not forget to reset styles between the tests.
Body:
Elements:
border: none;into the CSS field should remove the border from the heading block.border: none;into the CSS field should remove the border from the post title block.Add a button without a style variation:
color: purple;into the CSS field should change the text color.Variations
Separator:
The custom CSS field for the block should contain
border:10px solid purple; max-width:100pxnot
border:10px solid blueThe following entry in theme.json should be ignored:
Button: Add a button with the outline style variation.
Screenshots or screencast