Plugin: Use bundled files for enqueuing global styles #50310

Mamaduka · 2023-05-04T08:38:26Z

What?

PR adds a script loader not tied to the specific WP version to override global styles enqueued by core. The new one will use the WP_Theme_JSON classes bundled with the plugin.

Why?

Bundled WP_Theme_JSON files should provide some of the settings for global styles.

How?

Testing Instructions

#50079 (comment)

Mamaduka · 2023-05-04T14:22:42Z

Hey, @oandregal

What do you think about these "permanent" overrides for settings that need to use bundled WP_Theme_JSON_*Gutenberg classes?

tellthemachines

Thanks for getting this started! It not working for me yet with the testing steps I outlined in the other PR, but I think this is a good approach in general, at least while global styles and design tools are still under active development.

Mamaduka · 2023-05-05T05:19:11Z

@tellthemachines, I think I got all the parts ported into the new system. Overriding the styles for block editor settings was the missing piece - eec751a.

lib/block-editor-settings.php

oandregal · 2023-05-05T07:26:50Z

What do you think about these "permanent" overrides for settings that need to use bundled WP_Theme_JSON_*Gutenberg classes?

Yeah, this is annoying. As long as the underlying classes receive changes, the public API needs to point at them in the plugin.

I'd think it's best to make them "evergreen" (so they'd sit in lib/) as the private classes do. I wouldn't be against making them part of lib/compat/6.X, though it seems that this would just "move the problem to the future": once the minimum WP version for Gutenberg is 6.2 they'll be removed, so we'll run into the same issue because the private classes will still receive updates. Does that sound sensible?

Mamaduka · 2023-05-05T07:44:21Z

I wouldn't be against making them part of lib/compat/6.X, though it seems that this would just "move the problem to the future"

I agree. Keeping this code in compat/wordpress- creates an "opportunity" for future regressions.

One more thing. Should I also move _gutenberg_clean_theme_json_caches into the "evergreen" files, or is there a plan to match the cache keys used by the plugin to WP?

oandregal · 2023-05-05T08:00:15Z

One more thing. Should I also move _gutenberg_clean_theme_json_caches into the "evergreen" files, or is there a plan to match the cache keys used by the plugin to WP?

No, they need to be different from core.

Would you be up to prepare small PRs that migrate each method separately? Otherwise, it's a big PR with a lot of changes that are difficult to match to each other. If it's just moving methods around, things are easy to spot.

Mamaduka · 2023-05-05T08:06:24Z

Would you be up to prepare small PRs that migrate each method separately?

I would, but all the changes in this PR are needed to fix the regression reported by @tellthemachines - #50079 (comment).

I've not touched the methods; I just moved them. The changes that could use a detailed review are concentrated in lib/block-editor-settings.php and lib/script-loader.php files.

lib/compat/wordpress-6.2/block-editor-settings.php

lib/compat/wordpress-6.2/script-loader.php

oandregal · 2023-05-05T08:46:56Z

lib/compat/wordpress-6.2/get-global-styles-and-settings.php

-
-	/*
-	 * For the remaining types (presets, styles), we do consider origins:
+if ( ! function_exists( 'wp_get_global_styles_custom_css' ) ) {


I was expecting that the only change here would be to move some methods from this file to lib/get-global-styles-and-settings.php.

As far as I've checked, we should move gutenberg_get_global_styles_custom_css, gutenberg_get_global_stylesheet, gutenberg_get_global_settings, _gutenberg_clean_theme_json_caches, _gutenberg_add_non_persistent_theme_json_cache_group.

I moved everything besides _gutenberg_add_non_persistent_theme_json_cache_group. The cache group would be the same, so there's no need to keep that function after 6.2.

lib/script-loader.php

Mamaduka · 2023-05-09T16:52:30Z

@oandregal, I addressed the feedback, and PR is ready for another review.

github-actions · 2023-05-09T17:44:57Z

Flaky tests detected in f95f040.
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/4928503293
📝 Reported issues:

[Flaky Test] should be able to create an hierarchical post without title support #36939 in /test/e2e/specs/editor/plugins/custom-post-types.spec.js

Mamaduka · 2023-05-10T06:31:09Z

Hi, @tellthemachines

Did you get a chance to test the fix?

tellthemachines · 2023-05-10T07:03:20Z

@Mamaduka I won't be able to do a full code review this week, but I've just tested the latest state of this branch and it's now working correctly. I checked the spacing styles within flow/constrained layouts and also the experimental grid styles, and both are as expected ✅

Mamaduka · 2023-05-10T07:08:35Z

I won't be able to do a full code review this week,

Not a problem. I just want to be sure that PR fixes the regression. Thanks for testing!

aaronrobertshaw

Thanks for addressing this @Mamaduka 👍

It's always a pleasure to find a bug already has a fix up for review!

It turns out #50079 also caused the custom selectors API to be missed. This PR fixes that issue.

The code changes LGTM but it might be best to wait on a final review from @tellthemachines to make sure everything has been covered from their end.

oandregal · 2023-05-11T15:40:36Z

lib/compat/wordpress-6.2/get-global-styles-and-settings.php

- *
- * @access private
- */
-function _gutenberg_clean_theme_json_caches() {


This has been moved to lib/global-styles-and-settings.php verbatim.

oandregal · 2023-05-11T15:43:45Z

lib/compat/wordpress-6.2/default-filters.php

@@ -17,13 +17,6 @@
 * @package gutenberg
 */

-/**


This has been moved to lib/global-styles-and-settings.php verbatim. The comment no longer makes sense as core already uses the proper function as a callback.

oandregal · 2023-05-11T15:49:06Z

lib/compat/wordpress-6.2/script-loader.php

- *
- * @since 6.2.0
- */
-function gutenberg_enqueue_global_styles_custom_css() {


The function and the filter has been moved to lib/script-loader.php verbatim.

oandregal · 2023-05-11T15:51:51Z

lib/global-styles-and-settings.php

+ *
+ * @return void
+ */
+function gutenberg_add_global_styles_for_blocks() {


This brings back this function, that had been removed in #50079

oandregal · 2023-05-11T15:55:30Z

lib/script-loader.php

+ *
+ * @return void
+ */
+function gutenberg_enqueue_global_styles() {


This brings this function back after having being removed at #50079

oandregal · 2023-05-11T15:58:19Z

lib/script-loader.php

+ *
+ * @since 5.9.0
+ */
+function gutenberg_enqueue_global_styles_css_custom_properties() {


This brings back a function that I haven't seen at #50079 though it still makes sense to me.

oandregal

I haven't tested this solves the issues brought by others. The code changes make sense to me: either move things around or add back things (added comments for each function trying to document what changes).

Mamaduka · 2023-05-11T16:25:10Z

Thanks, @oandregal!

I’ll merge this to fix the regressions. Happy to follow up in case I missed something.

Mamaduka · 2023-05-12T06:35:59Z

@fabiankaegy, the #50079 will ship with Gutenberg 15.8; I think it makes sense to cherry-pick this PR and avoid regressions in the release.

fabiankaegy · 2023-05-12T06:52:06Z

I just cherry-picked this PR to the release/15.8 branch to get it included in the next release: f7d2fbd

* Plugin: Use bundled files for enqueuing global styles * Override the '__experimentalFeatures' * Recreate global styles for block editors * Move global styles custom CSS handlers * Move _gutenberg_clean_theme_json_caches * Override global styles custom CSS properties

oandregal · 2023-05-12T14:38:07Z

I've got a couple of follow-ups at #50596 and #50597.

ajlende · 2023-05-17T21:57:29Z

lib/block-editor-settings.php

+		}
+	}
+
+	$settings['styles'] = array_merge( $global_styles, get_block_editor_theme_styles() );


It seems this broke duotone filters which rely on updating $settings['styles'] in the same hook. Is there a reason that this hook needs to run at PHP_INT_MAX priority?

Is there a reason that this hook needs to run at PHP_INT_MAX priority?

That's what the plugin always used for block setting overrides. I guess to prevent other plugins from overriding new feature settings.

I personally don't like the use of PHP_INT_MAX; it prevents extensibility.

Can you point me to where the Duotone filters are located? Happy to help with the fix.

That's what the plugin always used for block setting overrides.

Ah, it just appended to the styles rather than replacing them before.

Can you point me to where the Duotone filters are located?

The duotone hook is here

gutenberg/lib/block-supports/duotone.php

Line 54 in b057776

add_filter( 'block_editor_settings_all', array( 'WP_Duotone_Gutenberg', 'add_editor_settings' ), 10 );

Happy to help with the fix.

Thanks! I'll probably push up a PR sometime today and add you as a reviewer. It sounds like it should be safe to lower the priority to allow for the extensibility that now comes from within the plugin.

@oandregal, any thoughts on lowering the priority for the plugin overrides?

Opened #50760. I found and fixed a few other things when I was digging through all of the code that used the same hook.

Mamaduka added [Type] Regression Related to a regression in the latest release Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels May 4, 2023

Mamaduka self-assigned this May 4, 2023

Mamaduka mentioned this pull request May 4, 2023

Update the Gutenberg plugin to require at least the WP 6.1 version #50079

Merged

gziolo added the Gutenberg Plugin Issues or PRs related to Gutenberg Plugin management related efforts label May 5, 2023

ramonjd mentioned this pull request May 5, 2023

Add block spacing and layout to Post Template #49050

Merged

tellthemachines reviewed May 5, 2023

View reviewed changes

Mamaduka force-pushed the fix/enqueue-global-styles branch from 6a605a0 to eec751a Compare May 5, 2023 05:15

Mamaduka marked this pull request as ready for review May 5, 2023 05:18

Mamaduka requested a review from spacedmonkey as a code owner May 5, 2023 05:18

Mamaduka commented May 5, 2023

View reviewed changes

lib/block-editor-settings.php Outdated Show resolved Hide resolved

oandregal reviewed May 5, 2023

View reviewed changes

lib/compat/wordpress-6.2/block-editor-settings.php Outdated Show resolved Hide resolved

oandregal reviewed May 5, 2023

View reviewed changes

lib/compat/wordpress-6.2/script-loader.php Outdated Show resolved Hide resolved

oandregal reviewed May 5, 2023

View reviewed changes

lib/script-loader.php Show resolved Hide resolved

Mamaduka force-pushed the fix/enqueue-global-styles branch from 0d48ea4 to f95f040 Compare May 9, 2023 17:04

aaronrobertshaw mentioned this pull request May 11, 2023

[Selectors API] Does not affect block styles, global styles works.. but only on the frontend #50416

Closed

aaronrobertshaw reviewed May 11, 2023

View reviewed changes

Plugin: Use bundled files for enqueuing global styles

16ca4ac

oandregal reviewed May 11, 2023

View reviewed changes

oandregal approved these changes May 11, 2023

View reviewed changes

Mamaduka merged commit 0f56002 into trunk May 11, 2023

Mamaduka deleted the fix/enqueue-global-styles branch May 11, 2023 16:25

github-actions bot added this to the Gutenberg 15.9 milestone May 11, 2023

tellthemachines mentioned this pull request May 12, 2023

Make grid layout Group variation an experiment. #49359

Merged

fabiankaegy added the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label May 12, 2023

fabiankaegy removed the Backport to Gutenberg RC Pull request that needs to be backported to a Gutenberg release candidate (RC) label May 12, 2023

Mamaduka modified the milestones: Gutenberg 15.9, Gutenberg 15.8 May 12, 2023

This was referenced May 12, 2023

Move gutenberg_get_global_styles function #50596

Merged

Move gutenberg_get_remote_theme_patterns #50597

Merged

ajlende reviewed May 17, 2023

View reviewed changes

Mamaduka mentioned this pull request May 19, 2023

Fix gutenberg_get_block_editor_settings overriding other hooks #50760

Merged

ramonjd mentioned this pull request Jun 5, 2023

Plugin: Synchronize PHP changes for WordPress 6.3 release #51077

Closed

ramonjd added the Needs PHP backport Needs PHP backport to Core label Jun 5, 2023

ramonjd removed the Needs PHP backport Needs PHP backport to Core label Jun 30, 2023

This was referenced Jul 2, 2023

gutenberg_add_global_styles_for_blocks should add all global styles. #52228

Closed

Block specific elements styles are not applied for non-core block plugins #52276

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Plugin: Use bundled files for enqueuing global styles #50310

Plugin: Use bundled files for enqueuing global styles #50310

Mamaduka commented May 4, 2023 •

edited

Loading

Mamaduka commented May 4, 2023

tellthemachines left a comment

Mamaduka commented May 5, 2023 •

edited

Loading

oandregal commented May 5, 2023

Mamaduka commented May 5, 2023

oandregal commented May 5, 2023

Mamaduka commented May 5, 2023

oandregal May 5, 2023 •

edited

Loading

Mamaduka May 9, 2023

Mamaduka commented May 9, 2023

github-actions bot commented May 9, 2023

Mamaduka commented May 10, 2023

tellthemachines commented May 10, 2023

Mamaduka commented May 10, 2023

aaronrobertshaw left a comment

oandregal May 11, 2023

oandregal May 11, 2023

oandregal May 11, 2023

oandregal May 11, 2023

oandregal May 11, 2023

oandregal May 11, 2023

oandregal left a comment •

edited

Loading

Mamaduka commented May 11, 2023

Mamaduka commented May 12, 2023

fabiankaegy commented May 12, 2023

oandregal commented May 12, 2023

ajlende May 17, 2023

Mamaduka May 18, 2023

ajlende May 18, 2023

Mamaduka May 18, 2023

ajlende May 19, 2023

Plugin: Use bundled files for enqueuing global styles #50310

Plugin: Use bundled files for enqueuing global styles #50310

Conversation

Mamaduka commented May 4, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

Mamaduka commented May 4, 2023

tellthemachines left a comment

Choose a reason for hiding this comment

Mamaduka commented May 5, 2023 • edited Loading

oandregal commented May 5, 2023

Mamaduka commented May 5, 2023

oandregal commented May 5, 2023

Mamaduka commented May 5, 2023

oandregal May 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mamaduka commented May 9, 2023

github-actions bot commented May 9, 2023

Mamaduka commented May 10, 2023

tellthemachines commented May 10, 2023

Mamaduka commented May 10, 2023

aaronrobertshaw left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oandregal left a comment • edited Loading

Choose a reason for hiding this comment

Mamaduka commented May 11, 2023

Mamaduka commented May 12, 2023

fabiankaegy commented May 12, 2023

oandregal commented May 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Mamaduka commented May 4, 2023 •

edited

Loading

Mamaduka commented May 5, 2023 •

edited

Loading

oandregal May 5, 2023 •

edited

Loading

oandregal left a comment •

edited

Loading