Global Styles: Add style variation partial cache #62610

aaronrobertshaw · 2024-06-17T03:33:04Z

Global Styles: Skip registration of variation styles when unsupported #62529

What?

Adds caching to processed theme.json partials for style variations.

Why?

With registration of block style variations moved to a separate init action, the processing of /styles directories, including parsing and translating variation partials, was happening multiple times.

How?

Processes all variation partials and stores them in a cache keyed by the theme, then locale.

Testing Instructions

Unit tests should be passing
Smoke test display and application of theme style variations in the site editor
Test section styles are still registered correctly and applying them works
- Detailed instructions on setting up custom block style variations for testing section styles can be found in #57908

github-actions · 2024-06-17T03:34:19Z

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

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

github-actions · 2024-06-17T05:10:14Z

Flaky tests detected in 72f8a82.
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/9578992360
📝 Reported issues:

[Flaky Test] Navigates from items list to preview via TAB, and vice versa #62434 in /test/e2e/specs/site-editor/dataviews-list-layout-keyboard.spec.js

github-actions · 2024-06-17T06:56:39Z

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 <[email protected]>
Co-authored-by: oandregal <[email protected]>
Co-authored-by: ramonjd <[email protected]>
Co-authored-by: joemcgill <[email protected]>
Co-authored-by: ellatrix <[email protected]>

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

oandregal · 2024-06-17T08:01:43Z

I've done some testing here and this is what I've got:

6.5	6.6 (no plugins)	6.6 + Gutenberg 18.5.0	6.6 + Gutenberg `trunk`	6.6 + this PR

I've noticed a couple of things:

In WordPress core trunk: the Onyx style variation defined by TwentyTwentyFour is not listed in the top-level theme style variations.
With Gutenberg trunk, the palettes and typography are not displayed.
With this PR, the Rust theme style variations is not listed.

ramonjd · 2024-06-17T08:17:53Z

With this PR, the Rust theme style variations is not listed.

Seems to be filtered out in the variations component here somewhere:

gutenberg/packages/edit-site/src/components/global-styles/style-variations-container.js

Line 37 in d66f4d1

const multiplePropertyVariations = variations?.filter( ( variation ) => {

I can see Rust in the REST response and in the return value of __experimentalGetCurrentThemeGlobalStylesVariations(), so I'd say it's not this PR (?)

ramonjd · 2024-06-17T08:24:48Z

I can see Rust in the REST response and in the return value of __experimentalGetCurrentThemeGlobalStylesVariations(), so I'd say it's not this PR (?)

Ah, so the variations are filtered by isVariationWithSingleProperty, and since Rust only contains colors, it's skipped, and its colors are turned into a "Palette" below in the ColorVariations component. Looks to be expected behavior.

oandregal · 2024-06-17T10:09:29Z

I've scoped down WordPress/wordpress-develop#6837 to only #62529 as to prevent it from being blocked until this PR is resolved. We'll need to update the backports changelog & create a new one when time comes.

ramonjd · 2024-06-17T23:51:29Z

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

+		$locale    = get_locale();
+
+		if (
+			isset( static::$style_variations_cache[ $theme_dir ][ $locale ][ $scope ] ) &&


Actually, I withdraw my comment.

~~Many of the Rust variation's block styles seems to be missing, e.g., "core/calendar" when the cache is returned here.~~

It seems to be missing in the return value of $variation = ( new WP_Theme_JSON_Gutenberg( $translated ) )->get_raw_data(); below.

ramonjd · 2024-06-18T00:09:03Z

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

+
+				if ( $variation_scope ) {
+					$translated = static::translate( $decoded_file, wp_get_theme()->get( 'TextDomain' ) );
+					$variation  = ( new WP_Theme_JSON_Gutenberg( $translated ) )->get_raw_data();


From what I'm seeing, many of Rust's block styles aren't registered and are therefore failing the test in ::get_blocks_metadata()

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

Lines 1080 to 1084 in d66f4d1

$registered_styles = $style_registry->get_all_registered();

foreach ( static::$blocks_metadata as $block_name => $block_metadata ) {

if ( ! empty( $registered_styles[ $block_name ] ) ) {

$style_selectors = $block_metadata['styleVariations'] ?? array();

aaronrobertshaw · 2024-06-19T00:38:45Z

I've removed the backport label for this PR.

A simpler approach caching only the read json file was merged in #62638.

I'll leave this PR open for now however as there are still plans to potentially iterate further on the caching strategy for style variations.

oandregal · 2024-06-19T07:51:57Z

I learned that the colors & typography panels are only to be filled with style variations that use a single property (e.g.: for color, it should be settings.color.palette + color styles) ((slack thread)). Essentially, what Ramon shared above. So this is the new expectation (as it works in trunk):

Captura de ecrã 2024-06-19, às 09 32 59

I'm going to rebase this PR and test again.

oandregal · 2024-06-19T08:49:00Z

I've rebased this and it works as expected.

Also prepared the corresponding core PR WordPress/wordpress-develop#6857 for us to gauge the impact this may have. I've left some thoughts and asked feedback from core-performance folks.

oandregal · 2024-06-19T08:50:44Z

I added back the "Backport to beta/RC" label, so we don't forget if this eventually gets merged. However, I'd like to hear feedback from core folks in the core PR before merging this one here.

joemcgill · 2024-06-19T23:32:23Z

I've left some relevant feedback in this PR along with some more general observations to the related Trac ticket, but wanted to post links here for visibility.

Is there a better tracking issue in this repo where you all are collaborating on the performance improvements for this feature? Given how fractured the development process is for changes affecting WP_Theme_JSON code between this repo and https://github.com/WordPress/wordpress-develop, I'm finding it somewhat difficult to keep track of where the relevant discussion is happening.

ellatrix · 2024-06-20T09:22:45Z

This only touches PHP files, so no need to backport it for packages?

oandregal · 2024-06-20T10:09:35Z

I've left some relevant feedback in WordPress/wordpress-develop#6857 (comment) along with some more general observations to the related Trac ticket, but wanted to post links here for visibility.

Thanks for double checking, oversharing never hurts. I also want to keep the conversation over there (core PR / trac ticket).

Is there a better tracking issue in this repo where you all are collaborating on the performance improvements for this feature?

The core trac ticket is the central place for this.

oandregal · 2024-06-20T10:15:06Z

This only touches PHP files, so no need to backport it for packages?

Thanks for the reminder, I've removed the label. I thought all PRs still had to have that. I guess it's now irrelevant given that we already have the backports-changelog. Btw, thanks for that, I'm finding it helpful. It changes the dynamics of backports: the burden is on the people with more context (the PR's authors/reviewers rather than the release leads).

aaronrobertshaw added [Type] Performance Related to performance efforts Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Jun 17, 2024

aaronrobertshaw requested a review from oandregal June 17, 2024 03:33

aaronrobertshaw self-assigned this Jun 17, 2024

aaronrobertshaw mentioned this pull request Jun 17, 2024

Global Styles: Avoid unnecessary processing of theme.json variation partials WordPress/wordpress-develop#6837

Closed

aaronrobertshaw added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Jun 17, 2024

aaronrobertshaw force-pushed the add/style-variation-cache branch from 5c5e69d to 9640387 Compare June 17, 2024 04:47

aaronrobertshaw marked this pull request as ready for review June 17, 2024 06:56

aaronrobertshaw requested a review from spacedmonkey as a code owner June 17, 2024 06:56

ramonjd reviewed Jun 17, 2024

View reviewed changes

ramonjd reviewed Jun 18, 2024

View reviewed changes

This was referenced Jun 18, 2024

Read only once the theme.json files from the styles/ folder. WordPress/wordpress-develop#6843

Closed

Theme JSON resolver: read theme.json files from the styles/ folder only once #62638

Merged

ellatrix mentioned this pull request Jun 18, 2024

Backports for 6.6 Beta/RC #62315

Closed

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

Global Styles: Add style variation partial cache

cdd91c9

oandregal force-pushed the add/style-variation-cache branch from d66f4d1 to 04948a0 Compare June 19, 2024 07:56

Refill variations cache if blocks changed

c99de18

oandregal force-pushed the add/style-variation-cache branch from 04948a0 to c99de18 Compare June 19, 2024 08:01

oandregal mentioned this pull request Jun 19, 2024

Global styles: cache post-processed style variations WordPress/wordpress-develop#6857

Closed

Add backport file

72f8a82

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

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

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Global Styles: Add style variation partial cache #62610

Global Styles: Add style variation partial cache #62610

aaronrobertshaw commented Jun 17, 2024 •

edited

Loading

github-actions bot commented Jun 17, 2024

github-actions bot commented Jun 17, 2024 •

edited

Loading

github-actions bot commented Jun 17, 2024 •

edited

Loading

oandregal commented Jun 17, 2024 •

edited

Loading

ramonjd commented Jun 17, 2024 •

edited

Loading

ramonjd commented Jun 17, 2024

oandregal commented Jun 17, 2024

ramonjd Jun 17, 2024 •

edited

Loading

ramonjd Jun 18, 2024

aaronrobertshaw commented Jun 19, 2024

oandregal commented Jun 19, 2024 •

edited

Loading

oandregal commented Jun 19, 2024

oandregal commented Jun 19, 2024

joemcgill commented Jun 19, 2024

ellatrix commented Jun 20, 2024

oandregal commented Jun 20, 2024

oandregal commented Jun 20, 2024

	$registered_styles = $style_registry->get_all_registered();
	foreach ( static::$blocks_metadata as $block_name => $block_metadata ) {
	if ( ! empty( $registered_styles[ $block_name ] ) ) {
	$style_selectors = $block_metadata['styleVariations'] ?? array();

Global Styles: Add style variation partial cache #62610

Are you sure you want to change the base?

Global Styles: Add style variation partial cache #62610

Conversation

aaronrobertshaw commented Jun 17, 2024 • edited Loading

What?

Why?

How?

Testing Instructions

github-actions bot commented Jun 17, 2024

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

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

oandregal commented Jun 17, 2024 • edited Loading

ramonjd commented Jun 17, 2024 • edited Loading

ramonjd commented Jun 17, 2024

oandregal commented Jun 17, 2024

ramonjd Jun 17, 2024 • edited Loading

Choose a reason for hiding this comment

ramonjd Jun 18, 2024

Choose a reason for hiding this comment

aaronrobertshaw commented Jun 19, 2024

oandregal commented Jun 19, 2024 • edited Loading

oandregal commented Jun 19, 2024

oandregal commented Jun 19, 2024

joemcgill commented Jun 19, 2024

ellatrix commented Jun 20, 2024

oandregal commented Jun 20, 2024

oandregal commented Jun 20, 2024

aaronrobertshaw commented Jun 17, 2024 •

edited

Loading

github-actions bot commented Jun 17, 2024 •

edited

Loading

github-actions bot commented Jun 17, 2024 •

edited

Loading

oandregal commented Jun 17, 2024 •

edited

Loading

ramonjd commented Jun 17, 2024 •

edited

Loading

ramonjd Jun 17, 2024 •

edited

Loading

oandregal commented Jun 19, 2024 •

edited

Loading