Section Styles: improve performance and conceptual consistency #62712

aaronrobertshaw · 2024-06-21T05:25:33Z

[Proof of Concept] Optimize how shared block style variations are processed wordpress-develop#6868
Section Styles: improve performance and conceptual consistency wordpress-develop#6873

What?

Remains unchanged: the block style variations (JSON files within styles/ with a blockTypes key).

Changes to the primary theme.json and theme style variations (JSON files within styles/ without a blockTypes) key :

Move shared variations from styles.blocks.variations to styles.variations.
Remove blockTypes from styles.variations.
Do not register shared variations from these files.

Other changes:

Instead of using the wp_theme_json_data_* filters to merge the data, uses the WP_Theme_JSON_Resolver and WP_Theme_JSON classes, as per #6868.

Why?

More conceptual consistency in overrides:
- block-level variations (styles.blocks.blockType.variations) are higher priority than top-level variations (styles.variations)
- from lower to higher priority: block registry < block style variation files < theme.json < user changes (including theme style variations)
Improved performance.

Testing Instructions

Disable these duotone filters so they don't register block style variations by calling the theme.json resolver which doesn't happen in core. See this comment for issue found during testing.

diff --git a/lib/block-supports/duotone.php b/lib/block-supports/duotone.php
index 98791fe05c..df8aa39414 100644
--- a/lib/block-supports/duotone.php
+++ b/lib/block-supports/duotone.php
@@ -18,8 +18,8 @@ if ( class_exists( 'WP_Duotone' ) ) {
 	remove_action( 'wp_loaded', array( 'WP_Duotone', 'set_global_styles_presets' ) );
 	remove_action( 'wp_loaded', array( 'WP_Duotone', 'set_global_style_block_names' ) );
 }
-add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_styles_presets' ), 10 );
-add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_style_block_names' ), 10 );
+// add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_styles_presets' ), 10 );
+// add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_style_block_names' ), 10 );
 
 // Add classnames to blocks using duotone support.
 if ( function_exists( 'wp_render_duotone_support' ) ) {

Defines block style variations via the different sources below:
- register_block_style
- Theme.json partial file under /styles directory including blockTypes
- Within a theme style variation under styles.variations
  - Define at least one that is registered via register_block_style or a theme.json partial as well
  - Define a variation that hasn't been registered
Confirm that only registered variations appear for selection (those defined via register_block_style, theme.json partials, or in core block types styles property such as Button's Outline style)
Try out all the variations and ensure they get applied correctly in the editor and frontend
Update the variation definitions to define inner element and block type styles as well
Check the inner element and block type styles are applied correctly for variations
Test applying variations on blocks in a nested fashion
Update variation styles via Styles > Blocks > Block Type > Variation and confirm they apply correctly everywhere

`register_block_style` Example

gutenberg_register_block_style(
	array( 'core/group', 'core/columns' ),
	array(
		'name'       => 'section-b',
		'label'      => __( 'Section B' ),
		'style_data' => array(
			'color'    => array(
				'background' => '#4f6f52',
				'text'       => '#d2e3c8',
			),
			'blocks'   => array(
				'core/group' => array(
					'color' => array(
						'background' => '#739072',
						'text'       => '#e3eedd',
					),
				),
			),
			'elements' => array(
				'link' => array(
					'color'  => array(
						'text' => '#ead196',
					),
					':hover' => array(
						'color' => array(
							'text' => '#ebd9b4',
						),
					),
				),
			),
		),
	)
);

Theme.json Partial Example

{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 3,
	"title": "Section A",
	"slug": "section-a",
	"blockTypes": [ "core/group" ],
	"styles": {
		"color": {
			"background": "slategrey",
			"text": "snow"
		},
		"blocks": {
			"core/group": {
				"color": {
					"background": "darkslategrey",
					"text": "whitesmoke"
				}
			}
		}
	}
}

Theme Style Variation Example (`styles.variations`)

{

	"styles": {
		"variations": {
			"section-a": {
				"color": {
					"background": "var(--wp--preset--color--theme-2)",
					"text": "var(--wp--preset--color--theme-5)"
				},
				"blocks": {
					"core/group": {
						"color": {
							"background": "var(--wp--preset--color--theme-4)",
							"text": "var(--wp--preset--color--theme-2)"
						}
					}
				}
			}
		}
	}
}

Demo

Screen.Recording.2024-06-24.at.2.29.21.PM.mp4

ramonjd · 2024-06-21T07:03:15Z

I had to time box testing today, but can carry on later.

I went through the steps outlined in #57908, taking into account the move to styles.variations and that I can't register shared variations in theme.json or theme variations.

Block style variation JSON files under `/styles`

These are files with blockTypes at the top level.

Example

{
	"$schema": "https://schemas.wp.org/trunk/theme.json",
	"version": 3,
	"title": "White fang",
	"blockTypes": [
		"core/group",
		"core/image",
		"core/columns"
	],
	"styles": {
		"color": {
			"background": "yellow"
		},
		"border": {
			"radius": "1111px",
			"width": "10px",
			"color": "red",
			"style": "solid"
		},
		"blocks": {
			"core/paragraph": {
				"border": {
					"width": "0"
				},
				"color": {
					"background": "aliceblue",
					"text": "purple"
				}
			}
		}
	}
}

I can see the variation buttons in the side bar and the styles are applied as expected in the editor and the frontend.

Nested block styles overwrite upper level styles as expected with the correct classnames (no duplicate classnames).

Editing block style variations in the Site Editor

When editing a block style in Global styles, the variations are saved in the database under styles.blocks[blockName].variations[variationName].

When editing the block styles, the editor updates in real time and the frontend reflects the new styles.

Updating the block style for one block does not affect other blocks that may share the original style variation.

Registering block style variations programmatically

Works as expected via gutenberg_register_block_style, similar to the findings above.

Global styles revisions/style book

To confirm.

I'm not sure if this works yet, or if I'm doing something wrong. When switching to the Style Book or browsing past global styles revisions I'm not seeing any variation styles, even if changes to variations are saved in the CPT revisions.

github-actions · 2024-06-21T07:10:23Z

Flaky tests detected in 9d8cf0e.
Some tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information.

🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/9609518843
📝 Reported issues:

[Flaky Test] Redirection after template creation #60678 in /test/e2e/specs/site-editor/site-editor-url-navigation.spec.js

aaronrobertshaw · 2024-06-21T07:45:04Z

Global styles revisions/style book

The preview of the styles doesn't seem to be working at first glance. Once I hit apply they seem to. I'll need to dig into that one.

lib/class-wp-theme-json-gutenberg.php

aaronrobertshaw · 2024-06-21T10:57:43Z

Replication steps for the Global Styles revisions issue:

Edit some block style variations that have been applied to the page or choose a theme style variation that redefines the block style variations
Save the changes
Navigate to Global Styles > Revisions
Select a prior revision that should have block style variations styles
Note that the variation styles aren't applied in the editor
Click apply and note the variations are now applied

Screen.Recording.2024-06-21.at.8.53.09.PM.mp4

github-actions · 2024-06-21T16:13:49Z

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: aaronrobertshaw <aaronrobertshaw@git.wordpress.org>
Co-authored-by: oandregal <oandregal@git.wordpress.org>
Co-authored-by: talldan <talldanwp@git.wordpress.org>
Co-authored-by: andrewserong <andrewserong@git.wordpress.org>
Co-authored-by: ramonjd <ramonopoly@git.wordpress.org>
Co-authored-by: richtabor <richtabor@git.wordpress.org>

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

oandregal · 2024-06-21T16:35:23Z

Prepared backport at WordPress/wordpress-develop#6873 to gauge performance impact.

lib/class-wp-theme-json-resolver-gutenberg.php

richtabor · 2024-06-21T20:21:10Z

I spun up a PR for Assembler (a theme I'm using to test this extensively) to align with this proposal.

richtabor · 2024-06-21T20:25:23Z

One change I noted is that the generated class is exclusively defined by the title from the style variation partial, whereas previously the slug was defined by the file name. Is that intentional?

For example, this results in a class of is-style-style-1:

While previously, it would have resulted in is-style-section-1.

richtabor · 2024-06-21T22:54:23Z

I suppose I could set slug independently as well.

aaronrobertshaw · 2024-06-22T05:44:59Z

One change I noted is that the generated class is exclusively defined by the title from the style variation partial, whereas previously the slug was defined by the file name. Is that intentional?

As far as I can tell this has changed at all.

In both trunk and this branch, the variation's class name was based off the title for variation partial theme.json files.

I also checked prior to each of the following PR commits

I went all the way back to the introduction of the feature in:

Block Styles: Extend block style variations as mechanism for achieving section styling #57908

It behaved the same at each point for me. With no slug value in the partial and the title differing from the file name, the name the variation was registered under was the kebab-cased version of its title. At each point tested, it is the variation name used for the CSS class.

Is it possible this was a misconception rather than an actual regression?

aaronrobertshaw · 2024-06-22T06:25:47Z

lib/class-wp-theme-json-gutenberg.php

+			return $theme_json;
+		}
+
+		$registered_styles = WP_Block_Styles_Registry::get_instance()->get_all_registered();


It doesn't appear that core block type variations (e.g. Button outline) get registered in the block styles registry.

We might be able to loop registered block types and check their styles property similar to what happens when determining valid variations for sanitization etc.

Perhaps a better option would be to move the call to unwrap_shared_block_style_variations to after the collection of $valid_variations, then pass those as a new param. They should then contain both the core block styles as well as any registered in the WP_Block_Styles_Registry.

What do you think?

In the interests of saving time, I've pushed a commit that I think does the job: bc758fc.

aaronrobertshaw · 2024-06-22T07:54:49Z

One change I noted is that the generated class is exclusively defined by the title from the style variation partial, whereas previously the slug was defined by the file name. Is that intentional?

I think I've got an explanation for this one now.

Since the section styles feature was introduced, if a theme.json partial had a title property the kebab-cased version of that was used as the variation name and in turn formed the basis of the CSS class name. However, if a theme.json partial omitted the title (and the slug after that was introduced), the file name is used.

This fallback to the filename is still in place, so I'm confident there's been no regression in this regard.

richtabor · 2024-06-22T22:28:20Z

Is it possible this was a misconception rather than an actual regression?

In the current version of the Assembler theme, where I'm testing this, I have "Style 1" visible labels, while using "section-1" formatting for the action variations. I don't think this is a regression, but a difference in how it works currently in trunk (whether that's intended or not). If a combination of using slug and title gets the same result, it's all good.

aaronrobertshaw · 2024-06-23T00:57:08Z

I don't think this is a regression, but a difference in how it works currently in trunk

That's the catch though, for me, it works the same as on trunk.

When the theme.json partials are parsed, if it doesn't have a title property it gets the base of the filename set to the property. You can see that code here which hasn't changed since #46554 (about a year).

The only recent change around this functionality was introducing support for the slug property, which is optional and would take precedence over the title, if slug is present. That occurred in #62550.

If there is a difference I haven't been able to replicate it, nor see it in code.

If a combination of using slug and title gets the same result, it's all good.

👍 That's the main thing I guess. If you're happy, I'm happy.

ramonjd · 2024-06-23T01:43:22Z

The preview of the styles doesn't seem to be working at first glance. Once I hit apply they seem to. I'll need to dig into that one.

Passing variationStyles: true to toStyles seems to fix it

		const globalStyles = toStyles(
			updatedConfig,
			blockSelectors,
			hasBlockGapSupport,
			hasFallbackGapSupport,
			disableLayoutStyles,
			options?.disableRootPadding,
			{
				variationStyles: true,
			}
		);

gutenberg/packages/block-editor/src/components/global-styles/use-global-styles-output.js

Lines 1366 to 1373 in bc758fc

    
           const globalStyles = toStyles( 
        
           	updatedConfig, 
        
           	blockSelectors, 
        
           	hasBlockGapSupport, 
        
           	hasFallbackGapSupport, 
        
           	disableLayoutStyles, 
        
           	disableRootPadding 
        
           );

The only question I have is whether variationStyles needs to always be true? I'm not sure, due to #62465, which introduced variationStyles

I have a PR with the change over here:

Global styles revisions: ensure that user-defined variation styles CSS is generated #62768

aaronrobertshaw · 2024-06-23T02:56:54Z

The only question I have is whether variationStyles needs to always be true? I'm not sure, due to #62465, which introduced variationStyles

There might be a catch with simply passing the variationStyles option at this stage. I'd expect it to generate generic is-style-* styles but not ones specific to an individual application of a block style variation.

Passing that option should appear to work for simple uses of variations but would likely fall short when variations are applied in a nested fashion. For example, a light section that contains a dark section, then another light section within the dark one.

I'll take a look at #62768 later today and double check what I've said.

Thanks for putting together that fix so quickly 🚀

Update: Unfortunately, I could confirm my hunch was correct. More details here: #62768 (review)

aaronrobertshaw · 2024-06-24T04:03:57Z

I've removed title and blockTypes from the shared definitions in the theme.json schema (i.e. styles.variations) given we now require prior registration of variations.

aaronrobertshaw · 2024-06-24T04:37:00Z

Thanks for the testing @andrewserong 👍

I've given this one another round of testing and I didn't notice any issues either. The extra unit tests around merging data give a nice bump in confidence as well.

A rough demo video, done after testing, has been added to the PR description for good measure.

aaronrobertshaw · 2024-06-24T07:19:42Z

While testing the backport for these changes I noticed that the theme style variation based definitions of block style variations are not being applied. I believe this is due to a couple of duotone filters in Gutenberg which trigger registration of block style variations from theme.json partials before the variations endpoint in the global styles controller is executed.

I've updated the test instructions to include toggling these duotone filters off.

diff --git a/lib/block-supports/duotone.php b/lib/block-supports/duotone.php
index 98791fe05c..df8aa39414 100644
--- a/lib/block-supports/duotone.php
+++ b/lib/block-supports/duotone.php
@@ -18,8 +18,8 @@ if ( class_exists( 'WP_Duotone' ) ) {
 	remove_action( 'wp_loaded', array( 'WP_Duotone', 'set_global_styles_presets' ) );
 	remove_action( 'wp_loaded', array( 'WP_Duotone', 'set_global_style_block_names' ) );
 }
-add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_styles_presets' ), 10 );
-add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_style_block_names' ), 10 );
+// add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_styles_presets' ), 10 );
+// add_action( 'wp_loaded', array( 'WP_Duotone_Gutenberg', 'set_global_style_block_names' ), 10 );
 
 // Add classnames to blocks using duotone support.
 if ( function_exists( 'wp_render_duotone_support' ) ) {

…yle variations

aaronrobertshaw · 2024-06-24T08:05:31Z

The fix for the issue noted above is to ensure the block style variations from partials were registered before the theme style variations are retrieved in the rest api endpoint.

I've tested and confirmed this fix is working while the duotone filters are disabled.

Screen.Recording.2024-06-24.at.6.02.33.PM.mp4

oandregal · 2024-06-24T08:24:51Z

I gave this some testing: tried things randomly, but also the core flows (layers override each other, users can update variations, etc.).

The direction is future-proof according to #62686 and this approach removes any concerns about backwards compatibility. It also improves performance as per the core PR's tests.

oandregal · 2024-06-24T08:33:30Z

There's a couple of playground failures. They are unrelated to this PR and happen in trunk, people are investigating. In the interest of merging this on time before WordPress 6.6 RC1, I'm going to force merge this PR when every other test is green.

oandregal · 2024-06-24T08:54:34Z

Corresponding core PR has been merged as well WordPress/wordpress-develop#6873 I was told we no longer need the label, as we are now only using it for tracking PRs that need package updates (we use the backport-changelog to cherry-pick the changes to the corresponding gutenberg branch).

This moves section styles within global style variations to the `styles.variations` property. It also removes the `blockTypes` definitions for each style variation. See: WordPress/gutenberg#62712

aaronrobertshaw added [Type] Enhancement A suggestion for improvement. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Jun 21, 2024

aaronrobertshaw self-assigned this Jun 21, 2024

talldan reviewed Jun 21, 2024

View reviewed changes

lib/class-wp-theme-json-gutenberg.php Outdated Show resolved Hide resolved

lib/class-wp-theme-json-gutenberg.php Outdated Show resolved Hide resolved

oandregal self-requested a review June 21, 2024 16:13

oandregal marked this pull request as ready for review June 21, 2024 16:13

oandregal requested review from tellthemachines and spacedmonkey as code owners June 21, 2024 16:13

oandregal changed the title ~~[WIP] Section Styles: Move shared definitions to styles.variations~~ Section Styles: improve performance and conceptual consistency Jun 21, 2024

oandregal mentioned this pull request Jun 21, 2024

Section Styles: improve performance and conceptual consistency WordPress/wordpress-develop#6873

Closed

oandregal reviewed Jun 21, 2024

View reviewed changes

lib/class-wp-theme-json-resolver-gutenberg.php Show resolved Hide resolved

richtabor mentioned this pull request Jun 21, 2024

Assembler: Use variation partials and move styles.blocks.variations to styles.variations Automattic/themes#7891

Open

aaronrobertshaw commented Jun 22, 2024

View reviewed changes

aaronrobertshaw force-pushed the update/style-variation-handling branch from a070981 to bc758fc Compare June 22, 2024 07:48

aaronrobertshaw mentioned this pull request Jun 23, 2024

Global styles revisions: ensure that user-defined variation styles CSS is generated #62768

Merged

1 task

Remove title from translatable theme.json schema

b7013f8

aaronrobertshaw added [Type] Performance Related to performance efforts 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 [Type] Enhancement A suggestion for improvement. labels Jun 24, 2024

ellatrix requested review from youknowriad, oandregal, talldan, richtabor, ramonjd and andrewserong June 24, 2024 07:01

Ensure block style variations are registered when retrieving theme st…

1e0065e

…yle variations

oandregal approved these changes Jun 24, 2024

View reviewed changes

oandregal enabled auto-merge (squash) June 24, 2024 08:27

oandregal disabled auto-merge June 24, 2024 08:27

oandregal merged commit 7c19dfd into trunk Jun 24, 2024
60 of 62 checks passed

oandregal deleted the update/style-variation-handling branch June 24, 2024 08:36

github-actions bot added this to the Gutenberg 18.7 milestone Jun 24, 2024

oandregal removed the Backport to WP 6.6 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jun 24, 2024

ellatrix mentioned this pull request Jun 24, 2024

Backports for 6.6 Beta/RC #62315

Open

This was referenced Jun 25, 2024

Global Styles: Remove obsolete hook resolving shared block style variations #62808

Merged

Global Styles: Add block variations to the isactive style variation calculation #62798

Closed

aaronrobertshaw mentioned this pull request Jun 26, 2024

Section Styles: Clean up block style variation filters #62858

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Section Styles: improve performance and conceptual consistency #62712

Section Styles: improve performance and conceptual consistency #62712

aaronrobertshaw commented Jun 21, 2024 •

edited

Loading

ramonjd commented Jun 21, 2024

github-actions bot commented Jun 21, 2024

aaronrobertshaw commented Jun 21, 2024

aaronrobertshaw commented Jun 21, 2024

github-actions bot commented Jun 21, 2024 •

edited

Loading

oandregal commented Jun 21, 2024 •

edited

Loading

richtabor commented Jun 21, 2024 •

edited

Loading

richtabor commented Jun 21, 2024

richtabor commented Jun 21, 2024

aaronrobertshaw commented Jun 22, 2024

aaronrobertshaw Jun 22, 2024

aaronrobertshaw Jun 22, 2024

aaronrobertshaw Jun 22, 2024

aaronrobertshaw commented Jun 22, 2024 •

edited

Loading

richtabor commented Jun 22, 2024 •

edited

Loading

aaronrobertshaw commented Jun 23, 2024

ramonjd commented Jun 23, 2024

aaronrobertshaw commented Jun 23, 2024 •

edited

Loading

aaronrobertshaw commented Jun 24, 2024

aaronrobertshaw commented Jun 24, 2024

aaronrobertshaw commented Jun 24, 2024 •

edited

Loading

aaronrobertshaw commented Jun 24, 2024

oandregal commented Jun 24, 2024

oandregal commented Jun 24, 2024

oandregal commented Jun 24, 2024

Section Styles: improve performance and conceptual consistency #62712

Section Styles: improve performance and conceptual consistency #62712

Conversation

aaronrobertshaw commented Jun 21, 2024 • edited Loading

What?

Why?

Testing Instructions

Demo

ramonjd commented Jun 21, 2024

Block style variation JSON files under /styles

Editing block style variations in the Site Editor

Registering block style variations programmatically

Global styles revisions/style book

github-actions bot commented Jun 21, 2024

aaronrobertshaw commented Jun 21, 2024

aaronrobertshaw commented Jun 21, 2024

github-actions bot commented Jun 21, 2024 • edited Loading

oandregal commented Jun 21, 2024 • edited Loading

richtabor commented Jun 21, 2024 • edited Loading

richtabor commented Jun 21, 2024

richtabor commented Jun 21, 2024

aaronrobertshaw commented Jun 22, 2024

aaronrobertshaw Jun 22, 2024

Choose a reason for hiding this comment

aaronrobertshaw Jun 22, 2024

Choose a reason for hiding this comment

aaronrobertshaw Jun 22, 2024

Choose a reason for hiding this comment

aaronrobertshaw commented Jun 22, 2024 • edited Loading

richtabor commented Jun 22, 2024 • edited Loading

aaronrobertshaw commented Jun 23, 2024

ramonjd commented Jun 23, 2024

aaronrobertshaw commented Jun 23, 2024 • edited Loading

aaronrobertshaw commented Jun 24, 2024

aaronrobertshaw commented Jun 24, 2024

aaronrobertshaw commented Jun 24, 2024 • edited Loading

aaronrobertshaw commented Jun 24, 2024

oandregal commented Jun 24, 2024

oandregal commented Jun 24, 2024

oandregal commented Jun 24, 2024

aaronrobertshaw commented Jun 21, 2024 •

edited

Loading

Block style variation JSON files under `/styles`

github-actions bot commented Jun 21, 2024 •

edited

Loading

oandregal commented Jun 21, 2024 •

edited

Loading

richtabor commented Jun 21, 2024 •

edited

Loading

aaronrobertshaw commented Jun 22, 2024 •

edited

Loading

richtabor commented Jun 22, 2024 •

edited

Loading

aaronrobertshaw commented Jun 23, 2024 •

edited

Loading

aaronrobertshaw commented Jun 24, 2024 •

edited

Loading