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.

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

Block custom CSS: Fix incorrect CSS when multiple root selectors #53602

Merged
merged 12 commits into from
Sep 29, 2023
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
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
20 changes: 17 additions & 3 deletions lib/class-wp-theme-json-gutenberg.php
Original file line number Diff line number Diff line change
Expand Up @@ -1145,9 +1145,23 @@ 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;
}
Comment on lines +1155 to +1157
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is to exclude strings with invalid braces, such as the following, and to ensure that they are correctly split into selector and value.

& ::marker{{ color: red;}

$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;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
toCustomProperties,
toStyles,
getStylesDeclarations,
processCSSNesting,
} from '../use-global-styles-output';
import { ROOT_BLOCK_SELECTOR } from '../utils';

Expand Down Expand Up @@ -967,4 +968,42 @@ describe( 'global styles renderer', () => {
] );
} );
} );

describe( 'processCSSNesting', () => {
it( 'should return a processed CSS without any nested selectors', () => {
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
expect(
processCSSNesting( 'color: red; margin: auto;', '.foo' )
).toEqual( '.foo{color: red; margin: auto;}' );
} );
it( 'should return a processed CSS with nested selectors', () => {
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
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', () => {
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
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', () => {
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
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;}'
);
} );
} );
} );
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -1124,18 +1129,33 @@ function updateConfigWithSeparator( config ) {
return config;
}

const processCSSNesting = ( css, blockSelector ) => {
export function processCSSNesting( css, blockSelector ) {
let processedCSS = '';

// 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.trim() }}`;
} 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 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.
Expand Down
21 changes: 21 additions & 0 deletions packages/block-editor/src/components/global-styles/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -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".
t-hamano marked this conversation as resolved.
Show resolved Hide resolved
*
* @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.
*
Expand Down
28 changes: 18 additions & 10 deletions phpunit/class-wp-theme-json-test.php
Original file line number Diff line number Diff line change
Expand Up @@ -2007,29 +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(
'input' => array(
'selector' => '.foo, .bar',
'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.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;}',
),
);
}
Expand Down
Loading