Global Styles: Add Global Styles CSS to inline block CSS #40513

scruffian · 2022-04-21T11:07:23Z

What?

This is an alternative to #39571. We should only load the Global Styles CSS for blocks that are used on the page, rather than loading it on every page as we do presently.

Why?

This means sending less data on each page load, so it should improve performance. This is is a pre-cursor to #34180, which will add a lot more CSS to each page load unless we improve the performance.

How?

The basic premise is that if we are loading separate block assets, we move the block specific Global Styles CSS from the global-styles inline CSS block to the inline CSS portion of the block's CSS. The loading of this inline CSS is handled by wp_common_block_scripts_and_styles which enqueues the inline CSS for each block, so this just builds on top of that existing logic.

The changes we need to make are:

Don't add block specific CSS to the global styles inline CSS
Add add the block specific CSS to the individual block styles inline CSS

The difficult part of this is that much of the code is duplicated from Gutenberg/core with only a few modifications. I have tried to add comments to show where the modifications are, but it would be great if we could find a way to achieve this kind of change without copying large chunks of code.

Testing Instructions

Switch to TT2
Go to a page without a quote block on
Check that this CSS isn't in either the global-styles-inline-css or wp-block-quote-css:
.wp-block-quote {
border-width: 1px;
}
Go to a page with a quote block on
Check that this CSS is not in the global-styles-inline-css but is in wp-block-quote-css:
.wp-block-quote {
border-width: 1px;
}

Note

The CSS for the blocks is doubled up - this is because its being enqueued in both core and by Gutenberg. This happens in trunk.

@WordPress/block-themers

scruffian · 2022-04-21T13:33:59Z

lib/compat/wordpress-6.0/class-wp-theme-json-6-0.php

@@ -14,7 +14,7 @@
 *
 * @access private
 */
-class WP_Theme_JSON_Gutenberg extends WP_Theme_JSON_5_9 {
+class WP_Theme_JSON_6_0 extends WP_Theme_JSON_5_9 {


I think we should settle on a convention to only use the _Gutenberg suffix in the experimental folder, and all version specific classes should use the version suffix. That way if we want to inherit all the features in the plugin we don't need to specify the version of the class, we can just always use the _Gutenberg suffix. If we don't do this then we need to copy/paste even more code!

scruffian · 2022-04-21T13:36:07Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ }
+
+ if ( in_array( 'styles', $types, true ) ) {
+ $stylesheet .= $this->get_block_classes( $style_nodes_without_blocks );


This is really the only change to this function, plus the lines that define this variable. The purpose is to ensure that we are only adding top-level CSS to the Global Styles block, rather than block specific CSS which is added elsewhere.

scruffian · 2022-04-21T13:36:29Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+
+ $blocks_metadata = static::get_blocks_metadata();
+ $separate_block_assets = wp_should_load_separate_core_block_assets();
+ $style_nodes_without_blocks = static::get_style_nodes( $this->theme_json, $blocks_metadata, $separate_block_assets );


Should be called $style_nodes_maybe_without_blocks, maybe?

Or let the third argument do all the explaining? E.g., $should_exclude_block_nodes

$should_exclude_block_nodes = wp_should_load_separate_core_block_assets(); $style_nodes = static::get_style_nodes( $this->theme_json, $blocks_metadata, $should_exclude_block_nodes );

scruffian · 2022-04-21T13:37:59Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ * @param array $exclude_blocks Exclude blocks from the style nodes.
+ * @return array
+ */
+ protected static function get_style_nodes( $theme_json, $selectors = array(), $exclude_blocks = false ) {


The exclude blocks parameter is new and is used to determine whether to get all style nodes, or only the top level and elements nodes. We might prefer to create three new functions, one to get blocks, one to get top level and one for elements, and then combine them as needed.

scruffian · 2022-04-21T13:38:18Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ }
+ }
+
+ if ( $exclude_blocks ) {


This is the addition.

scruffian · 2022-04-21T13:38:54Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ return $nodes;
+ }
+
+ $nodes = array_merge( $nodes, static::get_block_nodes_private( $theme_json ) );


This is a refactor, moving this part of the code into a new function so it can be shared.

scruffian · 2022-04-21T13:39:27Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ return $nodes;
+ }
+
+ public function get_block_nodes() {


This is a wrapper so that we can call this function as a method on the class as well as statically. I'm not sure this is a great idea.

Hmmm, yeah I see we use exclusively public functions for the API, e.g., $tree->get_stylesheet() but we need to call the static function static::get_block_nodes_private() given to the get_style_nodes() is protected static (can't call public/private members).

I'm not sure there's an easy approach without a major refactor.

scruffian · 2022-04-21T13:39:38Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ return static::get_block_nodes_private( $this->theme_json );
+ }
+
+ private static function get_block_nodes_private( $theme_json ) {


This needs a better name!

This isn't new code, it's just extracted from get_style_nodes.

get_block_style_nodes ? 😄

scruffian · 2022-04-21T13:41:16Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ *
+ * @return array Styles for the block.
+ */
+ public function get_styles_for_block( $block_metadata ) {


This is a new function! It gets the Global Styles CSS for one block, which can then be added to that block's inline CSS.

scruffian · 2022-04-21T13:41:44Z

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

+ *
+ * @return string Stylesheet.
+ */
+function wp_get_global_stylesheet_gutenberg( $types = array() ) {


This function is almost a copy of what we have in core

Is this any different to gutenberg_get_global_stylesheet() in lib/compat/wordpress-6.0/get-global-styles-and-settings.php ?

I think the general approach is to copy that function over to the new compat file and keep the gutenberg_ naming convention.

scruffian · 2022-04-21T13:42:27Z

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

+ }
+ }
+
+ $tree = WP_Theme_JSON_Resolver_Gutenberg::get_merged_data();


This (and the line below) are the only differences - we need to call the _Gutenberg version of the class.

scruffian · 2022-04-21T13:43:05Z

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

+/**
+ * Adds global style rules to the inline style for each block.
+ */
+function wp_add_global_styles_for_blocks() {


This is also a new function. It fetches the CSS for the block and adds it to the inline CSS block.

scruffian · 2022-04-21T13:43:26Z

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

+ *
+ * @since 5.8.0
+ */
+function gutenberg_enqueue_global_styles() {


This is a copy of wp_enqueue_global_styles.

scruffian · 2022-04-21T13:44:08Z

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

+ return;
+ }
+
+ if ( $separate_assets ) {


This is the only change - if we are loading separate block assets then we should include the Global Styles CSS in the block CSS rather than in the global-styles one.

scruffian · 2022-04-21T13:44:46Z

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

+remove_action( 'wp_footer', 'gutenberg_enqueue_global_styles_assets' );
+
+add_action( 'wp_enqueue_scripts', 'gutenberg_enqueue_global_styles' );
+add_action( 'wp_footer', 'gutenberg_enqueue_global_styles', 1 );


We need to remove all the other times when we do this, so that we don't end up with many copies of the same styles.

scruffian · 2022-04-21T13:45:25Z

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

+ *
+ * @access private
+ */
+class WP_Theme_JSON_Gutenberg extends WP_Theme_JSON_6_1 {


This enables us to always reference WP_Theme_JSON_Gutenberg in experimental libs, rather than targetting a specific version.

ramonjd · 2022-04-27T03:38:25Z

This is working well for me. Using emptytheme it prevents the global styles for individual blocks from loading on every page, e.g., on trunk this css is loaded for every page regardless of whether a Quote Block appears on the page.

.wp-block-quote {
	background-color: var(--wp--preset--color--luminous-vivid-amber);
}

The code looks in good shape to me, but I'd be interested to hear from folks with more theme.json fu about implementation.

As for the implications for

Theme JSON: use block global style for block gap where no gap value exists in layout.php #39870 (comment)

I can't quite picture any overlap at this stage, since I haven't yet considered the idea of block partials. Even if there were, this PR appears seems to have more impact so I'd try to work around any changes introduced here.

oandregal

Hey, I've taken a look at what's the problem we want to solve here. It seems that the piece we're missing is the ability to filter styles coming from theme.json, as in:

theme.json > filter > stylesheet

There's a few approaches here. One would be to filter the actual theme.json structure before passing it to the WP_Theme_JSON (remove the blocks not to render). Another that seems simpler is to tap into get_blocks_metadata: this function is the responsible to provide the list of blocks. Can we somehow modify its behavior so it only returns a list of "allowed blocks"? Throwing some quick ideas:

by adding a filter to that function so consumers can modify the block list
by passing the WP_Theme_JSON class a subset of the registered block names (for example, in the constructor) that this function can access

What do you think of this approach?

In line with "filtering styles", we should also consider a related problem: how can we make it so the preset classes rendered are only the ones in use? Not that we need to solve both things at once, but keeping it mind for later would help. For example, consider that, in addition to a list of "allowed blocks", we'll need a list of "allowed preset" later.

Other thoughts:

Do we need to actually modify where the styles go? Or can we still use the global stylesheet just with a reduced set of styles instead of all of them?
Can we make it so the logic for either loading all the styles or only the styles for the blocks in use is shared between global styles & should_load_separate_block_assets?

oandregal · 2022-04-27T12:55:10Z

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

+ }
+
+ $blocks_metadata = static::get_blocks_metadata();
+ $separate_block_assets = wp_should_load_separate_core_block_assets();


This is a bit of a red flag for me. This class was designed to avoid having "global state from WordPress" here: things like "has theme support" or "is this a classic or a block theme", etc.

In my view, this class should be focused on low-level primitives (transform a theme.json into a stylesheet, filtering, etc) and the "domain logic" should go elsewhere (resolver or the higher-level global styles functions). Things like testing or continue to modify its behavior is a lot harder if we don't.

scruffian · 2022-04-27T22:44:02Z

Thanks for the review!

Can we somehow modify its behavior so it only returns a list of "allowed blocks"? Throwing some quick ideas:

This seems to push the problem to a different place, but doesn't really solve it. We would need to make Global Styles aware of which blocks are used on the page, which is a pretty difficult thing to do, and potentially has the same issues you raised above about making Global Styles aware of things that aren't its concern.

I do think that with block partials on the way it will be helpful for theme.json styles to be output in the inline CSS for the block, and also I think that's easier to read and understand.

Having said all that I can see that the current approach does violate the idea of keeping Global Styles ignorant of the implementation so I'll see if I can work around that and if not then I'll try some of your ideas!

scruffian · 2022-05-19T14:15:52Z

Closing in favour of #41160

scruffian added 4 commits April 21, 2022 12:06

Global Style: Add Global Styles CSS to inline block CSS

7e6a8f6

remove unneeded code

51a3b43

remove old comments

109cac5

remove old comments

ee5a64a

scruffian commented Apr 21, 2022

View reviewed changes

lib/compat/wordpress-6.1/class-wp-theme-json-6-1.php

}

}

if ( $exclude_blocks ) {

Copy link

Contributor Author

scruffian Apr 21, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the addition.

scruffian commented Apr 21, 2022

View reviewed changes

scruffian self-assigned this Apr 21, 2022

scruffian added [Feature] Themes Questions or issues with incorporating or styling blocks in a theme. Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json labels Apr 21, 2022

linting fixes

a4e5e02

This was referenced Apr 21, 2022

Global Styles: Add Global styles to inline block styles #39571

Closed

Theme.json: Block specific CSS is printed even if a block is not used on the current page #31642

Closed

scruffian changed the title ~~Global Style: Add Global Styles CSS to inline block CSS~~ Global Styles: Add Global Styles CSS to inline block CSS Apr 21, 2022

scruffian marked this pull request as ready for review April 21, 2022 14:44

scruffian requested review from TimothyBJacobs and spacedmonkey as code owners April 21, 2022 14:44

scruffian requested review from adamziel and draganescu April 21, 2022 14:44

carolinan requested a review from aristath April 26, 2022 12:53

scruffian mentioned this pull request Apr 26, 2022

Theme JSON: use block global style for block gap where no gap value exists in layout.php #39870

Closed

scruffian requested a review from youknowriad April 27, 2022 08:35

youknowriad requested a review from oandregal April 27, 2022 08:38

oandregal reviewed Apr 27, 2022

View reviewed changes

oandregal requested a review from jorgefilipecosta April 27, 2022 13:16

scruffian mentioned this pull request Apr 29, 2022

Add support for button elements to theme.json #40260

Merged

scruffian mentioned this pull request May 19, 2022

Global Styles: Load block CSS conditionally #41160

Merged

scruffian closed this May 19, 2022

scruffian deleted the add/global-styles-to-block-css branch May 19, 2022 14:16

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Global Styles: Add Global Styles CSS to inline block CSS #40513

Global Styles: Add Global Styles CSS to inline block CSS #40513

scruffian commented Apr 21, 2022 •

edited

Loading

scruffian Apr 21, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

ramonjd Apr 27, 2022 •

edited

Loading

scruffian Apr 21, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022 •

edited

Loading

ramonjd Apr 27, 2022

scruffian Apr 21, 2022 •

edited

Loading

ramonjd Apr 27, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

ramonjd Apr 27, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

scruffian Apr 21, 2022

ramonjd commented Apr 27, 2022

oandregal left a comment

oandregal Apr 27, 2022

scruffian commented Apr 27, 2022

scruffian commented May 19, 2022

Global Styles: Add Global Styles CSS to inline block CSS #40513

Global Styles: Add Global Styles CSS to inline block CSS #40513

Conversation

scruffian commented Apr 21, 2022 • edited Loading

What?

Why?

How?

Testing Instructions

Note

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramonjd Apr 27, 2022 • edited Loading

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

scruffian Apr 21, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scruffian Apr 21, 2022 • edited Loading

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

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

ramonjd commented Apr 27, 2022

oandregal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scruffian commented Apr 27, 2022

scruffian commented May 19, 2022

scruffian commented Apr 21, 2022 •

edited

Loading

ramonjd Apr 27, 2022 •

edited

Loading

scruffian Apr 21, 2022 •

edited

Loading

scruffian Apr 21, 2022 •

edited

Loading