Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 5 commits into from
Jun 13, 2024
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
453c9e6
Update custom CSS handling to be consistent with block global styles.
tellthemachines Jun 6, 2024
6db7830
Deprecate functions and update tests.
tellthemachines Jun 7, 2024
000d1c6
Add backport ref
tellthemachines Jun 7, 2024
01a0e42
Don't output root custom CSS in `get_styles_for_block`
tellthemachines Jun 11, 2024
2dbc8aa
Update deprecated function docblocks.
tellthemachines Jun 13, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions backport-changelog/6.7/6750.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
https://github.com/WordPress/wordpress-develop/pull/6750

* https://github.com/WordPress/gutenberg/pull/62357
2 changes: 1 addition & 1 deletion lib/block-editor-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ function gutenberg_get_block_editor_settings( $settings ) {
* entered by users does not break other global styles.
*/
$global_styles[] = array(
'css' => gutenberg_get_global_styles_custom_css(),
'css' => gutenberg_get_global_stylesheet( array( 'custom-css' ) ),
'__unstableType' => 'user',
'isGlobalStyles' => true,
);
Expand Down
26 changes: 20 additions & 6 deletions lib/class-wp-theme-json-gutenberg.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1355,6 +1355,12 @@ public function get_stylesheet( $types = array( 'variables', 'styles', 'presets'
$stylesheet .= $this->get_preset_classes( $setting_nodes, $origins );
}

// Load the custom CSS last so it has the highest specificity.
if ( in_array( 'custom-css', $types, true ) ) {
// Add the global styles root CSS.
$stylesheet .= _wp_array_get( $this->theme_json, array( 'styles', 'css' ) );
}

return $stylesheet;
}

Expand Down Expand Up @@ -1399,10 +1405,12 @@ protected function process_blocks_custom_css( $css, $selector ) {
* Returns the global styles custom css.
*
* @since 6.2.0
* @deprecated 6.7.0 Use {@see 'get_stylesheet'} instead.
*
* @return string The global styles custom CSS.
*/
public function get_custom_css() {
_deprecated_function( __METHOD__, '6.7.0', 'get_stylesheet' );
$block_custom_css = '';
$block_nodes = $this->get_block_custom_css_nodes();
foreach ( $block_nodes as $node ) {
Expand All @@ -1415,23 +1423,23 @@ public function get_custom_css() {

/**
* Returns the global styles base custom CSS.
*
* @since 6.6.0
* This function is deprecated; please do not sync to core.
*
* @return string The global styles base custom CSS.
*/
public function get_base_custom_css() {
Copy link
Member

Choose a reason for hiding this comment

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

So these never made it to Core, right? Should we remove the @since annotations and maybe add a "don't backport note"?

Doesn't have to be here, just wondering if it'll help later.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh good point, we should remove the @since on these!

_deprecated_function( __METHOD__, 'Gutenberg 18.6.0', 'get_stylesheet' );
return isset( $this->theme_json['styles']['css'] ) ? $this->theme_json['styles']['css'] : '';
}

/**
* Returns the block nodes with custom CSS.
*
* @since 6.6.0
* This function is deprecated; please do not sync to core.
*
* @return array The block nodes.
*/
public function get_block_custom_css_nodes() {
_deprecated_function( __METHOD__, 'Gutenberg 18.6.0', 'get_block_nodes' );
$block_nodes = array();

// Add the global styles block CSS.
Expand All @@ -1455,15 +1463,15 @@ public function get_block_custom_css_nodes() {

/**
* Returns the global styles custom CSS for a single block.
*
* @since 6.6.0
* This function is deprecated; please do not sync to core.
*
* @param array $css The block css node.
* @param string $selector The block selector.
*
* @return string The global styles custom CSS for the block.
*/
public function get_block_custom_css( $css, $selector ) {
_deprecated_function( __METHOD__, 'Gutenberg 18.6.0', 'get_styles_for_block' );
return $this->process_blocks_custom_css( $css, $selector );
}

Expand Down Expand Up @@ -2646,6 +2654,7 @@ private static function get_block_nodes( $theme_json, $selectors = array() ) {
'selectors' => $feature_selectors,
'duotone' => $duotone_selector,
'variations' => $variation_selectors,
'css' => $selector,
);

if ( isset( $theme_json['styles']['blocks'][ $name ]['elements'] ) ) {
Expand Down Expand Up @@ -2855,6 +2864,11 @@ static function ( $pseudo_selector ) use ( $selector ) {
$block_rules .= static::to_ruleset( ":root :where($style_variation_selector)", $individual_style_variation_declarations );
}

// 7. Generate and append any custom CSS rules.
if ( isset( $node['css'] ) && ! $is_root_selector ) {
$block_rules .= $this->process_blocks_custom_css( $node['css'], $selector );
Copy link
Member

Choose a reason for hiding this comment

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

I think this line allows adding custom CSS declarations to the root selector in theme.json

	"styles": {
		"css": "background-color:purple;"
	},

I can see the background color on the frontend but not in the site editor

Whereas in trunk only this works:

	"styles": {
		"css": "body{background-color:purple;}"
	},

Should Gutenberg allow both? Or is it a regression?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm that's a good question. This is how it was before #47396, but given that PR is over a year old it might be best to keep things as they were more recently. I'd expect to have to write complete rules in the top level custom CSS control and not necessarily for loose properties to be attached to the body element.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oooooh this is interesting; I think we've detected a bug in the existing behaviour. Testing in trunk, the loose properties are output outside any rule, so they don't apply because they're invalid, both in editor and front end:

Screenshot 2024-06-11 at 3 43 26 PM Screenshot 2024-06-11 at 3 47 01 PM

Whereas in this branch, those properties are nested under the root selector in the front end, though they're still invalid output in the editor. I think ideally we should fix things so they aren't output at all. I'll look into it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok I fixed things so that this branch behaves like trunk (no valid CSS output if only properties are added to the top level custom CSS control) but the loose properties are still being output (just like they are on trunk).

The problem with this is that any rule output straight after a loose property won't work. I can repro this in core trunk, and it's also a problem with the customizer additional CSS (which may not manifest in classic themes because additional CSS is enqueued after everything else, but on hybrid themes we're adding it before global styles custom CSS, so it can also break stuff)

We could maybe just check that there aren't any loose properties at the end of the custom CSS string? Checking the whole string is valid CSS might be overkill as site admins should be responsible for the validity of their CSS. We could also Do Nothing 😄 and leave things as they are.

If we do want to do anything though, it's probably better as a separate PR.

Copy link
Member

Choose a reason for hiding this comment

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

If we do want to do anything though, it's probably better as a separate PR.

For sure. Thanks for looking into it.

We could maybe just check that there aren't any loose properties at the end of the custom CSS string?

Or maybe some light-weight parsing where the styles are added?

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

Line 1361 in 01a0e42

$stylesheet .= _wp_array_get( $this->theme_json, array( 'styles', 'css' ) );

}

return $block_rules;
}

Expand Down
3 changes: 3 additions & 0 deletions lib/global-styles-and-settings.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,7 @@ function gutenberg_get_global_settings( $path = array(), $context = array() ) {
* @return string
*/
function gutenberg_get_global_styles_custom_css() {
_deprecated_function( __FUNCTION__, 'Gutenberg 18.6.0', 'gutenberg_get_global_stylesheet' );
// Ignore cache when `WP_DEBUG` is enabled, so it doesn't interfere with the theme developers workflow.
$can_use_cached = ! WP_DEBUG;
$cache_key = 'gutenberg_get_global_custom_css';
Expand Down Expand Up @@ -177,6 +178,7 @@ function gutenberg_get_global_styles_custom_css() {
* @return string The global base custom CSS.
*/
function gutenberg_get_global_styles_base_custom_css() {
_deprecated_function( __FUNCTION__, 'Gutenberg 18.6.0', 'gutenberg_get_global_stylesheet' );
if ( ! wp_theme_has_theme_json() ) {
return '';
}
Expand Down Expand Up @@ -211,6 +213,7 @@ function gutenberg_get_global_styles_base_custom_css() {
* @global WP_Styles $wp_styles
*/
function gutenberg_add_global_styles_block_custom_css() {
_deprecated_function( __FUNCTION__, 'Gutenberg 18.6.0', 'gutenberg_add_global_styles_for_blocks' );
global $wp_styles;

if ( ! wp_theme_has_theme_json() || ! wp_should_load_separate_core_block_assets() ) {
Expand Down
42 changes: 13 additions & 29 deletions lib/script-loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,45 +43,29 @@ function gutenberg_enqueue_global_styles() {
add_filter( 'wp_theme_json_get_style_nodes', 'wp_filter_out_block_nodes' );

$stylesheet = gutenberg_get_global_stylesheet();
if ( empty( $stylesheet ) ) {
return;
}

wp_register_style( 'global-styles', false );
wp_add_inline_style( 'global-styles', $stylesheet );
wp_enqueue_style( 'global-styles' );

// Add each block as an inline css.
gutenberg_add_global_styles_for_blocks();

/*
* Add the custom CSS for the global styles.
* Before that, dequeue the Customizer's custom CSS
* Dequeue the Customizer's custom CSS
* and add it before the global styles custom CSS.
* Don't enqueue Customizer's custom CSS separately.
*/
remove_action( 'wp_head', 'wp_custom_css_cb', 101 );
// Get the custom CSS from the Customizer and add it to the global stylesheet.
$custom_css = wp_get_custom_css();
$stylesheet .= $custom_css;

$custom_css = wp_get_custom_css();
// Add the global styles custom CSS at the end.
$stylesheet .= gutenberg_get_global_stylesheet( array( 'custom-css' ) );

if ( ! wp_should_load_separate_core_block_assets() ) {
/*
* If loading all block assets together, add both
* the base and block custom CSS at once. Else load
* the base custom CSS only, and the block custom CSS
* will be added to the inline CSS for each block in
* gutenberg_add_global_styles_block_custom_css().
*/
$custom_css .= gutenberg_get_global_styles_custom_css();
} else {
$custom_css .= gutenberg_get_global_styles_base_custom_css();
if ( empty( $stylesheet ) ) {
return;
}

if ( ! empty( $custom_css ) ) {
wp_add_inline_style( 'global-styles', $custom_css );
}
wp_register_style( 'global-styles', false );
wp_add_inline_style( 'global-styles', $stylesheet );
wp_enqueue_style( 'global-styles' );

gutenberg_add_global_styles_block_custom_css();
// Add each block as an inline css.
gutenberg_add_global_styles_for_blocks();
}
add_action( 'wp_enqueue_scripts', 'gutenberg_enqueue_global_styles' );
add_action( 'wp_footer', 'gutenberg_enqueue_global_styles', 1 );
Expand Down
36 changes: 32 additions & 4 deletions phpunit/class-wp-theme-json-test.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4850,12 +4850,31 @@ public function test_get_top_level_background_image_styles() {
$this->assertSame( $expected_styles, $theme_json->get_stylesheet(), 'Styles returned from "::get_stylesheet()" with top-level background image as string type does not match expectations' );
}

public function test_get_custom_css_handles_global_custom_css() {
/**
* Tests that base custom CSS is generated correctly.
*/
public function test_get_stylesheet_handles_base_custom_css() {
$theme_json = new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'styles' => array(
'css' => 'body {color:purple;}',
),
)
);

$custom_css = 'body {color:purple;}';
$this->assertSame( $custom_css, $theme_json->get_stylesheet( array( 'custom-css' ) ) );
}

/**
* Tests that block custom CSS is generated correctly.
*/
public function test_get_styles_for_block_handles_block_custom_css() {
$theme_json = new WP_Theme_JSON_Gutenberg(
array(
'version' => WP_Theme_JSON_Gutenberg::LATEST_SCHEMA,
'styles' => array(
'css' => 'body {color:purple;}',
'blocks' => array(
'core/paragraph' => array(
'css' => 'color:red;',
Expand All @@ -4865,8 +4884,17 @@ public function test_get_custom_css_handles_global_custom_css() {
)
);

$custom_css = 'body {color:purple;}:root :where(p){color:red;}';
$this->assertSame( $custom_css, $theme_json->get_custom_css() );
$paragraph_node = array(
'name' => 'core/paragraph',
'path' => array( 'styles', 'blocks', 'core/paragraph' ),
'selector' => 'p',
'selectors' => array(
'root' => 'p',
),
);

$custom_css = ':root :where(p){color:red;}';
$this->assertSame( $custom_css, $theme_json->get_styles_for_block( $paragraph_node ) );
}

/**
Expand Down
Loading