-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Section styles: consolidate variation name #62550
Section styles: consolidate variation name #62550
Conversation
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/styles/block-style-variation-with-slug.json ❔ lib/block-supports/block-style-variations.php ❔ lib/class-wp-theme-json-gutenberg.php ❔ phpunit/block-supports/block-style-variations-test.php ❔ phpunit/class-wp-theme-json-resolver-test.php |
@@ -455,7 +455,7 @@ function gutenberg_register_block_style_variations_from_theme_json_data( $variat | |||
* Block style variations read in via standalone theme.json partials | |||
* need to have their name set to the kebab case version of their title. | |||
*/ | |||
$variation_name = $have_named_variations ? $key : _wp_to_kebab_case( $variation['title'] ); | |||
$variation_name = $have_named_variations ? $key : ( $variation['slug'] ?? _wp_to_kebab_case( $variation['title'] ) ); |
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.
For most scenarios, just kebab-casing the title
will be enough and that's a great devexp we may want to keep. The slug
(could also use name
) is a way to make it work for any scenario.
41808df
to
c0cca63
Compare
The resolution of block style variations from partials needed to be tweaked in the same way the registration was so their key is consistent. I pushed a tweak in ebf2162 I'll give this a more thorough review shortly. Then create a backport PR, update the backport changelog etc. |
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.
To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
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.
Thanks for putting this together @oandregal 👍
This tests well for me resolving both the "duplicate" style option and variation merging issues.
I've created a backport for the PHP changes and added a backport changelog entry to this PR.
Before we merge this one, we might need to update a couple of tests for the block style variations support and theme.json resolver. I'll sort those out on WordPress/wordpress-develop#6824 and then bring those back across to this PR. |
The unit test updates are pretty basic but cover:
|
It looks like tests have been failing on trunk since this merged and these changes look suspect. @aaronrobertshaw @oandregal have you noticed the test failures? Can you confirm whether they're related or not?
|
The error does appear related, we'll look into it 👀 |
Co-authored-by: oandregal <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: oandregal <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]>
Co-authored-by: oandregal <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]>
I just cherry-picked this PR to the wp/6.6-beta-3 branch to get it included in the next release: 7cfc6c5 |
Co-authored-by: oandregal <[email protected]> Co-authored-by: aaronrobertshaw <[email protected]>
What?
This PR enables theme.json partials to provide a different slug than the generated kebab-case to follow what any other registration mechanism (
block.json
,register_style_variation
) does.Why?
For theme.json partials to work like any other registration mechanism, and to enable them to override variations with names that don't follow the kebab-case convention.
The existing mechanisms to register block style variations require a
name
and alabel
, see docs. Thename
is used to generate the associated class (e.g.:is-style-NAME
) and we don't do any processing of it — all the following are correct:dark
is-style-dark
Dark
is-style-Dark
PitchDark
is-style-PitchDark
pitch-dark
is-style-pitch-dark
Before this PR, the
title
from the theme.json partials was preprocessed to generate a kebab-cased variation name & class:dark
dark
is-style-dark
Dark
dark
is-style-dark
PitchDark
pitch-dark
is-style-pitch-dark
pitch-dark
pitch-dark
is-style-pitch-dark
After this PR, the theme.json partials can contain an optional
slug
(alternative:name
?) key to be used in registering the variation and generating its class. This enables the theme.json partials to work with any variation name or class that wasn't kebab-case:dark
dark
is-style-dark
Dark
Dark
Dark
is-style-Dark
PitchDark
PitchDark
PitchDark
is-style-PitchDark
pitch-dark
pitch-dark
is-style-pitch-dark
Note the
slug
is optional and most of the cases will just lettitle
be kebab-cased.How?
Allow
slug
to be provided in theme.json partials:slug
if present, otherwise kebab-case thetitle
a602aaeTesting Instructions
Register a variation with a non-kebab-cased name. For example, paste the following in the
functions.php
of TT4:styles/partial.json
with the following contents:partial.json
to include aslug
that matches the variation name provided byregister_block_style
call: