Move custom css to its own inline style generation method and combine with customizer CSS #47396

glendaviesnz · 2023-01-25T01:03:44Z

What?

Moves global styles custom CSS to its own methods so its position in the output can be modified independently to the other global styles

Why?

This allows the global styles custom CSS to be loaded after any Customizer custom CSS that may have been added in hybrid themes, otherwise this CSS has higher specificity.

How?

Extract the loading of custom CSS into its own method, separate to the other global styles, and include any customizer CSS first in this output.

Testing Instructions

Add some custom CSS in global styles, including some block level custom CSS
Load in front end and make sure styles appear at the bottom of the global-styles-inline-css style element
In a hybrid theme like BlocksBase also add some custom CSS in Customizer and make sure this CSS loads before the Global Style custom CSS in the global-styles-inline-css style element

Screenshots or screencast

Before:

After:

glendaviesnz · 2023-01-25T01:07:12Z

@Mamaduka some of the feedback from custom CSS testing was that it is loading in a different place to the customizer custom CSS.

Getting it to the bottom of the header would require pulling it out into its own method - something like in this draft PR.

What are your thoughts on this? Can you think of a better way?

A couple of things to think about if we go with this approach:

Do we also pull the block specific custom CSS into this?
Do we need to worry about caching this?

github-actions · 2023-01-25T01:20:13Z

Flaky tests detected in 3b44904.
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/4053838918
📝 Reported issues:

[Flaky Test] should not allow blocks to be inserted in inner blocks #44341 in specs/editor/plugins/cpt-locking.test.js

Mamaduka · 2023-01-25T05:14:10Z

@glendaviesnz, it would be hard to ensure that Custom CSS styles are always loaded last. In block themes, global styles are loaded in the footer, so Custom CSS loaded in the header can be overridden by global styles.

@aristath, you've worked on style loading before. Do you have any suggestions?

glendaviesnz · 2023-01-25T07:58:23Z

it would be hard to ensure that Custom CSS styles are always loaded last. In block themes, global styles are loaded in the footer, so Custom CSS loaded in the header can be overridden by global styles.

@Mamaduka I probably wasn't clear in my description, ie. rather than ensuring it loads 'absolutely last' the intention is to at least make it load after the majority of other core styles, and particularly to load after the existing customizer custom CSS so it overrides any custom CSS that may have been added there in hybrid themes. The priority of 102 is one lower than the customizer CSS.

I think this is the best we can do, or should try to do. If people still have any problems with the specificity of custom CSS they should just increase the specificity of the selectors they are using.

Mamaduka · 2023-01-25T08:06:40Z

ie. rather than ensuring it loads 'absolutely last' the intention is to at least make it load after the majority of other core styles.

Right, that should be our goal.

Currently, hybrid themes shouldn't have access to global styles. How can add "Additional CSS" in Global Styles that conflicts with the customizer?

glendaviesnz · 2023-01-25T08:16:58Z

Currently, hybrid themes shouldn't have access to global styles. How can add "Additional CSS" in Global Styles that conflicts with the customizer?

I was surprised to see that hybrid themes, like BlockBase, get both Customizer and Global styles access. Is that not supposed to be the case? If not then we probably need another PR to sort that as a separate issue to the load order.

Mamaduka · 2023-01-25T08:26:16Z

BlockBase is a block theme that uses Customizer, but I don't know if I would call it a hybrid theme. This is the line that enables Customizer backward compatibility - https://github.com/Automattic/themes/blob/trunk/blockbase/inc/customizer/wp-customize-colors.php#L8.

Personally, I think these are rare cases, and we should discourage using both custom CSS options together.

glendaviesnz · 2023-01-25T08:36:35Z

Personally, I think these are rare cases, and we should discourage using both custom CSS options together.

Yeh, in that case probably not worth worrying about anything for 6.2 at least - there is an easy fix for someone confused about the interaction of styles in both places, ie. just use one or the other.

carolinan · 2023-01-25T12:01:13Z

It is not that easy to discourage use of both customizer and the site editor. Many plugins add options that enables the Customizer, (and they have to, if they need their options to work with both types of themes).

Mamaduka · 2023-01-25T13:59:42Z

@carolinan, that's true. But if you use both Customizer and Editor Custom CSS settings, one of them will be broken.

carolinan · 2023-01-25T17:19:35Z

I wish we could find a way for them to match before it is too late and people have CSS in both places.

richtabor · 2023-01-25T19:48:23Z

It is not that easy to discourage use of both customizer and the site editor.

This is more about discouraging the use of custom CSS in both places.

But if you use both Customizer and Editor Custom CSS settings, one of them will be broken.

What's broken? Is it that both could have competing CSS, where one would overwrite the other?

If we think it's a big enough problem, we could add a reference to the new Additional CSS panel in the Site Editor within the Customizer, though I'm not sure how necessary it is:

…rder to get the output in the correct place after the customizer css

glendaviesnz · 2023-01-25T22:30:17Z

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

+	$custom_css     = get_global_styles_custom_css();
+	$is_block_theme = wp_is_block_theme();
+	if ( $custom_css && $is_block_theme ) {
+		?>


Outputting the style element here, instead of using wp_enqueue_style was the only way I could get this to load in the header after the customizer custom CSS - there may be a better approach that I am overlooking

glendaviesnz · 2023-01-25T22:41:31Z

@Mamaduka, @carolinan - this PR now pulls out global and block-level custom CSS into its own style element at the end of the header, so it now matches the placement of the customizer custom CSS.

Mamaduka · 2023-01-26T06:24:19Z

What's broken? Is it that both could have competing CSS, where one would overwrite the other?

Correct. What do we gain from fixing this when the user still can have competing CSS while using both Additional CSS settings?

Should we shelve this issue until WP 6.3 and see the feedback?

glendaviesnz · 2023-01-26T07:31:23Z

Should we shelve this issue until WP 6.3 and see the feedback?

@Mamaduka This PR is more about making sure that the new global styles custom CSS is loaded in a similar position in the header as the customizer CSS so that it behaves in a similar way for anyone that is familiar with the customizer, so I think this is worth getting into 6.2.

I agree though that we shouldn't worry about trying to resolve the fact that some themes/plugins might end up with customizer and site editor custom CSS inputs for 6.2, that can wait until after 6.2 and see what feedback there is in relation to that I think.

aristath · 2023-01-26T07:42:30Z

Regarding the conflict between custom-css in the customizer and global styles:
It's not an insurmountable problem. It's easy to get the custom-css from the customizer, inject it in global-styles (to solve any backwards-compatibility issues in case the user has added CSS in both places), and then hide the customizer option.
The code below does just that (tested and confirmed)

add_action( 'wp', function() {

	// Bail early if not a block theme.
	if ( ! wp_is_block_theme() ) {
		return;
	}

	// Get the custom CSS from the custom-css post (legacy, customizer).
	$custom_css = wp_get_custom_css();

	// Bail early if there is no custom CSS.
	if ( empty( $custom_css ) ) {
		return;
	}

	// Get the global-styles post.
	$global_styles_post = WP_Theme_JSON_Resolver::get_user_data_from_wp_global_styles( wp_get_theme(), true );
	$global_styles      = json_decode( $global_styles_post['post_content'], true );

	// Ensure we have styles.css in global-styles.
	$global_styles['styles']        = ( empty( $global_styles['styles'] ) ) ? array() : $global_styles['styles'];
	$global_styles['styles']['css'] = ( empty( $global_styles['styles']['css'] ) ) ? '' : $global_styles['styles']['css'];

	// Append the customizer custom-css to the global-styles custom-css.
	$global_styles['styles']['css'] .= $custom_css;

	// Update global-styles.
	wp_update_post( array(
		'ID'           => $global_styles_post['ID'],
		'post_content' => wp_json_encode( $global_styles ),
	) );

	// Remove custom-css.
	wp_update_custom_css_post( '' );
} );

// Remove the "Custom CSS" section from the customizer.
add_action( 'customize_register', function( $wp_customize ) {
	$wp_customize->remove_section( 'custom_css' );
}, 999 );

Notes: The above code runs on the wp action which is wrong as it requires us to visit the frontend in order for the migration to run (it doesn't run in the site-editor). But that's a minor thing we can easily solve with some experimentation.

The code above does the following:

Check if it's a block theme. If not, don't do anything.
Check if we have legacy custom-css in the customizer. If not, don't do anything.
Append the legacy custom-css to the global-styles custom-css
Delete the legacy custom-css
Remove the custom-css section from the customizer.

It's simple and it works.

glendaviesnz · 2023-01-26T07:47:47Z

Thanks @aristath - that looks pretty straightforward. I don't have a strong opinion on whether we try to get something like that into 6.2, but either way, I think we should keep it separate from this PR.

aristath · 2023-01-26T07:51:11Z

Agreed. Just pointing out that we should not consider the customizer legacy implementation a blocker for any improvements we make. These problems can be solved with some additional code 👍

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

Mamaduka · 2023-01-26T08:23:11Z

@glendaviesnz, I've got an alternative idea inspired by @aristath's comment. I will try to test it and share it as soon as possible.

glendaviesnz · 2023-01-29T21:10:33Z

@Mamaduka how did you get on with your alternative PR for this? If that is not ready yet should we go with this approach for 15.1/6.2 and follow up with your one later?

Mamaduka · 2023-01-30T13:07:20Z

Sorry for the delay, @glendaviesnz. Here's my alternative method; it can be directly merged into this PR - #47554.

* Update enqueuing method * Minor updates to get_global_styles_custom_css * Always return string

glendaviesnz · 2023-01-31T21:23:11Z

Thanks @Mamaduka & @aristath for merging that change, that is a better approach I think. This is testing well for me now and global styles custom CSS is output after the other global styles and after any customizer custom CSS.

apeatling

Confirmed that:

✅ Styles in the block editor added to the site level appear at the bottom of the inlineglobal-styles-inline-css.
✅ Styles in the block editor added to the block level appear at the bottom of the inline global-styles-inline-css.
✅ Styles added in the Customizer appear before the styles added in the block editor at the bottom of global-styles-inline-css when using the Blockbase theme.

In block editor:

In Customizer:

Bottom of global-styles-inline-css in Blockbase theme:

Move custom css to its on inline style at bottom of header

c7787b0

glendaviesnz self-assigned this Jan 25, 2023

glendaviesnz added the Global Styles Anything related to the broader Global Styles efforts, including Styles Engine and theme.json label Jan 25, 2023

Fix linting issues

c2ecf73

richtabor mentioned this pull request Jan 25, 2023

Global Styles: an input for custom CSS #30142

Closed

10 tasks

glendaviesnz added 2 commits January 26, 2023 09:52

Also move block custom CSS to separate stylesheet

d8debd6

Update test

f0597dc

glendaviesnz marked this pull request as ready for review January 25, 2023 21:42

glendaviesnz requested a review from spacedmonkey as a code owner January 25, 2023 21:42

Switch from wp_enqueue_style to just outputting the style tags in o…

f67ba8c

…rder to get the output in the correct place after the customizer css

glendaviesnz commented Jan 25, 2023

View reviewed changes

glendaviesnz requested review from Mamaduka and carolinan January 25, 2023 22:39

aristath reviewed Jan 26, 2023

View reviewed changes

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

Move to 6.2 script-loader compat file

317c39e

glendaviesnz mentioned this pull request Jan 26, 2023

Backport global styles custom css changes WordPress/wordpress-develop#3896

Closed

Mamaduka mentioned this pull request Jan 30, 2023

Global Styles: Alternative method for enqueueing custom CSS #47554

Merged

Global Styles: Alternative method for enqueueing custom CSS (#47554)

3b44904

* Update enqueuing method * Minor updates to get_global_styles_custom_css * Always return string

glendaviesnz changed the title ~~Move custom css to its own inline style at bottom of header~~ Move custom css to its own inline style generation method and combine with customizer CSS Jan 31, 2023

apeatling self-requested a review January 31, 2023 22:05

apeatling approved these changes Jan 31, 2023

View reviewed changes

glendaviesnz merged commit bb9b5c2 into trunk Jan 31, 2023

glendaviesnz deleted the update/custom-css-load-order branch January 31, 2023 22:41

github-actions bot added this to the Gutenberg 15.1 milestone Jan 31, 2023

juanmaguitar added the [Type] Enhancement A suggestion for improvement. label Feb 1, 2023

femkreations added the Needs User Documentation Needs new user documentation label Feb 3, 2023

femkreations mentioned this pull request Feb 27, 2023

[HelpHub] Global Styles WordPress/Documentation-Issue-Tracker#675

Closed

34 tasks

tellthemachines mentioned this pull request Feb 19, 2024

Enqueue block custom CSS only when block renders on the page. #58991

Merged

tellthemachines mentioned this pull request Jun 6, 2024

Update custom CSS handling to be consistent with block global styles. #62357

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Move custom css to its own inline style generation method and combine with customizer CSS #47396

Move custom css to its own inline style generation method and combine with customizer CSS #47396

glendaviesnz commented Jan 25, 2023 •

edited

Loading

glendaviesnz commented Jan 25, 2023 •

edited

Loading

github-actions bot commented Jan 25, 2023 •

edited

Loading

Mamaduka commented Jan 25, 2023

glendaviesnz commented Jan 25, 2023 •

edited

Loading

Mamaduka commented Jan 25, 2023

glendaviesnz commented Jan 25, 2023

Mamaduka commented Jan 25, 2023

glendaviesnz commented Jan 25, 2023

carolinan commented Jan 25, 2023 •

edited

Loading

Mamaduka commented Jan 25, 2023

carolinan commented Jan 25, 2023

richtabor commented Jan 25, 2023

glendaviesnz Jan 25, 2023

glendaviesnz commented Jan 25, 2023

Mamaduka commented Jan 26, 2023

glendaviesnz commented Jan 26, 2023

aristath commented Jan 26, 2023

glendaviesnz commented Jan 26, 2023

aristath commented Jan 26, 2023 •

edited

Loading

Mamaduka commented Jan 26, 2023

glendaviesnz commented Jan 29, 2023

Mamaduka commented Jan 30, 2023

glendaviesnz commented Jan 31, 2023

apeatling left a comment •

edited

Loading

Move custom css to its own inline style generation method and combine with customizer CSS #47396

Move custom css to its own inline style generation method and combine with customizer CSS #47396

Conversation

glendaviesnz commented Jan 25, 2023 • edited Loading

What?

Why?

How?

Testing Instructions

Screenshots or screencast

glendaviesnz commented Jan 25, 2023 • edited Loading

github-actions bot commented Jan 25, 2023 • edited Loading

Mamaduka commented Jan 25, 2023

glendaviesnz commented Jan 25, 2023 • edited Loading

Mamaduka commented Jan 25, 2023

glendaviesnz commented Jan 25, 2023

Mamaduka commented Jan 25, 2023

glendaviesnz commented Jan 25, 2023

carolinan commented Jan 25, 2023 • edited Loading

Mamaduka commented Jan 25, 2023

carolinan commented Jan 25, 2023

richtabor commented Jan 25, 2023

glendaviesnz Jan 25, 2023

Choose a reason for hiding this comment

glendaviesnz commented Jan 25, 2023

Mamaduka commented Jan 26, 2023

glendaviesnz commented Jan 26, 2023

aristath commented Jan 26, 2023

glendaviesnz commented Jan 26, 2023

aristath commented Jan 26, 2023 • edited Loading

Mamaduka commented Jan 26, 2023

glendaviesnz commented Jan 29, 2023

Mamaduka commented Jan 30, 2023

glendaviesnz commented Jan 31, 2023

apeatling left a comment • edited Loading

Choose a reason for hiding this comment

glendaviesnz commented Jan 25, 2023 •

edited

Loading

glendaviesnz commented Jan 25, 2023 •

edited

Loading

github-actions bot commented Jan 25, 2023 •

edited

Loading

glendaviesnz commented Jan 25, 2023 •

edited

Loading

carolinan commented Jan 25, 2023 •

edited

Loading

aristath commented Jan 26, 2023 •

edited

Loading

apeatling left a comment •

edited

Loading