Skip to content
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

Merged

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Jun 13, 2024

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 a label, see docs. The name 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:

Variation Name Generated class
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:

Partial's Title Variation Name Generated 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:

Partial's Title Partial's slug Variation Name Generated class
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 let title be kebab-cased.

How?

Allow slug to be provided in theme.json partials:

  • Use slug if present, otherwise kebab-case the title a602aae
  • Update schemas c0cca63

Testing Instructions

Register a variation with a non-kebab-cased name. For example, paste the following in the functions.php of TT4:

		register_block_style(
			'core/group',
			array(
				'name'         => 'MyVariation',
				'label'        => __( 'My variation', 'twentytwentyfour' ),
				'inline_style' => '
				.is-style-MyVariation {
					background-color: red;
				}',
			)
		);
  • Go to the editor and verify there is a variation for the group block called "My variation" and that applying it would set the background to red.
  • Create the file styles/partial.json with the following contents:
{
    "$schema": "https://schemas.wp.org/trunk/theme.json",
    "version": 2,
    "title": "My variation",
    "blockTypes": [ "core/group" ],
    "styles": {
            "color": {
                    "background": "blue",
                    "text": "white"
            }
    }
}
  • Go to the editor. You'll see two variations registered.
Captura de ecrã 2024-06-13, às 23 15 33
  • Now, update the partial.json to include a slug that matches the variation name provided by register_block_style call:
{
    "$schema": "https://schemas.wp.org/trunk/theme.json",
    "version": 2,
    "title": "My variation",
    "slug": "MyVariation",
    "blockTypes": [ "core/group" ],
    "styles": {
            "color": {
                    "background": "blue",
                    "text": "white"
            }
    }
}
  • Go to the editor and verify that there is only one variation again:
Captura de ecrã 2024-06-13, às 23 12 48
  • Confirm that the partial's style gets applied correctly as it takes precedence over the PHP block style registration.
@oandregal oandregal self-assigned this Jun 13, 2024
@oandregal oandregal added [Type] Bug An existing feature does not function as intended Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json Backported to WP Core Pull request that has been successfully merged into WP Core labels Jun 13, 2024
Copy link

github-actions bot commented Jun 13, 2024

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
@oandregal oandregal changed the title Section styles: support for i18n & consolidate name label Jun 13, 2024
@@ -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'] ) );
Copy link
Member Author

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.

@oandregal oandregal changed the title Section styles: support for i18n & consolidate variation name mechanism Jun 13, 2024
@oandregal oandregal force-pushed the update/name-label-for-block-style-variations-from-theme-json branch from 41808df to c0cca63 Compare June 13, 2024 14:47
@oandregal oandregal added Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta and removed Backported to WP Core Pull request that has been successfully merged into WP Core labels Jun 13, 2024
@aaronrobertshaw
Copy link
Contributor

aaronrobertshaw commented Jun 14, 2024

There's an issue with styles generation: when the slug is provided the styles are not generated.

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.

@aaronrobertshaw aaronrobertshaw marked this pull request as ready for review June 14, 2024 06:10
Copy link

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Copy link
Contributor

@aaronrobertshaw aaronrobertshaw left a 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.

@aaronrobertshaw
Copy link
Contributor

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.

@aaronrobertshaw
Copy link
Contributor

The unit test updates are pretty basic but cover:

  • That a partial using a slug overrides one supplied directly through register_block_style (duplicate UI option issue)
  • That a partial supplied block style variation is registered and resolved by its slug (styles not generating correctly if it relied on title)
@oandregal oandregal enabled auto-merge (squash) June 14, 2024 08:37
@oandregal oandregal merged commit 6f0950b into trunk Jun 14, 2024
62 checks passed
@oandregal oandregal deleted the update/name-label-for-block-style-variations-from-theme-json branch June 14, 2024 08:54
@github-actions github-actions bot added this to the Gutenberg 18.6 milestone Jun 14, 2024
@sirreal
Copy link
Member

sirreal commented Jun 14, 2024

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?

1) Gutenberg_REST_Block_Editor_Settings_Controller_Test::test_get_items
Undefined array key "WithSlug"

Here's a recent example run.

@aaronrobertshaw
Copy link
Contributor

The error does appear related, we'll look into it 👀

@aaronrobertshaw
Copy link
Contributor

The changes to the tests have been reverted in #62579 while we investigate the issue via #62587

patil-vipul pushed a commit to patil-vipul/gutenberg that referenced this pull request Jun 17, 2024
Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
ellatrix pushed a commit that referenced this pull request Jun 18, 2024
Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
ellatrix pushed a commit that referenced this pull request Jun 18, 2024
Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
@ellatrix
Copy link
Member

I just cherry-picked this PR to the wp/6.6-beta-3 branch to get it included in the next release: 7cfc6c5

@ellatrix ellatrix added Backported to WP Core Pull request that has been successfully merged into WP Core and removed Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta labels Jun 18, 2024
ellatrix pushed a commit that referenced this pull request Jun 18, 2024
Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backported to WP Core Pull request that has been successfully merged into WP Core Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json [Type] Bug An existing feature does not function as intended
4 participants