Theme.json: Allow shared block style variations via separate theme.json files #57787
Conversation
aaronrobertshaw
added
[Type] Experimental
|
This pull request has changed or added PHP files. Please confirm whether these changes need to be synced to WordPress Core, and therefore featured in the next release of WordPress. If so, it is recommended to create a new Trac ticket and submit a pull request to the WordPress Core Github repository soon after this pull request is merged. If you're unsure, you can always ask for help in the #core-editor channel in WordPress Slack. Thank you! ❤️ View changed files❔ phpunit/data/themedir1/block-theme-child-with-block-style-variations/block-styles/block-style-variation-a.json ❔ phpunit/data/themedir1/block-theme-child-with-block-style-variations/style.css ❔ phpunit/data/themedir1/block-theme-child-with-block-style-variations/theme.json ❔ phpunit/data/themedir1/block-theme/block-styles/block-style-variation-a.json ❔ phpunit/data/themedir1/block-theme/block-styles/block-style-variation-b.json ❔ lib/class-wp-theme-json-gutenberg.php ❔ lib/class-wp-theme-json-resolver-gutenberg.php ❔ phpunit/class-wp-theme-json-resolver-test.php |
|
Size Change: 0 B Total Size: 1.69 MB ℹ️ View Unchanged
|
|
FWIW I'm currently looking at simplifying this approach so that the theme.json resolver will absorb referenced variations into the theme.json data avoiding the need to merge the referenced variation with user changes in multiple locations. This approach has been used also in #57908. |
|
Why it's |
|
Also, this could also be separate files entirely as well. an alternative theme.json file is just a style variation IMO. So maybe we don't need to introduce a new key within theme.json at all and just reference another file? I guess the question becomes how to reference a "saved variation" (id) |
The thinking here was that these are block style variations and living under I don't feel strongly about
This would make extending the block style variations to support their own specific settings much neater too. I like that it wouldn't introduce a new property to theme.json for the variations. We'll still be adding the nested In #57908, I looked at using an array property within a variation definition to specify which block types were eligible to use it. It saves needing individual blocks to all reference the variation by ID or whatever but it does come at the cost of being a new theme.json property again whether that's under Do you have any thoughts on that? In the meantime, I'll get this PR updated and give the separate files for block style variations a run. |
If the variations are on separate files, we could have a top level "supportedBlockTypes" property added there as well. |
aaronrobertshaw
force-pushed
the
try/referencing-variations-in-theme-json
branch
from
42b805f
to
c66f888
Compare
aaronrobertshaw
changed the title
There was a problem hiding this comment.
@youknowriad I've updated this PR to leverage standalone files for specifying block style variations as discussed above.
There's a couple of thing I'd like to get your thoughts on with the latest approach.
- Is there a better place for these block style variations to live than under a
block-stylesdirectory in a theme? - Should we be automatically registering block style variations defined in these files?
| // Automatically register the block style variation if it | ||
| // hasn't been already. | ||
| $registered_styles = $registry->get_registered_styles_for_block( $block_type ); | ||
| if ( ! array_key_exists( $variation_slug, $registered_styles ) ) { | ||
| gutenberg_register_block_style( | ||
| $block_type, | ||
| array( | ||
| 'name' => $variation_slug, | ||
| 'label' => $variation['title'], | ||
| ) | ||
| ); | ||
| } |
There was a problem hiding this comment.
Should block style variations defined within these standalone files be registered for blocks automatically?
Child themes wouldn't be able to deregister block style variations coming from standalone files via unregister_block_style but they could override the variation setting empty styles or supportedBlockTypes properties to avoid the automatic registration and omit that variation.
There was a problem hiding this comment.
-
I would prefer that, yes.
-
This is currently how child themes need to override (remove) theme style variations, so it makes sense.
Yes it is a little limiting, but it is already documented that "The function unregister_block_style only unregisters styles that were registered on the server using register_block_style" so there would be no change.
|
|
||
| // Resolve shared block style variations that were bundled in the | ||
| // theme via standalone theme.json files. | ||
| $shared_block_style_variations = static::get_style_variations( '/block-styles' ); |
There was a problem hiding this comment.
For the moment, the shared block style variations are being looked for in a block-styles directory within the theme. As with theme style variations under styles, this takes into account child theme variations overriding a parent's definition.
Is there a better location to store these block style variations? It seemed keeping them separate from theme style variations made sense to limit further updates to ensure block style variations weren't accidentally included with theme styles.
There was a problem hiding this comment.
I think it needs to be a different directory than the existing styles, but the naming is trickier.
|
Thank you for working on this, this is very exciting. Is there also a way to set the variation as the default? Let me try to describe my use case: |
There was a problem hiding this comment.
Oh, fascinating idea! I haven't taken a close look yet, but just had a couple of immediate thoughts / questions about moving to separate files:
- If block style variations are stored in separate files instead of within
theme.json, where would changes to block style variations be stored in user styles when (eventually) it's possible for these shared variations to be edited in the UI? Would that be in the main global styles entity, or a separate one? If it winds up being merged into the main entity, then I'm wondering if it'd be worth making it possible for the shared variation to be stored in a separatetheme.jsonfile or as a separate key in the maintheme.jsonfile, to offer a bit more flexibility? - Will the separate
theme.jsonfiles for style variations ever use thesettingsobject? (Not a blocker, but just curious since it currently looks like they're effectively full theme.json files, just with asupportedBlockTypesscope). - Will performance need to be taken into account? In core, there's some caching done on loading core
block.jsonfiles so that they all get loaded from a single PHP file here instead of having to grab each individual file. If a theme were to have many style variations (i.e. a dozen or more), would this be a performance concern?
All up, I think separate files makes for a much more readable approach, and also would make it easier for folks to manually edit / copy from one theme to another, which could be really handy. If supporting multiple files, though, my main thought is whether it'd still be worthwhile to support it as a key in the main theme.json also 🤔
|
I'm glad you're liking the potential here @carolinan 👍
At this stage, no, there isn't a means by which you could define one of these block style variation files as the default block styles. Once the dust settles on extending block style variations, this could easily be a follow-up. My initial thoughts jumped to being able to add a top-level property to flag that instead of a block style variation the contents of the given file gets merged into the theme.json as if they were normal block styles. Or perhaps, along with the The merging should be straightforward and much the same as this PR does for the style variations. I'm just not sold that we can keep adding more and more top-level properties. Perhaps we need a single top-level property that can be an object defining these sorts of things e.g. supported block types, which block types the partial theme.json represents the default block styles etc. Given the short time to WP 6.5 and the slim chance this all has to make it, I don't think I can expand the scope much here. |
|
Great feedback as always @andrewserong, thank you 🙇
Credit here is all @youknowriad's
Because the theme.json resolver merges these block style variations into the theme.json data as if they were defined on each block type by the theme's theme.json, the UI and editing/saving of block style variations functions as per normal.
It's the main global styles entity, primarily, as current core block style variations already work this way. Also, back on the point about the styles being "resolved" by the theme.json resolver for each block type, this means that each block type has its own copy effectively of the shared variation. There were a few reasons that pushed me in that direction but the most important being to reduce confusion. At the moment, if a user is editing a block style variation within Global Styles, they aren't going to know if it has come from a shared variation created by the theme's author. So there is no pushing of user changes back to the source of the shared variation.
I think this might not offer the expected flexibility particularly if/when these shared variations end up coming with their own settings e.g. palettes. Also, I'm not sure there's a great appetite to introduce many new properties within theme.json. For example, in the early explorations into section styling a There's probably something to be said for having a single clear means of implementing these shared variations too. Not to mention the theme style variations do not offer a separate key in the main theme.json file.
That's the plan eventually. For an initial MVP though, block style variations have historically only been styles, so will remain limited to that. Settings The ability to define custom settings is also something I considered with the alternate approach to extending block style variations that creates a custom per-variation-application stylesheet (#57908). With that approach, it wouldn't be a great stretch to grab the settings from the block style variation and use those or merge them with the main theme.json settings before generating the stylesheet.
Definitely.
Honestly, I'm not certain. Adding a little caching here wouldn't hurt in case the theme.json resolver is for some reason being requested to merge theme data repeatedly. I'd like to move doing that to a follow-up, if possible, to keep reviewing these block style variation PRs simpler.
Or to be supplied alongside patterns. |
|
Thanks for laying all that out @aaronrobertshaw! Those answers all point to separate files being desirable to me, with minimal drawbacks, and like you mention, caching can be something to add further down the track if needed. Also, that there's a use case for including |
Related
What?
This PR makes the registration and definition of block style variations easier.
Shared block style variation definitions can now be created as standalone files within a theme's
block-stylesdirectoryBlock style variations defined in this way will be automatically registered and merged into the theme's theme.json data under each block identified in a new top-level theme.json property called
supportedBlockTypes.An example theme.json file for a block style variation could be as simple as:
Why?
With the extended functionality of block style variations, they will hopefully get further use. Helping theme authors cut down on repetitive block style variation definitions will further increase their adoption.
How?
supportedBlockTypes, allowing a block style variation to define for which blocks can be usedblock-stylesdirectory within the themeAdd and updates theme.json resolver tests to cover new functionality
Testing Instructions
block-stylesdirectory (see snippets below or test theme for examples)Examples