Fix inconsistencies in feature selector processing part 1: global styles

backport-changelog/7.1/11857.md

@@ -0,0 +1,3 @@
+https://github.com/WordPress/wordpress-develop/pull/11857
+
+* https://github.com/WordPress/gutenberg/pull/78276

packages/global-styles-engine/src/core/render.tsx

@@ -23,6 +23,7 @@ import {
 	scopeFeatureSelectors,
 	appendToSelector,
 	getBlockStyleVariationSelector,
+	getBlockStyleVariationFeatureSelector,
 	getResolvedValue,
 } from '../utils/common';
 import { getBlockSelector } from './selectors';
@@ -152,6 +153,7 @@ export type BlockSelectors = Record<
  * - `fallbackGapValue`: fallback block gap value used by layout rules.
  * - `hasLayoutSupport`: whether layout styles can be generated for the node.
  * - `isStyleVariation`: whether this node is a block style variation.
+ * - `variationName`: block style variation name used for feature selectors.
  * - `layoutSelector`: optional selector override for layout styles.
  * - `layoutHasBlockGapSupport`: optional block gap support override for layout styles.
  * - `name`: block name used by block-specific declaration adjustments.
@@ -170,6 +172,7 @@ interface StylesNode {
 	fallbackGapValue?: string;
 	hasLayoutSupport?: boolean;
 	isStyleVariation?: boolean;
+	variationName?: string;
 	layoutSelector?: string;
 	layoutHasBlockGapSupport?: boolean;
 	name?: string;
@@ -339,27 +342,6 @@ function flattenTree(
 	return result;
 }
 
-/**
- * Gets variation selector string from feature selector.
- *
- * @param featureSelector        The feature selector
- * @param styleVariationSelector The style variation selector
- * @return Combined selector string
- */
-function concatFeatureVariationSelectorString(
-	featureSelector: string,
-	styleVariationSelector: string
-): string {
-	const featureSelectors = featureSelector.split( ',' );
-	const combinedSelectors: string[] = [];
-	featureSelectors.forEach( ( selector ) => {
-		combinedSelectors.push(
-			`${ styleVariationSelector.trim() }${ selector.trim() }`
-		);
-	} );
-	return combinedSelectors.join( ', ' );
-}
-
 /**
  * Updates the text indent selector for paragraph blocks based on the textIndent setting.
  *
@@ -978,6 +960,7 @@ function getPseudoStyleNodes( node: StylesNode ): StylesNode[] {
 		name,
 		elementName,
 		mediaQuery,
+		variationName,
 	} = node;
 	const pseudoSelectors = name
 		? VALID_BLOCK_PSEUDO_SELECTORS[ name ] ?? []
@@ -1005,6 +988,7 @@ function getPseudoStyleNodes( node: StylesNode ): StylesNode[] {
 						: undefined,
 				name,
 				elementName,
+				variationName,
 			},
 		];
 	} );
@@ -1027,6 +1011,7 @@ function getResponsiveStyleNodes( node: StylesNode ): StylesNode[] {
 		name,
 		elementName,
 		isStyleVariation,
+		variationName,
 	} = node;
 
 	if ( ! name && ! elementName ) {
@@ -1052,60 +1037,13 @@ function getResponsiveStyleNodes( node: StylesNode ): StylesNode[] {
 					name,
 					elementName,
 					isStyleVariation,
+					variationName,
 				},
 			];
 		}
 	);
 }
 
-/**
- * Scopes feature selectors to a style variation selector.
- *
- * Variation feature selectors are compound selectors rather than suffixes. For
- * example, `.wp-image-spacing` becomes `.is-style-foo.wp-image.wp-image-spacing`.
- *
- * @param featureSelectors       Feature-level selectors from a style node.
- * @param styleVariationSelector Selector for the style variation.
- * @return Feature-level selectors scoped to the style variation.
- */
-function getVariationFeatureSelectors(
-	featureSelectors: StylesNode[ 'featureSelectors' ],
-	styleVariationSelector: string
-): StylesNode[ 'featureSelectors' ] {
-	if ( ! featureSelectors || typeof featureSelectors === 'string' ) {
-		return undefined;
-	}
-
-	return Object.fromEntries(
-		Object.entries( featureSelectors ).map( ( [ feature, selector ] ) => {
-			if ( typeof selector === 'string' ) {
-				return [
-					feature,
-					concatFeatureVariationSelectorString(
-						selector,
-						styleVariationSelector
-					),
-				];
-			}
-
-			return [
-				feature,
-				Object.fromEntries(
-					Object.entries( selector ).map(
-						( [ subfeature, subfeatureSelector ] ) => [
-							subfeature,
-							concatFeatureVariationSelectorString(
-								subfeatureSelector,
-								styleVariationSelector
-							),
-						]
-					)
-				),
-			];
-		} )
-	);
-}
-
 export const getNodesWithStyles = (
 	tree: GlobalStylesConfig,
 	blockSelectors: string | BlockSelectors
@@ -1179,15 +1117,14 @@ export const getNodesWithStyles = (
 							variationStyleNodesToAdd.push( {
 								styles: variationStyles,
 								selector: variationSelector,
-								featureSelectors: getVariationFeatureSelectors(
+								featureSelectors:
 									blockSelector?.featureSelectors,
-									variationSelector
-								),
 								fallbackGapValue:
 									blockSelector?.fallbackGapValue,
 								hasLayoutSupport:
 									blockSelector?.hasLayoutSupport,
 								isStyleVariation: true,
+								variationName,
 								layoutSelector:
 									variationSelector + blockSelector.selector,
 								layoutHasBlockGapSupport: true,
@@ -1635,6 +1572,7 @@ function renderStylesNode(
 		layoutHasBlockGapSupport,
 		skipSelectorWrapper,
 		name,
+		variationName,
 	} = node;
 	let ruleset = '';
 	const effectiveSelector = selectorSuffix
@@ -1665,9 +1603,15 @@ function renderStylesNode(
 		Object.entries( featureDeclarations ).forEach(
 			( [ featureSelector, declarations ] ) => {
 				if ( declarations.length ) {
-					const selectorForRule = selectorSuffix
-						? appendToSelector( featureSelector, selectorSuffix )
+					let selectorForRule = variationName
+						? getBlockStyleVariationFeatureSelector(
+								variationName,
+								featureSelector
+						  )
 						: featureSelector;
+					selectorForRule = selectorSuffix
+						? appendToSelector( selectorForRule, selectorSuffix )
+						: selectorForRule;
 					const rules = declarations.join( ';' );
 					ruleset += `:root :where(${ selectorForRule }){${ rules };}`;
 				}

packages/global-styles-engine/src/test/render.test.ts

@@ -661,7 +661,7 @@ describe( 'global styles renderer', () => {
 				styleOptions
 			);
 			expect( withVariations ).toEqual(
-				':root :where(.is-style-foo.wp-image.wp-image-spacing){padding-top: 2px;}:root :where(.is-style-foo.wp-image.wp-image-border-color){border-color: blue;}:root :where(.is-style-foo.wp-image){color: blue;}'
+				':root :where(.wp-image-spacing.is-style-foo){padding-top: 2px;}:root :where(.wp-image-border-color.is-style-foo){border-color: blue;}:root :where(.is-style-foo.wp-image){color: blue;}'
 			);
 		} );
 
@@ -868,10 +868,16 @@ describe( 'global styles renderer', () => {
 									color: {
 										text: 'green',
 									},
+									dimensions: {
+										width: '10rem',
+									},
 									':hover': {
 										color: {
 											text: 'yellow',
 										},
+										dimensions: {
+											width: '20rem',
+										},
 									},
 								},
 							},
@@ -882,9 +888,15 @@ describe( 'global styles renderer', () => {
 
 			const blockSelectors = {
 				'core/button': {
-					selector: '.wp-block-button',
+					selector: '.wp-block-button .wp-block-button__link',
+					featureSelectors: {
+						dimensions: {
+							root: '.wp-block-button',
+							width: '.wp-block-button',
+						},
+					},
 					styleVariationSelectors: {
-						foo: '.is-style-foo.wp-block-button',
+						foo: '.wp-block-button.is-style-foo .wp-block-button__link',
 					},
 				},
 			};
@@ -908,11 +920,11 @@ describe( 'global styles renderer', () => {
 			);
 
 			expect( result ).toEqual(
-				':root :where(.is-style-foo.wp-block-button){color: green;}:root :where(.is-style-foo.wp-block-button:hover){color: yellow;}'
+				':root :where(.wp-block-button.is-style-foo){width: 10rem;}:root :where(.wp-block-button.is-style-foo .wp-block-button__link){color: green;}:root :where(.wp-block-button.is-style-foo:hover){width: 20rem;}:root :where(.wp-block-button.is-style-foo .wp-block-button__link:hover){color: yellow;}'
 			);
 		} );
 
-		it( 'outputs variation pseudo styles after variation responsive base styles', () => {
+		it( 'outputs variation responsive pseudo styles after default pseudo styles', () => {
 			const tree = {
 				styles: {
 					blocks: {
@@ -922,19 +934,31 @@ describe( 'global styles renderer', () => {
 									color: {
 										text: 'green',
 									},
+									dimensions: {
+										width: '10rem',
+									},
 									':hover': {
 										color: {
 											text: 'blue',
 										},
+										dimensions: {
+											width: '20rem',
+										},
 									},
-									mobile: {
+									tablet: {
 										color: {
 											text: 'red',
 										},
+										dimensions: {
+											width: '30rem',
+										},
 										':hover': {
 											color: {
 												text: 'orange',
 											},
+											dimensions: {
+												width: '40rem',
+											},
 										},
 									},
 								},
@@ -946,9 +970,15 @@ describe( 'global styles renderer', () => {
 
 			const blockSelectors = {
 				'core/button': {
-					selector: '.wp-block-button',
+					selector: '.wp-block-button .wp-block-button__link',
+					featureSelectors: {
+						dimensions: {
+							root: '.wp-block-button',
+							width: '.wp-block-button',
+						},
+					},
 					styleVariationSelectors: {
-						foo: '.is-style-foo.wp-block-button',
+						foo: '.wp-block-button.is-style-foo .wp-block-button__link',
 					},
 				},
 			};
@@ -967,7 +997,7 @@ describe( 'global styles renderer', () => {
 			);
 
 			expect( result ).toEqual(
-				':root :where(.is-style-foo.wp-block-button){color: green;}@media (width <= 480px){:root :where(.is-style-foo.wp-block-button){color: red;}}:root :where(.is-style-foo.wp-block-button:hover){color: blue;}@media (width <= 480px){:root :where(.is-style-foo.wp-block-button:hover){color: orange;}}'
+				':root :where(.wp-block-button.is-style-foo){width: 10rem;}:root :where(.wp-block-button.is-style-foo .wp-block-button__link){color: green;}@media (480px < width <= 782px){:root :where(.wp-block-button.is-style-foo){width: 30rem;}:root :where(.wp-block-button.is-style-foo .wp-block-button__link){color: red;}}:root :where(.wp-block-button.is-style-foo:hover){width: 20rem;}:root :where(.wp-block-button.is-style-foo .wp-block-button__link:hover){color: blue;}@media (480px < width <= 782px){:root :where(.wp-block-button.is-style-foo:hover){width: 40rem;}:root :where(.wp-block-button.is-style-foo .wp-block-button__link:hover){color: orange;}}'
 			);
 		} );
 

packages/global-styles-engine/src/test/utils.test.ts

@@ -8,6 +8,7 @@
  */
 import type { GlobalStylesConfig } from '../types';
 import {
+	getBlockStyleVariationFeatureSelector,
 	getBlockStyleVariationSelector,
 	getValueFromVariable,
 	getPresetVariableFromValue,
@@ -302,6 +303,13 @@ describe( 'editor utils', () => {
 				expected:
 					'.wp-block.is-style-custom:is(.outer .inner:first-child)',
 			},
+			{
+				type: ':is with selector list',
+				selector:
+					'.wp-block:is(.outer, .inner:first-child) .content, .wp-block-alternative',
+				expected:
+					'.wp-block.is-style-custom:is(.outer, .inner:first-child) .content, .wp-block-alternative.is-style-custom',
+			},
 			{
 				type: ':not',
 				selector: '.wp-block:not(.outer .inner:first-child)',
@@ -342,6 +350,37 @@ describe( 'editor utils', () => {
 		);
 	} );
 
+	describe( 'getBlockStyleVariationFeatureSelector', () => {
+		it( 'adds the variation class to the feature selector target', () => {
+			expect(
+				getBlockStyleVariationFeatureSelector(
+					'outline',
+					'.wp-block-button'
+				)
+			).toBe( '.wp-block-button.is-style-outline' );
+		} );
+
+		it( 'removes an outer variation scope before adding the variation class', () => {
+			expect(
+				getBlockStyleVariationFeatureSelector(
+					'outline--3',
+					'.is-style-outline--3 .wp-block-button'
+				)
+			).toBe( '.wp-block-button.is-style-outline--3' );
+		} );
+
+		it( 'preserves selector-list commas inside pseudo-class functions', () => {
+			expect(
+				getBlockStyleVariationFeatureSelector(
+					'outline--3',
+					'.is-style-outline--3 .wp-block-button:is(.primary, .secondary), .is-style-outline--3 .wp-block-button .inner'
+				)
+			).toBe(
+				'.wp-block-button.is-style-outline--3:is(.primary, .secondary),.wp-block-button.is-style-outline--3 .inner'
+			);
+		} );
+	} );
+
 	describe( 'scopeFeatureSelectors', () => {
 		it( 'correctly scopes selectors while maintaining selectors object structure', () => {
 			const actual = scopeFeatureSelectors( '.custom, .secondary', {