Theme.json: Add block support feature level selectors for blocks #42087
Conversation
aaronrobertshaw
added
[Type] Enhancement
|
Size Change: +196 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
aaronrobertshaw
force-pushed
the
add/feature-level-selectors-to-theme-json
branch
from
637f2f5
to
6d0592a
Compare
aaronrobertshaw
changed the title
Just checking, are we blocked from testing manually due to this loading sequence? |
Sorry for the confusion @ramonjd You can test manually either via:
I'll update the PR description and testing instructions. |
There was a problem hiding this comment.
This is testing pretty nicely for me @aaronrobertshaw. It's working well for me in global styles, and the individual block-level border overrides the global styles version correctly on the site front-end:
| Global styles | Individual block overriding global styles (this is a different gallery block to the previous screenshot) |
|---|---|
 |
 |
Overall, I quite like the idea of the feature, too — it slots in nicely with the existing __experimentalSelector and I like how it concatenates the root level selector with the feature-level one, too. While it is adding extra complexity to the global styles class, the changeset is a bit smaller than I'd imagined, so from my perspective, I think it's a worthwhile addition. Also the filter-based approach for ensuring a test block is available in the test environment sounds like a reasonable approach to me — it could be useful for other tests, too, having a test block around.
The one issue I ran into is that the border didn't appear to be displaying for me at the block level within the post editor, however, it renders correctly on the front end of the site: :
| Post editor | Site front end |
|---|---|
 |
 |
I'm testing with TwentyTwentyTwo theme and via checking out #31366, so I wasn't sure if it's more to do with that PR?
phpunit/class-wp-theme-json-test.php
Outdated
|
|
||
| $this->assertEquals( $expected, $theme_json->get_stylesheet() ); | ||
|
|
||
| unregister_block_type( 'test/test' ); |
There was a problem hiding this comment.
Tiny nit: Does this test still need to unregister the block? If so, should the unregister be moved to before the assertEquals so that if the test fails it doesn't potentially affect other tests?
There was a problem hiding this comment.
Honestly, I'd missed this when moving the block registration to the tests_add_filter. I did wonder though if unregistering the block limits unexpected results for other tests or creates more variability. I think I'd lean towards removing this.
|
Thanks for taking the time to review this one @andrewserong 👍
This one is due to WP core adding generic If I select a border style in the Post Editor the border displays correctly. Screen.Recording.2022-07-05.at.5.39.21.pm.mp4As you noted the issue is more to do with #31366 rather than this one so I don't think it's a blocker here. I'll address this in #31366, even if it is by adding |
| @@ -47,6 +47,15 @@ class WP_Theme_JSON_6_1 extends WP_Theme_JSON_6_0 { | |||
| 'caption' => 'wp-element-caption', | |||
| ); | |||
|
|
|||
| // List of block support features that can have their related styles | |||
| // generated under their own feature level selector rather than the block's. | |||
| const SUPPORT_FEATURES = array( | |||
There was a problem hiding this comment.
Probably a nitpick:
The comment is pretty clear, but I'm thinking about whether SUPPORT_FEATURE describes the role of the whitelist to folks digging about in the code.
Would BLOCK_SUPPORT_FEATURES or even BLOCK_SUPPORT_FEATURE_LEVEL_SELECTORS add anything?
| // to leverage existing `compute_style_properties` function. | ||
| $feature = array( $feature_name => $node[ $feature_name ] ); | ||
| // Generate the feature's declarations only. | ||
| $feature_declarations[ $feature_selector ] = static::compute_style_properties( $feature, $settings, null, $this->theme_json ); |
There was a problem hiding this comment.
I'm not sure what I'm seeing is correct, but I can't get multiple features to work at the same time, and I suspect it's because the features are either being unset or overwritten here.
So the border styles won't show when I have color/spacing/typography defined.
block.json supports example
"supports": {
"anchor": true,
"align": true,
"color": {
"__experimentalSkipSerialization": true,
"gradients": true,
"__experimentalDefaultControls": {
"background": true,
"text": true
},
"__experimentalSelector": "> table"
},
"typography": {
"fontSize": true,
"lineHeight": true,
"__experimentalFontStyle": true,
"__experimentalFontWeight": true,
"__experimentalLetterSpacing": true,
"__experimentalTextTransform": true,
"__experimentalDefaultControls": {
"fontSize": true
},
"__experimentalSelector": "> table"
},
"__experimentalBorder": {
"__experimentalSkipSerialization": true,
"color": true,
"style": true,
"width": true,
"__experimentalSelector": "> table",
"__experimentalDefaultControls": {
"color": true,
"style": true,
"width": true
}
},
"__experimentalSelector": ".wp-block-table",
"__experimentalStyle": {
"spacing": {
"margin": "0 0 1em 0"
}
}
},
theme.json styles
"styles": {
"blocks": {
"core/cover": {
"border": {
"color": "fuchsia",
"style": "dashed",
"width": "3px"
},
"color": {
"text": "fuchsia",
"background": "black"
},
"typography": {
"fontSize": "1px"
},
"spacing": {
"padding": "100px"
}
},
"core/table": {
"border": {
"color": "fuchsia",
"style": "dashed",
"width": "3px"
},
"color": {
"text": "fuchsia",
"background": "black"
},
"typography": {
"fontSize": "100px"
},
"spacing": {
"padding": "100px"
}
}
}
},
The only way I can get it to work is:
// Generate the feature's declarations only.
if ( isset( $feature_declarations[ $feature_selector ] ) ) {
$feature_declarations[ $feature_selector ] = array_merge( $feature_declarations[ $feature_selector ], static::compute_style_properties( $feature, $settings, null, $this->theme_json ) );
} else {
$feature_declarations[ $feature_selector ] = static::compute_style_properties( $feature, $settings, null, $this->theme_json );
}But it might an indicator of a deeper issue.
It could be related to the fact that I'm seeing duplicate rules in the HTML source? Here's what I'm seeing when testing the cover block:
<style id='global-styles-inline-css'> \* lots of styles here*\ .wp-block-cover > .wp-block-cover__inner-container > p{padding: 100px;} \* lots of styles here*\ .
</style>
<style id='wp-block-cover-inline-css'>
.wp-block-cover{font-size: 1px;}.wp-block-cover > .wp-block-cover__inner-container > p{padding: 100px;}
</style>There was a problem hiding this comment.
Thanks for catching this issue @ramonjd. I'd clearly been caught up testing different supports targeting different inner elements.
I've fixed this by leveraging your snippet, adding an explanatory inline comment, and expanding the unit tests to cover:
- two supports using a shared custom feature level selector e.g.
.inner - one other support using its own unique selector e.g.
.sub-heading - and, a further support that doesn't have a feature level selector specified.
But it might an indicator of a deeper issue.
It could be related to the fact that I'm seeing duplicate rules in the HTML source? Here's what I'm seeing when testing the cover block:
This looks to be unrelated to me. There was definitely a flaw in the previous logic generating the feature styles.
There was a problem hiding this comment.
There was a problem hiding this comment.
This is a nice change. And compact too!
With one rule, say, border, the selectors are output as expected in the frontend and site/block editors.
So I see .wp-block-table > table .some_selector, where .wp-block-table > table is the root __experimentalSelector and .some_selector is the feature selector.
I was wondering if there'd ever be a case for & like selectors, where we could conditionally apply only one feature's style to the container? Sounds a bit weird when I type it out, but was just curious.
With various feature rules added, I noticed that CSS rules are missing when I activate multiple features (color/border etc). See comment
|
Thanks for reviewing @ramonjd 👍
I did wonder about something similar as well. I'm not sure how valid the following hypothetical would be but might illustrate the potential for a future desire to modify the root It could be possible that we'd have a block that has a few Block Styles. Maybe we'd like to apply the block support styles only if a certain block style hasn't been selected. Such as;
In future iterations of these feature level selectors, I'd like to think we could extend it to perhaps check if the feature level selector starts with
Thanks for flagging this, I'll look into it. |
|
I had another thought (and it isn't at all a blocker for this PR), that when it comes to rendering block supports at the individual block level for server-rendered blocks, we might need to add a conditional that switches between inline styles (if no experimental selector is used) and a style block — so we'd potentially call the style engine differently based on whether or not the current feature uses the experimental selector? I imagine that'd be a bit neater than attempting to inject a Just thought I'd mention it as a potential to-do list item for the future, or in case it affects plans for server-rendered blocks like Search 🤔 (Apologies if this has been discussed already!) |
I see these selectors primarily being used when skipping the serialization of individual block styles. A means to offer control via both theme.json and Global Styles when the styles generated would otherwise be incorrectly targetted. If skipping serialization the block would have the styles manually generated and applied directly where it sees fit during its render callback. Could the choice between inline styles or a style block be made here avoiding an additional config flag? Do you think it makes sense that we only enable the feature level selectors if the block support also skips serialization?
It might do although I'm not sure if we'd then run into specificity issues. For all their downsides, the inline styles make it clear that the individual block's support styles will be taking precedence. |
Agreed! It's one of the things I like about the Image block PR — that it seems quite natural injecting the inline style at the correct level in the hierarchy in the JS implementation.
Yes, I think that's a good way to handle it at the individual block support level. It means that in the shorter-term, this is a feature for blocks that skip serialization, but it doesn't stop us from exploring options in the style engine further down the track, if we want to leverage the feature-level selector for outputting |
Sounds like a plan to me. Happy to tweak the approach as more blocks leverage feature-level selectors but for now it would be great to be able to land this to unblock the application of borders on images in #31366. I'll add a few extra reviewers and 🤞 that we can get this merged in the near future. |
Sounds great — I think this feature is looking really good, and the scope of this PR is nice and incremental, so opening up for wider review sounds like a plan to me, too! |
|
@aaronrobertshaw Nice to see someone else working on this! I did an exploration in a gist a while back for multiple selectors in the context of using them to to apply SVG filters (duotone) to the inner parts of a block since I'd like to stabilize a method of allowing blocks to choose multiple selectors and wrap I'm in transit to a meetup at the moment, so I'll do a proper code review when I arrive, but I figured it would be worth sharing my initial thoughts first. If the details of stabilizing that API are too far out of scope for here, feel free to comment on the gist or I can open up a proper discussion when I have a bit more time. |
aaronrobertshaw
force-pushed
the
add/feature-level-selectors-to-theme-json
branch
from
a9a9cf1
to
3740ddb
Compare
|
I've rebased this and resolved the conflicts after #40875 landed. The PR is still working for me and the unit tests pass. Any extra testing is still welcome of course. @jorgefilipecosta do you have any thoughts or objections to the approach taken in this PR? If not, I'll look to merge this soon. |
There was a problem hiding this comment.
I've taken this for another spin and, despite trying, I can't break it 😄
I've added __experimentalSelector values to different styles in block.json and then styled blocks via theme.json.
The correct CSS is generated even when I mix selectors and use multiple styles.
Great stuff.
This PR seems to have been very well discussed and passed by many reviewers so I trust your judgment here. |
|
Thanks everyone for the reviews, testing and feedback. Appreciate it 👍 |
|
Do I understand correctly that I do not make any changes or add any new keys to theme.json? |
|
Hi @carolinan 👋 I'm not sure I follow your questions exactly, so my apologies if I miss the mark answering them here.
No new keys or changes to your theme.json are required. A block support feature's values are configured the same way whether they are applied to the block's root wrapper or a specific selector for that feature. When a theme.json configuration is being processed, CSS declarations are created for each block. These are then converted to a ruleset for a given selector. Before this PR, the block could define its root level selector via Now a block can define the selector for a specific block support feature, e.g.
If I understand correctly, you are concerned a block might suddenly change the selector configured in its block.json? The same could happen with the root level If a block is changing its markup such that it needs a new selector to apply feature-level styles, it would likely already be needing a deprecation or to maintain the old selector to maintain stylings on deprecated blocks. The Hope that helps clarify things 🤞 |
|
Yes thank you. It doesn't literally "Add a "feature level selector" to theme.json ". |
Correct. Apologies if the PR title is misleading.
It adds processing and support for feature level selectors to the theme.json class ( e.g. |
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54159 602fd350-edb4-49c9-b593-d223f7449a82
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. Built from https://develop.svn.wordpress.org/trunk@54159 git-svn-id: http://core.svn.wordpress.org/trunk@53718 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. Built from https://develop.svn.wordpress.org/trunk@54159 git-svn-id: https://core.svn.wordpress.org/trunk@53718 1a063a9b-81f0-0310-95a4-ce76da25c4cd
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. Built from https://develop.svn.wordpress.org/trunk@54159
This change backports the following changes from Gutenberg repository: - [WordPress/gutenberg#40875 gutenberg#40875] Layout: Use semantic classnames, centralize layout definitions, reduce duplication, and fix blockGap in theme.json - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42087 gutenberg#42087] Theme.json: Add block support feature level selectors for blocks gutenberg#42087 - [WordPress/gutenberg#43792 gutenberg#43792] Global Styles: Split root layout rules into a different function gutenberg#43792 - [WordPress/gutenberg#42544 gutenberg#42544] Layout: Add a disable-layout-styles theme supports flag to opt out of all layout styles gutenberg#42544 - [WordPress/gutenberg#42665 gutenberg#42665] Layout: Reduce specificity of fallback blockGap styles gutenberg#42665 - [WordPress/gutenberg#42085 gutenberg#42085] Core CSS support for root padding and alignfull blocks gutenberg#42085 Note that it doesn't entirely port over PR40875 — the remaining PHP changes for that PR will be explored in a separate PR targeting `layout.php`. Props andrewserong, aaronrobertshaw, isabel_brison. See #56467. git-svn-id: https://develop.svn.wordpress.org/trunk@54159 602fd350-edb4-49c9-b593-d223f7449a82
Related: #31366
What?
Allows custom CSS selectors for individual block support features e.g. for border support. This means styles generated for individual block support features can be applied to different elements within a block e.g the block wrapper or another element that can't be targeted via the elements API.
Why?
While attempting to add border support to the image block we hit the issue where we need to skip border support serialization so we can apply the borders only to the inner
imgelement.If we add the
imgelement to the Elements API, we end up creating a disjoint between the block editor and theme.json/global styles. The problem lies in the fact that we can't style theimgelements via Global Styles. This leads us to a situation where a theme author could style all image block'simgelements but if a user wished to tweak that styling they would have to edit every individual image.Adding a "feature level selector" to theme.json and global styles means that we can keep the usual theme.json shape (no elements API use) and also keep using the Global Styles UI as per normal. The difference is that the generated styles target new custom CSS selectors.
How?
__experimentalSelectorprop within each individual block support__experimentalSelectorvalues to generate scoped CSS selectors specific to that block supportNote: PHP unit tests were a little tricky to implement given that WordPress is loaded prior to the tests running meaning that WordPress' theme.json generates block metadata before our single, feature-level selector test could register a block. As a means of still having a programmatic test for the new selectors, I've added a
tests_add_filter()call to the bootstrap to get a suitable block registered in time for testing. Once the Image block adopts these new selectors via #31366 we could remove this filter and test via the image block.We can also remove the
tests_add_filterprior to merging this PR if we wish to keep things cleaner in the interim.Testing Instructions
npm run test-unit-phpnpm run test-unit packages/edit-site/src/components/global-styles/test/use-global-styles-output.jsManual Testing
The generated styles can be manually tested via #31366 which has been based on this PR.
.wp-block-image img, .wp-block-image .wp-block-image__crop-area{ ... "styles": { "blocks": { "core/image": { "border": { "radius": "9999px", "color": "fuchsia", "style": "dashed", "width": "3px" }, } } } }Alternatively;
__experimentalSelectorat the feature level (not rootsupportsconfig)