From 65d02b2e416c35fdedd231460a64747c15c005c1 Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Sat, 12 Aug 2023 20:01:23 +0900 Subject: [PATCH 01/10] Block custom CSS: Fix incorrect CSS when multiple root selectors --- lib/class-wp-theme-json-gutenberg.php | 24 ++++++++++++++++--- .../global-styles/use-global-styles-output.js | 23 +++++++++++++++--- phpunit/class-wp-theme-json-test.php | 8 +++++++ 3 files changed, 49 insertions(+), 6 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index 97b9c58b13df29..b23bb678fb725b 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -1138,9 +1138,27 @@ protected function process_blocks_custom_css( $css, $selector ) { // Split CSS nested rules. $parts = explode( '&', $css ); foreach ( $parts as $part ) { - $processed_css .= ( ! str_contains( $part, '{' ) ) - ? trim( $selector ) . '{' . trim( $part ) . '}' // If the part doesn't contain braces, it applies to the root level. - : trim( $selector . $part ); // Prepend the selector, which effectively replaces the "&" character. + $is_root_css = ( ! str_contains( $part, '{' ) ); + if ( $is_root_css ) { + // If the part doesn't contain braces, it applies to the root level. + $processed_css .= trim( $selector ) . '{' . trim( $part ) . '}'; + } else { + // If the part contains braces, it's a nested CSS rule. + $part = explode( '{', str_replace( '}', '', $part ) ); + if ( count( $part ) !== 2 ) { + continue; + } + $nested_selector = $part[0]; + $css_value = $part[1]; + $root_selectors = explode( ',', $selector ); + $combined_selectors = array_map( + static function( $root_selector ) use ( $nested_selector ) { + return $root_selector . $nested_selector; + }, + $root_selectors + ); + $processed_css .= implode( ',', $combined_selectors ) . '{' . trim( $css_value ) . '}'; + } } return $processed_css; } diff --git a/packages/block-editor/src/components/global-styles/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/use-global-styles-output.js index 1db7e46c6eb775..492b55f6262ba3 100644 --- a/packages/block-editor/src/components/global-styles/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/use-global-styles-output.js @@ -1127,9 +1127,26 @@ const processCSSNesting = ( css, blockSelector ) => { // Split CSS nested rules. const parts = css.split( '&' ); parts.forEach( ( part ) => { - processedCSS += ! part.includes( '{' ) - ? blockSelector + '{' + part + '}' // If the part doesn't contain braces, it applies to the root level. - : blockSelector + part; // Prepend the selector, which effectively replaces the "&" character. + const isRootCss = ! part.includes( '{' ); + if ( isRootCss ) { + // If the part doesn't contain braces, it applies to the root level. + processedCSS += `${ blockSelector }{${ part }}`; + } else { + // If the part contains braces, it's a nested CSS rule. + const splittedPart = part.replace( '}', '' ).split( '{' ); + if ( splittedPart.length !== 2 ) { + return; + } + + const [ nestedSelector, cssValue ] = splittedPart; + const rootSelectors = blockSelector.split( ',' ); + const combinedSelectors = rootSelectors.map( + ( rootSelector ) => rootSelector + nestedSelector + ); + processedCSS += `${ combinedSelectors.join( + ', ' + ) }{${ cssValue.trim() }}`; + } } ); return processedCSS; }; diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index 11ec35e0925bfe..c5881b100538d5 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -2033,6 +2033,14 @@ public function data_process_blocks_custom_css() { ), 'expected' => '.foo{color: red; margin: auto;}.foo .bar{color: blue;}.foo::before{color: green;}', ), + // CSS with multiple root selectors. + 'with multiple root selectors' => array( + 'input' => array( + 'selector' => '.foo, .bar', + 'css' => 'color: red; margin: auto; & .baz{color: blue;} &::before{color: green;}', + ), + 'expected' => '.foo, .bar{color: red; margin: auto;}.foo .baz, .bar .baz{color: blue;}.foo::before, .bar::before{color: green;}', + ), ); } From 46650e886880070acaaf79ff933fa33b139be3f9 Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Wed, 27 Sep 2023 12:25:55 +0900 Subject: [PATCH 02/10] Fix PHP lint error --- lib/class-wp-theme-json-gutenberg.php | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index 8f2cb832c5cca0..39774a0dd9fdf0 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -1159,7 +1159,7 @@ protected function process_blocks_custom_css( $css, $selector ) { $css_value = $part[1]; $root_selectors = explode( ',', $selector ); $combined_selectors = array_map( - static function( $root_selector ) use ( $nested_selector ) { + static function ( $root_selector ) use ( $nested_selector ) { return $root_selector . $nested_selector; }, $root_selectors From d738b3f25b9bab161a90e0f42fc3b993ee8796f4 Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Wed, 27 Sep 2023 13:17:36 +0900 Subject: [PATCH 03/10] Use `scope_selector` and `append_to_selector` method and update unit test --- lib/class-wp-theme-json-gutenberg.php | 16 ++++++---------- phpunit/class-wp-theme-json-test.php | 26 +++++++++++++------------- 2 files changed, 19 insertions(+), 23 deletions(-) diff --git a/lib/class-wp-theme-json-gutenberg.php b/lib/class-wp-theme-json-gutenberg.php index 39774a0dd9fdf0..3aa07158df8534 100644 --- a/lib/class-wp-theme-json-gutenberg.php +++ b/lib/class-wp-theme-json-gutenberg.php @@ -1155,16 +1155,12 @@ protected function process_blocks_custom_css( $css, $selector ) { if ( count( $part ) !== 2 ) { continue; } - $nested_selector = $part[0]; - $css_value = $part[1]; - $root_selectors = explode( ',', $selector ); - $combined_selectors = array_map( - static function ( $root_selector ) use ( $nested_selector ) { - return $root_selector . $nested_selector; - }, - $root_selectors - ); - $processed_css .= implode( ',', $combined_selectors ) . '{' . trim( $css_value ) . '}'; + $nested_selector = $part[0]; + $css_value = $part[1]; + $part_selector = str_starts_with( $nested_selector, ' ' ) + ? static::scope_selector( $selector, $nested_selector ) + : static::append_to_selector( $selector, $nested_selector ); + $processed_css .= $part_selector . '{' . trim( $css_value ) . '}'; } } return $processed_css; diff --git a/phpunit/class-wp-theme-json-test.php b/phpunit/class-wp-theme-json-test.php index 6abad044ad1de6..0e212983d9080f 100644 --- a/phpunit/class-wp-theme-json-test.php +++ b/phpunit/class-wp-theme-json-test.php @@ -2007,37 +2007,37 @@ public function test_process_blocks_custom_css( $input, $expected ) { */ public function data_process_blocks_custom_css() { return array( - // Simple CSS without any child selectors. - 'no child selectors' => array( + // Simple CSS without any nested selectors. + 'no nested selectors' => array( 'input' => array( 'selector' => '.foo', 'css' => 'color: red; margin: auto;', ), 'expected' => '.foo{color: red; margin: auto;}', ), - // CSS with child selectors. - 'with children' => array( + // CSS with nested selectors. + 'with nested selector' => array( 'input' => array( 'selector' => '.foo', - 'css' => 'color: red; margin: auto; & .bar{color: blue;}', + 'css' => 'color: red; margin: auto; &.one{color: blue;} & .two{color: green;}', ), - 'expected' => '.foo{color: red; margin: auto;}.foo .bar{color: blue;}', + 'expected' => '.foo{color: red; margin: auto;}.foo.one{color: blue;}.foo .two{color: green;}', ), - // CSS with child selectors and pseudo elements. - 'with children and pseudo elements' => array( + // CSS with pseudo elements. + 'with pseudo elements' => array( 'input' => array( 'selector' => '.foo', - 'css' => 'color: red; margin: auto; & .bar{color: blue;} &::before{color: green;}', + 'css' => 'color: red; margin: auto; &::before{color: blue;} & ::before{color: green;} &.one::before{color: yellow;} & .two::before{color: purple;}', ), - 'expected' => '.foo{color: red; margin: auto;}.foo .bar{color: blue;}.foo::before{color: green;}', + 'expected' => '.foo{color: red; margin: auto;}.foo::before{color: blue;}.foo ::before{color: green;}.foo.one::before{color: yellow;}.foo .two::before{color: purple;}', ), // CSS with multiple root selectors. - 'with multiple root selectors' => array( + 'with multiple root selectors' => array( 'input' => array( 'selector' => '.foo, .bar', - 'css' => 'color: red; margin: auto; & .baz{color: blue;} &::before{color: green;}', + 'css' => 'color: red; margin: auto; &.one{color: blue;} & .two{color: green;} &::before{color: yellow;} & ::before{color: purple;} &.three::before{color: orange;} & .four::before{color: skyblue;}', ), - 'expected' => '.foo, .bar{color: red; margin: auto;}.foo .baz, .bar .baz{color: blue;}.foo::before, .bar::before{color: green;}', + 'expected' => '.foo, .bar{color: red; margin: auto;}.foo.one, .bar.one{color: blue;}.foo .two, .bar .two{color: green;}.foo::before, .bar::before{color: yellow;}.foo ::before, .bar ::before{color: purple;}.foo.three::before, .bar.three::before{color: orange;}.foo .four::before, .bar .four::before{color: skyblue;}', ), ); } From e2d8f78cb807147109c48ec6f03734e4cc70de17 Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Wed, 27 Sep 2023 13:57:10 +0900 Subject: [PATCH 04/10] Use `scopeSelector` and `appendToSelector` function and update JS unit test --- .../test/use-global-styles-output.js | 39 +++++++++++++++++++ .../global-styles/use-global-styles-output.js | 25 ++++++------ .../src/components/global-styles/utils.js | 21 ++++++++++ 3 files changed, 74 insertions(+), 11 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js index 1aead846e95cdb..522ce17111f71e 100644 --- a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js @@ -14,6 +14,7 @@ import { toCustomProperties, toStyles, getStylesDeclarations, + processCSSNesting, } from '../use-global-styles-output'; import { ROOT_BLOCK_SELECTOR } from '../utils'; @@ -967,4 +968,42 @@ describe( 'global styles renderer', () => { ] ); } ); } ); + + describe( 'processCSSNesting', () => { + it( 'should return a processed CSS without any nested selectors', () => { + expect( + processCSSNesting( 'color: red; margin: auto;', '.foo' ) + ).toEqual( '.foo{color: red; margin: auto;}' ); + } ); + it( 'should return a processed CSS with nested selectors', () => { + expect( + processCSSNesting( + 'color: red; margin: auto; &.one{color: blue;} & .two{color: green;}', + '.foo' + ) + ).toEqual( + '.foo{color: red; margin: auto;}.foo.one{color: blue;}.foo .two{color: green;}' + ); + } ); + it( 'should return a processed CSS with pseudo elements', () => { + expect( + processCSSNesting( + 'color: red; margin: auto; &::before{color: blue;} & ::before{color: green;} &.one::before{color: yellow;} & .two::before{color: purple;}', + '.foo' + ) + ).toEqual( + '.foo{color: red; margin: auto;}.foo::before{color: blue;}.foo ::before{color: green;}.foo.one::before{color: yellow;}.foo .two::before{color: purple;}' + ); + } ); + it( 'should return a processed CSS with multiple root selectors', () => { + expect( + processCSSNesting( + 'color: red; margin: auto; &.one{color: blue;} & .two{color: green;} &::before{color: yellow;} & ::before{color: purple;} &.three::before{color: orange;} & .four::before{color: skyblue;}', + '.foo, .bar' + ) + ).toEqual( + '.foo, .bar{color: red; margin: auto;}.foo.one, .bar.one{color: blue;}.foo .two, .bar .two{color: green;}.foo::before, .bar::before{color: yellow;}.foo ::before, .bar ::before{color: purple;}.foo.three::before, .bar.three::before{color: orange;}.foo .four::before, .bar .four::before{color: skyblue;}' + ); + } ); + } ); } ); diff --git a/packages/block-editor/src/components/global-styles/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/use-global-styles-output.js index d2c7a13ae5ffcf..7e99eca355b52e 100644 --- a/packages/block-editor/src/components/global-styles/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/use-global-styles-output.js @@ -15,7 +15,12 @@ import { getCSSRules } from '@wordpress/style-engine'; /** * Internal dependencies */ -import { PRESET_METADATA, ROOT_BLOCK_SELECTOR, scopeSelector } from './utils'; +import { + PRESET_METADATA, + ROOT_BLOCK_SELECTOR, + scopeSelector, + appendToSelector, +} from './utils'; import { getBlockCSSSelector } from './get-block-css-selector'; import { getTypographyFontSizeValue, @@ -1124,7 +1129,7 @@ function updateConfigWithSeparator( config ) { return config; } -const processCSSNesting = ( css, blockSelector ) => { +export function processCSSNesting( css, blockSelector ) { let processedCSS = ''; // Split CSS nested rules. @@ -1133,7 +1138,7 @@ const processCSSNesting = ( css, blockSelector ) => { const isRootCss = ! part.includes( '{' ); if ( isRootCss ) { // If the part doesn't contain braces, it applies to the root level. - processedCSS += `${ blockSelector }{${ part }}`; + processedCSS += `${ blockSelector }{${ part.trim() }}`; } else { // If the part contains braces, it's a nested CSS rule. const splittedPart = part.replace( '}', '' ).split( '{' ); @@ -1142,17 +1147,15 @@ const processCSSNesting = ( css, blockSelector ) => { } const [ nestedSelector, cssValue ] = splittedPart; - const rootSelectors = blockSelector.split( ',' ); - const combinedSelectors = rootSelectors.map( - ( rootSelector ) => rootSelector + nestedSelector - ); - processedCSS += `${ combinedSelectors.join( - ', ' - ) }{${ cssValue.trim() }}`; + const combinedSelector = nestedSelector.startsWith( ' ' ) + ? scopeSelector( blockSelector, nestedSelector ) + : appendToSelector( blockSelector, nestedSelector ); + + processedCSS += `${ combinedSelector }{${ cssValue.trim() }}`; } } ); return processedCSS; -}; +} /** * Returns the global styles output using a global styles configuration. diff --git a/packages/block-editor/src/components/global-styles/utils.js b/packages/block-editor/src/components/global-styles/utils.js index d4f2d959a33659..f8ef8602fe473a 100644 --- a/packages/block-editor/src/components/global-styles/utils.js +++ b/packages/block-editor/src/components/global-styles/utils.js @@ -393,6 +393,27 @@ export function scopeSelector( scope, selector ) { return selectorsScoped.join( ', ' ); } +/** + * Appends a sub-selector to an existing one. + * + * Given the compounded $selector "h1, h2, h3" + * and the $to_append selector ".some-class" the result will be + * "h1.some-class, h2.some-class, h3.some-class". + * + * @param {string} selector Original selector. + * @param {string} toAppend Selector to append. + * + * @return {string} The new selector. + */ +export function appendToSelector( selector, toAppend ) { + if ( ! selector.includes( ',' ) ) { + return selector + toAppend; + } + const selectors = selector.split( ',' ); + const newSelectors = selectors.map( ( sel ) => sel + toAppend ); + return newSelectors.join( ',' ); +} + /** * Compares global style variations according to their styles and settings properties. * From 85c5b05c3e0e7f509e339e9f9e5f3365042b5a2e Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Thu, 28 Sep 2023 19:51:03 +0900 Subject: [PATCH 05/10] Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- .../components/global-styles/test/use-global-styles-output.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js index 522ce17111f71e..cdc94747071c37 100644 --- a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js @@ -970,7 +970,7 @@ describe( 'global styles renderer', () => { } ); describe( 'processCSSNesting', () => { - it( 'should return a processed CSS without any nested selectors', () => { + it( 'should return processed CSS without any nested selectors', () => { expect( processCSSNesting( 'color: red; margin: auto;', '.foo' ) ).toEqual( '.foo{color: red; margin: auto;}' ); From 837690bcebd465fda136aa1f7524222b1d7db6d4 Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Thu, 28 Sep 2023 19:51:10 +0900 Subject: [PATCH 06/10] Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- .../components/global-styles/test/use-global-styles-output.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js index cdc94747071c37..8af49df058d18a 100644 --- a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js @@ -975,7 +975,7 @@ describe( 'global styles renderer', () => { processCSSNesting( 'color: red; margin: auto;', '.foo' ) ).toEqual( '.foo{color: red; margin: auto;}' ); } ); - it( 'should return a processed CSS with nested selectors', () => { + it( 'should return processed CSS with nested selectors', () => { expect( processCSSNesting( 'color: red; margin: auto; &.one{color: blue;} & .two{color: green;}', From 4505084a336a9f5965f850f1cb60a1bd25eee0a0 Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Thu, 28 Sep 2023 19:51:15 +0900 Subject: [PATCH 07/10] Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- .../components/global-styles/test/use-global-styles-output.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js index 8af49df058d18a..53d129c0f54bc3 100644 --- a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js @@ -985,7 +985,7 @@ describe( 'global styles renderer', () => { '.foo{color: red; margin: auto;}.foo.one{color: blue;}.foo .two{color: green;}' ); } ); - it( 'should return a processed CSS with pseudo elements', () => { + it( 'should return processed CSS with pseudo elements', () => { expect( processCSSNesting( 'color: red; margin: auto; &::before{color: blue;} & ::before{color: green;} &.one::before{color: yellow;} & .two::before{color: purple;}', From 660d7e92aa7fdab22833ea8cf26672d3fb026358 Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Thu, 28 Sep 2023 19:51:22 +0900 Subject: [PATCH 08/10] Update packages/block-editor/src/components/global-styles/test/use-global-styles-output.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- .../components/global-styles/test/use-global-styles-output.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js index 53d129c0f54bc3..b05381a8325b06 100644 --- a/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js +++ b/packages/block-editor/src/components/global-styles/test/use-global-styles-output.js @@ -995,7 +995,7 @@ describe( 'global styles renderer', () => { '.foo{color: red; margin: auto;}.foo::before{color: blue;}.foo ::before{color: green;}.foo.one::before{color: yellow;}.foo .two::before{color: purple;}' ); } ); - it( 'should return a processed CSS with multiple root selectors', () => { + it( 'should return processed CSS with multiple root selectors', () => { expect( processCSSNesting( 'color: red; margin: auto; &.one{color: blue;} & .two{color: green;} &::before{color: yellow;} & ::before{color: purple;} &.three::before{color: orange;} & .four::before{color: skyblue;}', From d09b6a3cb2775f5dabded3a5023d916dea8a86fb Mon Sep 17 00:00:00 2001 From: Aki Hamano <54422211+t-hamano@users.noreply.github.com> Date: Thu, 28 Sep 2023 19:51:37 +0900 Subject: [PATCH 09/10] Update packages/block-editor/src/components/global-styles/utils.js Co-authored-by: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> --- packages/block-editor/src/components/global-styles/utils.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/global-styles/utils.js b/packages/block-editor/src/components/global-styles/utils.js index f8ef8602fe473a..f4adb7a7903122 100644 --- a/packages/block-editor/src/components/global-styles/utils.js +++ b/packages/block-editor/src/components/global-styles/utils.js @@ -396,8 +396,8 @@ export function scopeSelector( scope, selector ) { /** * Appends a sub-selector to an existing one. * - * Given the compounded $selector "h1, h2, h3" - * and the $to_append selector ".some-class" the result will be + * Given the compounded `selector` "h1, h2, h3" + * and the `toAppend` selector ".some-class" the result will be * "h1.some-class, h2.some-class, h3.some-class". * * @param {string} selector Original selector. From 97a1593f4c4f913b9162b3449eddc3f3bc056eb4 Mon Sep 17 00:00:00 2001 From: Tetsuaki Hamano Date: Thu, 28 Sep 2023 20:26:43 +0900 Subject: [PATCH 10/10] re-trigger CI