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

Update: register block style variations defined by the theme using the init action #6756

Conversation

oandregal
Copy link
Member

@oandregal oandregal commented Jun 7, 2024

Trac ticket: https://core.trac.wordpress.org/ticket/61312

Backports WordPress/gutenberg#62461 and WordPress/gutenberg#62495
Fixes: WordPress/gutenberg#62303
Alternative to WordPress/gutenberg#62405 (comment)

What?

This PR changes how the block style variations defined by the theme are registered: from using the wp_theme_json_data_* filters to use the init filter.

Why?

Using the wp_theme_json_data_* filters is problematic: in some scenarios (e.g.: ) variations end up not being registered because those filters were not dispatched, hence their styles are not generated. Using init is in line with what themes have been doing until now in their functions.php and makes sure the style variations are registered in all the places.

How?

  • b5e5f4d Hook into the init hook and register the block style variations from the 3 places where they can come from:
    • theme.json "partials": json files stored in styles/ that are not theme style variations
    • theme's theme.json
    • user's theme.json (this would come from the theme style variations defined by the theme)
  • 85e3c2f Remove the code that registers them upon dispatching the wp_theme_json_data_* filters.

Testing Instructions

  1. Register style variations in all the places the theme is allowed to (use TT4).

In the theme.json file of the theme, paste the following under styles.blocks.variations:

"ThemeDark": {
    "blockTypes": [
        "core/group",
        "core/columns"
    ],
    "color": {
            "background": "blue"
    }
}

In the styles/ember.json (theme style variation) file, paste the following under styles.blocks.variations:

"VariationDark": {
    "blockTypes": [
        "core/group",
        "core/columns"
    ],
    "color": {
            "background": "yellow"
    }
}

Create a new file called styles/partial.json (partial theme.json), and paste the following:

{
        "$schema": "https://schemas.wp.org/trunk/theme.json",
        "version": 2,
        "title": "PartialDark",
        "blockTypes": [ "core/group", "core/columns", "core/media-text" ],
        "styles": {
                "color": {
                        "background": "green"
                }
        }
}
  1. Open the site editor, go to Styles, apply the "Ember" theme style variation and save.
  2. Go to the site editor, select a group block, and verify the three new style variations are available:
Captura de ecrã 2024-06-07, às 17 41 03
  1. Edit the styles of one of them, for example, PartialDark. Go to "Global Styles > Blocks > Group > PartialDark". Modify the background color to something else.
  2. Save the post. The expected result is that it stays as it is (it's not reset to the default).
Gravacao.do.ecra.2024-06-07.as.17.56.06.mov

Copy link

github-actions bot commented Jun 7, 2024

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@oandregal oandregal marked this pull request as ready for review June 10, 2024 16:43
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.

Core Committers: Use this line as a base for the props when committing in SVN:

Props oandregal, aaronrobertshaw, annezazu.

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

@oandregal
Copy link
Member Author

There's a legit error on the WP_Block_Supports_Block_Style_Variations_Test::test_add_registered_block_styles_to_theme_data test. I ran out of time today to look at this but will take a look tomorrow.

@aaronrobertshaw
Copy link

There's a legit error on the WP_Block_Supports_Block_Style_Variations_Test::test_add_registered_block_styles_to_theme_data test

This test retrieves the theme origin data via the resolver. So the registration of styles was happening via the wp_theme_json_data_theme prior to this PR's switch to the init action.

If we either call do_action('init') or wp_register_block_style_variations_from_theme() within the test after the theme switch, it passes successfully again.

It might also be worth tweaking the name of wp_register_block_style_variations_from_theme to reflect its from theme json rather than generically from a theme.

@aaronrobertshaw
Copy link

I've created a corresponding PR for Gutenberg: WordPress/gutenberg#62461

@oandregal oandregal force-pushed the fix/block-style-variation-registration branch from 5b94370 to f6ca6b3 Compare June 11, 2024 06:23
@oandregal oandregal self-assigned this Jun 11, 2024
@oandregal
Copy link
Member Author

oandregal commented Jun 11, 2024

In testing we uncovered one issue when the style variation is modified but not saved:

  1. Register style variations the styles/ember.json (theme style variation) file, paste the following under styles.blocks.variations:
"VariationDark": {
    "blockTypes": [
        "core/group",
        "core/columns"
    ],
    "color": {
            "background": "yellow"
    }
}
  1. Open the site editor, go to Styles, apply the "Ember" theme style variation AND DO NOT SAVE IT YET. This is the key to the issue.
  2. Go to the site editor, select a group block, and verify the VariationDark is available:
Captura de ecrã 2024-06-07, às 17 41 03
  1. Update the VariationDark variation by going to "Global Styles > Blocks > Group > VariationDark". Modify the background color to something else.
  2. Save the post. The expected result is that the new background stays as it is and it's not reset. However, it's reset and the new color will only be available when reloading the site editor.

@oandregal oandregal force-pushed the fix/block-style-variation-registration branch from f6ca6b3 to 6a197d3 Compare June 11, 2024 18:32
Copy link

@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 the continued iteration on this one @oandregal 👍

It's testing well for me across each of the registration sources for block style variations:

✅ Theme.json
✅ Theme style variations
✅ Partial theme.json files

In particular, I can also confirm the recent updates fix the issue we found yesterday.

Before After
Screen.Recording.2024-06-12.at.12.56.15.PM.mp4
Screen.Recording.2024-06-12.at.12.52.29.PM.mp4

There are a few minor nits that we need to address before committing this one. I've left inline comments for them and a diff below given I can't push to this PR.

Suggested changes
diff --git a/src/wp-includes/block-supports/block-style-variations.php b/src/wp-includes/block-supports/block-style-variations.php
index 8312f5edcf..ae0e65e39c 100644
--- a/src/wp-includes/block-supports/block-style-variations.php
+++ b/src/wp-includes/block-supports/block-style-variations.php
@@ -249,7 +249,7 @@ function wp_resolve_block_style_variations( $variations ) {
 		 * 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 : _wp_to_kebab_case( $variation['title'] );
 
 		foreach ( $supported_blocks as $block_type ) {
 			// Add block style variation data under current block type.
@@ -410,6 +410,7 @@ add_filter( 'wp_theme_json_data_user', 'wp_resolve_block_style_variations_from_t
  * theme.json data.
  *
  * @access private
+ * @since 6.6.0
  *
  * @param array $variations Shared block style variations.
  */
@@ -469,6 +470,7 @@ function wp_register_block_style_variations_from_theme_json_data( $variations )
  * - the user's theme.json (for example, theme style variations the user selected)
  *
  * @access private
+ * @since 6.6.0
  */
 function wp_register_block_style_variations_from_theme() {
 	// Partials from `/styles`.
diff --git a/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php b/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php
index 24c8123b2a..1012ff0ba9 100644
--- a/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php
+++ b/src/wp-includes/rest-api/endpoints/class-wp-rest-global-styles-controller.php
@@ -231,6 +231,7 @@ class WP_REST_Global_Styles_Controller extends WP_REST_Posts_Controller {
 	 *
 	 * @since 5.9.0
 	 * @since 6.2.0 Added validation of styles.css property.
+	 * @since 6.6.0 Added registration of theme style variation defined block style variations.
 	 *
 	 * @param WP_REST_Request $request Request object.
 	 * @return stdClass|WP_Error Prepared item on success. WP_Error on when the custom CSS is not valid.
@@ -264,14 +265,16 @@ class WP_REST_Global_Styles_Controller extends WP_REST_Posts_Controller {
 				$config['styles'] = $existing_config['styles'];
 			}
 
-			// If the incoming request is going to create a new variation
-			// that is not yet registered, we register it here.
-			// This is because the variations are registered on init,
-			// but we want this endpoint to return the new variation immediately:
-			// if we don't register it, it'll be stripped out of the response
-			// just in this request (subsequent ones will be ok).
-			// Take the variations defined in styles.blocks.variations from the incoming request
-			// that are not part of the $exsting_config.
+			/*
+			 * If the incoming request is going to create a new variation
+			 * that is not yet registered, we register it here.
+			 * This is because the variations are registered on init,
+			 * but we want this endpoint to return the new variation immediately:
+			 * if we don't register it, it'll be stripped out of the response
+			 * just in this request (subsequent ones will be ok).
+			 * Take the variations defined in styles.blocks.variations from the incoming request
+			 * that are not part of the $existing_config.
+			 */
 			if ( isset( $request['styles']['blocks']['variations'] ) ) {
 				$existing_variations = isset( $existing_config['styles']['blocks']['variations'] ) ? $existing_config['styles']['blocks']['variations'] : array();
 				$new_variations      = array_diff_key( $request['styles']['blocks']['variations'], $existing_variations );
diff --git a/tests/phpunit/tests/block-supports/block-style-variations.php b/tests/phpunit/tests/block-supports/block-style-variations.php
index 987959e3cb..3adbaa4075 100644
--- a/tests/phpunit/tests/block-supports/block-style-variations.php
+++ b/tests/phpunit/tests/block-supports/block-style-variations.php
@@ -65,9 +65,11 @@ class WP_Block_Supports_Block_Style_Variations_Test extends WP_UnitTestCase {
 	public function test_add_registered_block_styles_to_theme_data() {
 		switch_theme( 'block-theme' );
 
-		// Trigger block style registration that occurs on `init` action.
-		// do_action( 'init' ) could be used here however this direct call
-		// means only the updates being tested are performed.
+		/*
+		 * Trigger block style registration that occurs on `init` action.
+		 * do_action( 'init' ) could be used here however this direct call
+		 * means only the updates being tested are performed.
+		 */
 		wp_register_block_style_variations_from_theme();
 
 		$variation_styles_data = array(

@aaronrobertshaw
Copy link

I've created a Gutenberg PR for the most recent updates here: WordPress/gutenberg#62495

@oandregal oandregal force-pushed the fix/block-style-variation-registration branch from f301591 to a40bb88 Compare June 12, 2024 06:44
Copy link

@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.

Code changes look good after the minor tweaks and including the test from WordPress/gutenberg#62495.

🚢

@oandregal oandregal changed the title Update: register block style variations defined by the theme using the init filter Update: register block style variations defined by the theme using the init action Jun 12, 2024
@oandregal
Copy link
Member Author

Committed at https://core.trac.wordpress.org/changeset/58394

@oandregal oandregal closed this Jun 12, 2024
@oandregal oandregal deleted the fix/block-style-variation-registration branch June 12, 2024 07:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Section styles: editing a block variation in Styles doesn't reflect in the editor
2 participants