From 5d89b1732eb10f727922f298aea5129859111ce5 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Fri, 17 May 2024 14:46:34 +1200 Subject: [PATCH 1/9] Copy custom CSS between variations when switching --- .../style-variations-container.js | 66 +++++++++++++++++-- 1 file changed, 59 insertions(+), 7 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index 0d19bcedb174b6..f7dd9a388fc052 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -3,9 +3,10 @@ */ import { store as coreStore } from '@wordpress/core-data'; import { useSelect } from '@wordpress/data'; -import { useMemo } from '@wordpress/element'; +import { useContext, useMemo, useState } from '@wordpress/element'; import { __experimentalGrid as Grid } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; +import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; /** * Internal dependencies @@ -13,8 +14,13 @@ import { __ } from '@wordpress/i18n'; import PreviewStyles from './preview-styles'; import Variation from './variations/variation'; import { isVariationWithSingleProperty } from '../../hooks/use-theme-style-variations/use-theme-style-variations-by-property'; +import { unlock } from '../../lock-unlock'; + +const { GlobalStylesContext } = unlock( blockEditorPrivateApis ); export default function StyleVariationsContainer( { gap = 2 } ) { + const { user } = useContext( GlobalStylesContext ); + const [ currentUserStyles ] = useState( { ...user } ); const variations = useSelect( ( select ) => { return select( coreStore @@ -36,13 +42,59 @@ export default function StyleVariationsContainer( { gap = 2 } ) { settings: {}, styles: {}, }, - ...( multiplePropertyVariations ?? [] ).map( ( variation ) => ( { - ...variation, - settings: variation.settings ?? {}, - styles: variation.styles ?? {}, - } ) ), + ...( multiplePropertyVariations ?? [] ).map( ( variation ) => { + const blockStyles = { ...variation?.styles?.blocks } || {}; + // We need to copy any user custom CSS to the variation to prevent it being lost + // when switching variations. + if ( currentUserStyles?.styles?.blocks ) { + Object.keys( currentUserStyles.styles.blocks ).forEach( + ( blockName ) => { + if ( + currentUserStyles.styles.blocks[ blockName ].css + ) { + blockStyles[ blockName ] = { + ...( blockStyles[ blockName ] + ? blockStyles[ blockName ] + : {} ), + css: `${ + blockStyles[ blockName ]?.css || '' + } ${ + currentUserStyles.styles.blocks[ + blockName + ].css + }`, + }; + } + } + ); + } + + const styles = { + ...variation.styles, + ...( currentUserStyles?.styles?.css || + variation?.styles?.css + ? { + css: `${ variation.styles?.css || '' } ${ + currentUserStyles.styles.css + }`, + } + : {} ), + blocks: { + ...blockStyles, + }, + }; + return { + ...variation, + settings: variation.settings ?? {}, + styles: styles ?? {}, + }; + } ), ]; - }, [ multiplePropertyVariations ] ); + }, [ + multiplePropertyVariations, + currentUserStyles.styles.blocks, + currentUserStyles.styles.css, + ] ); return ( Date: Fri, 17 May 2024 16:48:54 +1200 Subject: [PATCH 2/9] Add some comments --- .../style-variations-container.js | 48 ++++++++----------- 1 file changed, 19 insertions(+), 29 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index f7dd9a388fc052..04f096d4e78629 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -21,6 +21,7 @@ const { GlobalStylesContext } = unlock( blockEditorPrivateApis ); export default function StyleVariationsContainer( { gap = 2 } ) { const { user } = useContext( GlobalStylesContext ); const [ currentUserStyles ] = useState( { ...user } ); + const userStyles = currentUserStyles?.styles; const variations = useSelect( ( select ) => { return select( coreStore @@ -46,36 +47,29 @@ export default function StyleVariationsContainer( { gap = 2 } ) { const blockStyles = { ...variation?.styles?.blocks } || {}; // We need to copy any user custom CSS to the variation to prevent it being lost // when switching variations. - if ( currentUserStyles?.styles?.blocks ) { - Object.keys( currentUserStyles.styles.blocks ).forEach( - ( blockName ) => { - if ( - currentUserStyles.styles.blocks[ blockName ].css - ) { - blockStyles[ blockName ] = { - ...( blockStyles[ blockName ] - ? blockStyles[ blockName ] - : {} ), - css: `${ - blockStyles[ blockName ]?.css || '' - } ${ - currentUserStyles.styles.blocks[ - blockName - ].css - }`, - }; - } + if ( userStyles?.blocks ) { + Object.keys( userStyles.blocks ).forEach( ( blockName ) => { + // First get any block specific custom CSS from the current user styles and merge with any custom CSS for + // that block in the variation. + if ( userStyles.blocks[ blockName ].css ) { + blockStyles[ blockName ] = { + ...( blockStyles[ blockName ] + ? blockStyles[ blockName ] + : {} ), + css: `${ + blockStyles[ blockName ]?.css || '' + } ${ userStyles.blocks[ blockName ].css }`, + }; } - ); + } ); } - + // Now merge any global custom CSS from current user styles with global custom CSS in the variation. const styles = { ...variation.styles, - ...( currentUserStyles?.styles?.css || - variation?.styles?.css + ...( userStyles?.css || variation.styles?.css ? { css: `${ variation.styles?.css || '' } ${ - currentUserStyles.styles.css + userStyles?.css }`, } : {} ), @@ -90,11 +84,7 @@ export default function StyleVariationsContainer( { gap = 2 } ) { }; } ), ]; - }, [ - multiplePropertyVariations, - currentUserStyles.styles.blocks, - currentUserStyles.styles.css, - ] ); + }, [ multiplePropertyVariations, userStyles.blocks, userStyles?.css ] ); return ( Date: Sat, 18 May 2024 18:54:57 +1200 Subject: [PATCH 3/9] Make spreads a bit more readable and make sure customCSS added to default also --- .../style-variations-container.js | 33 ++++++++++--------- 1 file changed, 18 insertions(+), 15 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index 04f096d4e78629..ad756ed468cfd0 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -36,13 +36,14 @@ export default function StyleVariationsContainer( { gap = 2 } ) { ); } ); + multiplePropertyVariations.unshift( { + title: __( 'Default' ), + settings: {}, + styles: {}, + } ); + const withEmptyVariation = useMemo( () => { return [ - { - title: __( 'Default' ), - settings: {}, - styles: {}, - }, ...( multiplePropertyVariations ?? [] ).map( ( variation ) => { const blockStyles = { ...variation?.styles?.blocks } || {}; // We need to copy any user custom CSS to the variation to prevent it being lost @@ -52,10 +53,11 @@ export default function StyleVariationsContainer( { gap = 2 } ) { // First get any block specific custom CSS from the current user styles and merge with any custom CSS for // that block in the variation. if ( userStyles.blocks[ blockName ].css ) { + const variationBlockStyles = + blockStyles[ blockName ] || {}; + blockStyles[ blockName ] = { - ...( blockStyles[ blockName ] - ? blockStyles[ blockName ] - : {} ), + ...variationBlockStyles, css: `${ blockStyles[ blockName ]?.css || '' } ${ userStyles.blocks[ blockName ].css }`, @@ -64,15 +66,16 @@ export default function StyleVariationsContainer( { gap = 2 } ) { } ); } // Now merge any global custom CSS from current user styles with global custom CSS in the variation. + const globalCustomCSS = + userStyles?.css || variation.styles?.css + ? `${ variation.styles?.css || '' } ${ + userStyles?.css || '' + }` + : ''; + const styles = { ...variation.styles, - ...( userStyles?.css || variation.styles?.css - ? { - css: `${ variation.styles?.css || '' } ${ - userStyles?.css - }`, - } - : {} ), + css: globalCustomCSS, blocks: { ...blockStyles, }, From bb2825daa3340c1fd80527d2d9fb84be61f947a3 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Sun, 19 May 2024 14:31:42 +1200 Subject: [PATCH 4/9] Change how empty variation is accounted for so it also picks up custom CSS --- .../style-variations-container.js | 20 ++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index ad756ed468cfd0..5d010ae5d27242 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -36,15 +36,17 @@ export default function StyleVariationsContainer( { gap = 2 } ) { ); } ); - multiplePropertyVariations.unshift( { - title: __( 'Default' ), - settings: {}, - styles: {}, - } ); - - const withEmptyVariation = useMemo( () => { + const themeVariations = useMemo( () => { + const withEmptyVariation = [ + { + title: __( 'Default' ), + settings: {}, + styles: {}, + }, + ...( multiplePropertyVariations ?? [] ), + ]; return [ - ...( multiplePropertyVariations ?? [] ).map( ( variation ) => { + ...withEmptyVariation.map( ( variation ) => { const blockStyles = { ...variation?.styles?.blocks } || {}; // We need to copy any user custom CSS to the variation to prevent it being lost // when switching variations. @@ -95,7 +97,7 @@ export default function StyleVariationsContainer( { gap = 2 } ) { className="edit-site-global-styles-style-variations-container" gap={ gap } > - { withEmptyVariation.map( ( variation, index ) => ( + { themeVariations.map( ( variation, index ) => ( { ( isFocused ) => ( Date: Wed, 22 May 2024 07:39:38 -0700 Subject: [PATCH 5/9] Don't add empty properties to style objects --- .../global-styles/style-variations-container.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index 5d010ae5d27242..b288b097ebe3b2 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -57,12 +57,23 @@ export default function StyleVariationsContainer( { gap = 2 } ) { if ( userStyles.blocks[ blockName ].css ) { const variationBlockStyles = blockStyles[ blockName ] || {}; + const customCSS = + blockStyles[ blockName ]?.css || + userStyles.blocks[ blockName ].css + ? { + css: `${ + blockStyles[ blockName ]?.css || + '' + } ${ + userStyles.blocks[ blockName ] + .css || '' + }`, + } + : {}; blockStyles[ blockName ] = { ...variationBlockStyles, - css: `${ - blockStyles[ blockName ]?.css || '' - } ${ userStyles.blocks[ blockName ].css }`, + ...customCSS, }; } } ); From 3623193726fa89f9177668c9b8bda7126e86baff Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Wed, 22 May 2024 08:16:28 -0700 Subject: [PATCH 6/9] Another spread fix --- .../style-variations-container.js | 23 +++++++++++-------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index b288b097ebe3b2..adf7fbafe222a6 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -79,19 +79,24 @@ export default function StyleVariationsContainer( { gap = 2 } ) { } ); } // Now merge any global custom CSS from current user styles with global custom CSS in the variation. - const globalCustomCSS = + const css = userStyles?.css || variation.styles?.css - ? `${ variation.styles?.css || '' } ${ - userStyles?.css || '' - }` - : ''; + ? { + css: `${ variation.styles?.css || '' } ${ + userStyles?.css || '' + }`, + } + : {}; + + const blocks = + Object.keys( blockStyles ).length > 0 + ? { blocks: blockStyles } + : {}; const styles = { ...variation.styles, - css: globalCustomCSS, - blocks: { - ...blockStyles, - }, + ...css, + ...blocks, }; return { ...variation, From cfb2057003f9819618121d524381a7ad185ce646 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Mon, 27 May 2024 12:15:23 +1200 Subject: [PATCH 7/9] Reset the custom css in variations if the globals styles are reset --- .../global-styles/style-variations-container.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index adf7fbafe222a6..8b0c2276bdc36c 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -3,7 +3,7 @@ */ import { store as coreStore } from '@wordpress/core-data'; import { useSelect } from '@wordpress/data'; -import { useContext, useMemo, useState } from '@wordpress/element'; +import { useContext, useEffect, useMemo, useState } from '@wordpress/element'; import { __experimentalGrid as Grid } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; @@ -20,8 +20,13 @@ const { GlobalStylesContext } = unlock( blockEditorPrivateApis ); export default function StyleVariationsContainer( { gap = 2 } ) { const { user } = useContext( GlobalStylesContext ); - const [ currentUserStyles ] = useState( { ...user } ); + const [ currentUserStyles, setCurrentUserStyles ] = useState( { ...user } ); const userStyles = currentUserStyles?.styles; + + useEffect( () => { + setCurrentUserStyles( { ...user } ); + }, [ user ] ); + const variations = useSelect( ( select ) => { return select( coreStore From f949d46f85d0dce62a062485bee23707fb5f81b7 Mon Sep 17 00:00:00 2001 From: Glen Davies Date: Thu, 30 May 2024 12:17:15 +1200 Subject: [PATCH 8/9] Changes from code review --- .../style-variations-container.js | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index 8b0c2276bdc36c..6a3fbacf71c9c9 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -20,11 +20,11 @@ const { GlobalStylesContext } = unlock( blockEditorPrivateApis ); export default function StyleVariationsContainer( { gap = 2 } ) { const { user } = useContext( GlobalStylesContext ); - const [ currentUserStyles, setCurrentUserStyles ] = useState( { ...user } ); + const [ currentUserStyles, setCurrentUserStyles ] = useState( user ); const userStyles = currentUserStyles?.styles; useEffect( () => { - setCurrentUserStyles( { ...user } ); + setCurrentUserStyles( user ); }, [ user ] ); const variations = useSelect( ( select ) => { @@ -53,6 +53,7 @@ export default function StyleVariationsContainer( { gap = 2 } ) { return [ ...withEmptyVariation.map( ( variation ) => { const blockStyles = { ...variation?.styles?.blocks } || {}; + // We need to copy any user custom CSS to the variation to prevent it being lost // when switching variations. if ( userStyles?.blocks ) { @@ -62,20 +63,14 @@ export default function StyleVariationsContainer( { gap = 2 } ) { if ( userStyles.blocks[ blockName ].css ) { const variationBlockStyles = blockStyles[ blockName ] || {}; - const customCSS = - blockStyles[ blockName ]?.css || - userStyles.blocks[ blockName ].css - ? { - css: `${ - blockStyles[ blockName ]?.css || - '' - } ${ - userStyles.blocks[ blockName ] - .css || '' - }`, - } - : {}; - + const customCSS = { + css: `${ + blockStyles[ blockName ]?.css || '' + } ${ + userStyles.blocks[ blockName ].css.trim() || + '' + }`, + }; blockStyles[ blockName ] = { ...variationBlockStyles, ...customCSS, @@ -110,7 +105,7 @@ export default function StyleVariationsContainer( { gap = 2 } ) { }; } ), ]; - }, [ multiplePropertyVariations, userStyles.blocks, userStyles?.css ] ); + }, [ multiplePropertyVariations, userStyles?.blocks, userStyles?.css ] ); return ( Date: Thu, 30 May 2024 12:28:47 +1200 Subject: [PATCH 9/9] remove unnecessary check for undefined --- .../src/components/global-styles/style-variations-container.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-site/src/components/global-styles/style-variations-container.js b/packages/edit-site/src/components/global-styles/style-variations-container.js index 6a3fbacf71c9c9..db567db52b3252 100644 --- a/packages/edit-site/src/components/global-styles/style-variations-container.js +++ b/packages/edit-site/src/components/global-styles/style-variations-container.js @@ -101,7 +101,7 @@ export default function StyleVariationsContainer( { gap = 2 } ) { return { ...variation, settings: variation.settings ?? {}, - styles: styles ?? {}, + styles, }; } ), ];