From 83f62ca81ff89b9d395863f6e9b7e82cec76d1b3 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Tue, 11 Jun 2024 09:44:50 +0100 Subject: [PATCH 01/11] Global Styles: Only use single property variations as color/type presets --- .../use-theme-style-variations-by-property.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 2a914efc6ee52..edb4148dc7bac 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -156,6 +156,14 @@ export default function useThemeStyleVariationsByProperty( { let processedStyleVariations = variations.reduce( ( accumulator, variation ) => { + // Include only variations contain nothing but the property. + if ( + ! isVariationWithSingleProperty( variation, property ) && + variation.title !== __( 'Default' ) + ) { + return accumulator; + } + const variationFilteredByProperty = filterObjectByProperty( cloneDeep( variation ), property From 1621cf9966aa2389ef0f4afbdf082c2f38b615a8 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Tue, 11 Jun 2024 10:26:54 +0100 Subject: [PATCH 02/11] remove unused code --- .../use-theme-style-variations-by-property.js | 30 ++++--------------- 1 file changed, 6 insertions(+), 24 deletions(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index edb4148dc7bac..8a4d0641c4b24 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -164,33 +164,15 @@ export default function useThemeStyleVariationsByProperty( { return accumulator; } - const variationFilteredByProperty = filterObjectByProperty( - cloneDeep( variation ), - property - ); - - // Remove variations that are empty once the property is filtered out. - if ( - variation.title !== __( 'Default' ) && - Object.keys( variationFilteredByProperty ).length === 0 - ) { - return accumulator; - } - - let result = { - ...variationFilteredByProperty, - title: variation?.title, - description: variation?.description, - }; - + /* + * Overwrites all baseVariation object `styleProperty` properties + * with the theme variation `styleProperty` properties. + */ + let result = variation; if ( clonedBaseVariation ) { - /* - * Overwrites all baseVariation object `styleProperty` properties - * with the theme variation `styleProperty` properties. - */ result = mergeBaseAndUserConfigs( clonedBaseVariation, - result + variation ); } From 81b1bfe9b361fa66341b03396473e1e7c00a47d0 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Tue, 11 Jun 2024 13:28:07 +0100 Subject: [PATCH 03/11] remove unused code --- .../use-theme-style-variations-by-property.js | 59 +++++++++++-------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 8a4d0641c4b24..8f5088fc6b2fa 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -51,14 +51,12 @@ export function removePropertyFromObject( object, property ) { * A convenience wrapper for `useThemeStyleVariationsByProperty()` that fetches the current theme style variations, * and user-defined global style/settings object. * - * @param {Object} props Object of hook args. - * @param {string} props.property The property to filter by. - * @param {Function} props.filter Optional. The filter function to apply to the variations. + * @param {Object} props Object of hook args. + * @param {string} props.property The property to filter by. * @return {Object[]|*} The merged object. */ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { property, - filter, } ) { const { variationsFromTheme } = useSelect( ( select ) => { const _variationsFromTheme = @@ -71,27 +69,26 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { }; }, [] ); const { user: baseVariation } = useContext( GlobalStylesContext ); + const clondedBaseVariation = cloneDeep( baseVariation ); + + const variationsWithSinglePropertyAndBase = variationsFromTheme + .filter( ( variation ) => { + return isVariationWithSingleProperty( variation, property ); + } ) + .map( ( variation ) => { + return mergeBaseAndUserConfigs( clondedBaseVariation, variation ); + } ); - const variations = useMemo( () => { + return useMemo( () => { return [ { title: __( 'Default' ), settings: {}, styles: {}, }, - ...variationsFromTheme, + ...variationsWithSinglePropertyAndBase, ]; - }, [ variationsFromTheme ] ); - - return useThemeStyleVariationsByProperty( { - variations, - property, - filter, - baseVariation: removePropertyFromObject( - cloneDeep( baseVariation ), - property - ), - } ); + }, [ variationsWithSinglePropertyAndBase ] ); } /** @@ -156,23 +153,33 @@ export default function useThemeStyleVariationsByProperty( { let processedStyleVariations = variations.reduce( ( accumulator, variation ) => { - // Include only variations contain nothing but the property. + const variationFilteredByProperty = filterObjectByProperty( + cloneDeep( variation ), + property + ); + + // Remove variations that are empty once the property is filtered out. if ( - ! isVariationWithSingleProperty( variation, property ) && - variation.title !== __( 'Default' ) + variation.title !== __( 'Default' ) && + Object.keys( variationFilteredByProperty ).length === 0 ) { return accumulator; } - /* - * Overwrites all baseVariation object `styleProperty` properties - * with the theme variation `styleProperty` properties. - */ - let result = variation; + let result = { + ...variationFilteredByProperty, + title: variation?.title, + description: variation?.description, + }; + if ( clonedBaseVariation ) { + /* + * Overwrites all baseVariation object `styleProperty` properties + * with the theme variation `styleProperty` properties. + */ result = mergeBaseAndUserConfigs( clonedBaseVariation, - variation + result ); } From f3d24fe7407a9009f5eb7ae820e47e63b2193f71 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Tue, 11 Jun 2024 14:17:42 +0100 Subject: [PATCH 04/11] remove unused code --- .../use-theme-style-variations-by-property.js | 876 +----------------- .../use-theme-style-variations-by-property.js | 92 -- 2 files changed, 1 insertion(+), 967 deletions(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/test/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/test/use-theme-style-variations-by-property.js index 89a58166257f6..c45162a6f9dbe 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/test/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/test/use-theme-style-variations-by-property.js @@ -1,12 +1,7 @@ -/** - * External dependencies - */ -import { renderHook } from '@testing-library/react'; - /** * Internal dependencies */ -import useThemeStyleVariationsByProperty, { +import { filterObjectByProperty, removePropertyFromObject, } from '../use-theme-style-variations-by-property'; @@ -95,875 +90,6 @@ describe( 'filterObjectByProperty', () => { ); } ); -describe( 'useThemeStyleVariationsByProperty', () => { - const mockVariations = [ - { - title: 'Title 1', - description: 'Description 1', - settings: { - color: { - duotone: [ - { - name: 'Dark grayscale', - colors: [ '#000000', '#7f7f7f' ], - slug: 'dark-grayscale', - }, - { - name: 'Grayscale', - colors: [ '#000000', '#ffffff' ], - slug: 'grayscale', - }, - { - name: 'Purple and yellow', - colors: [ '#8c00b7', '#fcff41' ], - slug: 'purple-yellow', - }, - ], - gradients: [ - { - name: 'Vivid cyan blue to vivid purple', - gradient: - 'linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%)', - slug: 'vivid-cyan-blue-to-vivid-purple', - }, - { - name: 'Light green cyan to vivid green cyan', - gradient: - 'linear-gradient(135deg,rgb(122,220,180) 0%,rgb(0,208,130) 100%)', - slug: 'light-green-cyan-to-vivid-green-cyan', - }, - { - name: 'Luminous vivid amber to luminous vivid orange', - gradient: - 'linear-gradient(135deg,rgba(252,185,0,1) 0%,rgba(255,105,0,1) 100%)', - slug: 'luminous-vivid-amber-to-luminous-vivid-orange', - }, - ], - palette: [ - { - name: 'Vivid red', - slug: 'vivid-red', - color: '#cf2e2e', - }, - { - name: 'Luminous vivid orange', - slug: 'luminous-vivid-orange', - color: '#ff6900', - }, - { - name: 'Luminous vivid amber', - slug: 'luminous-vivid-amber', - color: '#fcb900', - }, - ], - }, - typography: { - fluid: true, - fontFamilies: { - theme: [ - { - name: 'Inter san-serif', - fontFamily: 'Inter san-serif', - slug: 'inter-san-serif', - fontFace: [ - { - src: 'inter-san-serif.woff2', - fontWeight: '400', - fontStyle: 'italic', - fontFamily: 'Inter san-serif', - }, - ], - }, - ], - }, - fontSizes: [ - { - name: 'Small', - slug: 'small', - size: '13px', - }, - { - name: 'Medium', - slug: 'medium', - size: '20px', - }, - { - name: 'Large', - slug: 'large', - size: '36px', - }, - ], - }, - layout: { - wideSize: '1200px', - }, - }, - styles: { - typography: { - letterSpacing: '3px', - }, - color: { - backgroundColor: 'red', - color: 'orange', - }, - elements: { - cite: { - color: { - text: 'white', - }, - }, - }, - blocks: { - 'core/quote': { - color: { - text: 'black', - background: 'white', - }, - typography: { - fontSize: '20px', - }, - }, - }, - }, - }, - { - title: 'Title 2', - description: 'Description 2', - settings: { - color: { - duotone: [ - { - name: 'Boom', - colors: [ '#000000', '#7f7f7f' ], - slug: 'boom', - }, - { - name: 'Gray to white', - colors: [ '#000000', '#ffffff' ], - slug: 'gray-to-white', - }, - { - name: 'Whatever to whatever', - colors: [ '#8c00b7', '#fcff41' ], - slug: 'whatever-to-whatever', - }, - ], - gradients: [ - { - name: 'Jam in the office', - gradient: - 'linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%)', - slug: 'jam-in-the-office', - }, - { - name: 'Open source', - gradient: - 'linear-gradient(135deg,rgb(122,220,180) 0%,rgb(0,208,130) 100%)', - slug: 'open-source', - }, - { - name: 'Here to there', - gradient: - 'linear-gradient(135deg,rgba(252,185,0,1) 0%,rgba(255,105,0,1) 100%)', - slug: 'here-to-there', - }, - ], - palette: [ - { - name: 'Chunky Bacon', - slug: 'chunky-bacon', - color: '#cf2e2e', - }, - { - name: 'Burrito', - slug: 'burrito', - color: '#ff6900', - }, - { - name: 'Dinosaur', - slug: 'dinosaur', - color: '#fcb900', - }, - ], - }, - typography: { - fontSizes: [ - { - name: 'Smallish', - slug: 'smallish', - size: '15px', - }, - { - name: 'Mediumish', - slug: 'mediumish', - size: '22px', - }, - { - name: 'Largish', - slug: 'largish', - size: '44px', - }, - ], - }, - layout: { - contentSize: '300px', - }, - }, - styles: { - typography: { - letterSpacing: '3px', - }, - color: { - backgroundColor: 'red', - text: 'orange', - }, - elements: { - link: { - typography: { - textDecoration: 'underline', - }, - }, - }, - blocks: { - 'core/paragraph': { - color: { - text: 'purple', - background: 'green', - }, - typography: { - fontSize: '20px', - }, - }, - }, - }, - }, - ]; - const mockBaseVariation = { - settings: { - typography: { - fontFamilies: { - custom: [ - { - name: 'ADLaM Display', - fontFamily: 'ADLaM Display, system-ui', - slug: 'adlam-display', - fontFace: [ - { - src: 'adlam.woff2', - fontWeight: '400', - fontStyle: 'normal', - fontFamily: 'ADLaM Display', - }, - ], - }, - ], - }, - fontSizes: [ - { - name: 'Base small', - slug: 'base-small', - size: '1px', - }, - { - name: 'Base medium', - slug: 'base-medium', - size: '2px', - }, - { - name: 'Base large', - slug: 'base-large', - size: '3px', - }, - ], - }, - color: { - palette: { - custom: [ - { - color: '#c42727', - name: 'Color 1', - slug: 'custom-color-1', - }, - { - color: '#3b0f0f', - name: 'Color 2', - slug: 'custom-color-2', - }, - ], - }, - }, - layout: { - wideSize: '1137px', - contentSize: '400px', - }, - }, - styles: { - typography: { - fontSize: '12px', - lineHeight: '1.5', - }, - color: { - backgroundColor: 'cheese', - color: 'lettuce', - }, - blocks: { - 'core/quote': { - color: { - text: 'hello', - background: 'dolly', - }, - typography: { - fontSize: '111111px', - }, - }, - 'core/group': { - typography: { - fontFamily: 'var:preset|font-family|system-sans-serif', - }, - }, - }, - }, - }; - - it( 'should return variations if property is falsy', () => { - const { result } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations: mockVariations, - property: '', - } ) - ); - - expect( result.current ).toEqual( mockVariations ); - } ); - - it( 'should return variations if variations is empty or falsy', () => { - const { result: emptyResult } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations: [], - property: 'layout', - } ) - ); - - expect( emptyResult.current ).toEqual( [] ); - - const { result: falsyResult } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations: null, - property: 'layout', - } ) - ); - - expect( falsyResult.current ).toEqual( null ); - } ); - - it( 'should return new, unreferenced object', () => { - const variations = [ - { - title: 'hey', - description: 'ho', - joe: { - where: { - you: 'going with that unit test in your hand', - }, - }, - }, - ]; - const { result } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations, - property: 'where', - } ) - ); - - expect( result.current ).toEqual( [ - { - title: 'hey', - description: 'ho', - joe: { - where: { - you: 'going with that unit test in your hand', - }, - }, - }, - ] ); - - expect( result.current[ 0 ].joe.where ).not.toBe( - variations[ 0 ].joe.where - ); - expect( result.current[ 0 ].joe ).not.toBe( variations[ 0 ].joe ); - } ); - - it( "should return the variation's typography properties", () => { - const { result } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations: mockVariations, - property: 'typography', - } ) - ); - - expect( result.current ).toEqual( [ - { - title: 'Title 1', - description: 'Description 1', - settings: { - typography: { - fluid: true, - fontFamilies: { - theme: [ - { - name: 'Inter san-serif', - fontFamily: 'Inter san-serif', - slug: 'inter-san-serif', - fontFace: [ - { - src: 'inter-san-serif.woff2', - fontWeight: '400', - fontStyle: 'italic', - fontFamily: 'Inter san-serif', - }, - ], - }, - ], - }, - fontSizes: [ - { - name: 'Small', - slug: 'small', - size: '13px', - }, - { - name: 'Medium', - slug: 'medium', - size: '20px', - }, - { - name: 'Large', - slug: 'large', - size: '36px', - }, - ], - }, - }, - styles: { - typography: { - letterSpacing: '3px', - }, - blocks: { - 'core/quote': { - typography: { - fontSize: '20px', - }, - }, - }, - }, - }, - { - title: 'Title 2', - description: 'Description 2', - settings: { - typography: { - fontSizes: [ - { - name: 'Smallish', - slug: 'smallish', - size: '15px', - }, - { - name: 'Mediumish', - slug: 'mediumish', - size: '22px', - }, - { - name: 'Largish', - slug: 'largish', - size: '44px', - }, - ], - }, - }, - styles: { - typography: { - letterSpacing: '3px', - }, - elements: { - link: { - typography: { - textDecoration: 'underline', - }, - }, - }, - blocks: { - 'core/paragraph': { - typography: { - fontSize: '20px', - }, - }, - }, - }, - }, - ] ); - } ); - - it( "should return the variation's color properties", () => { - const { result } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations: mockVariations, - property: 'color', - } ) - ); - - expect( result.current ).toEqual( [ - { - title: 'Title 1', - description: 'Description 1', - settings: { - color: { - duotone: [ - { - name: 'Dark grayscale', - colors: [ '#000000', '#7f7f7f' ], - slug: 'dark-grayscale', - }, - { - name: 'Grayscale', - colors: [ '#000000', '#ffffff' ], - slug: 'grayscale', - }, - { - name: 'Purple and yellow', - colors: [ '#8c00b7', '#fcff41' ], - slug: 'purple-yellow', - }, - ], - gradients: [ - { - name: 'Vivid cyan blue to vivid purple', - gradient: - 'linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%)', - slug: 'vivid-cyan-blue-to-vivid-purple', - }, - { - name: 'Light green cyan to vivid green cyan', - gradient: - 'linear-gradient(135deg,rgb(122,220,180) 0%,rgb(0,208,130) 100%)', - slug: 'light-green-cyan-to-vivid-green-cyan', - }, - { - name: 'Luminous vivid amber to luminous vivid orange', - gradient: - 'linear-gradient(135deg,rgba(252,185,0,1) 0%,rgba(255,105,0,1) 100%)', - slug: 'luminous-vivid-amber-to-luminous-vivid-orange', - }, - ], - palette: [ - { - name: 'Vivid red', - slug: 'vivid-red', - color: '#cf2e2e', - }, - { - name: 'Luminous vivid orange', - slug: 'luminous-vivid-orange', - color: '#ff6900', - }, - { - name: 'Luminous vivid amber', - slug: 'luminous-vivid-amber', - color: '#fcb900', - }, - ], - }, - }, - styles: { - color: { - backgroundColor: 'red', - color: 'orange', - }, - elements: { - cite: { - color: { - text: 'white', - }, - }, - }, - blocks: { - 'core/quote': { - color: { - text: 'black', - background: 'white', - }, - }, - }, - }, - }, - { - title: 'Title 2', - description: 'Description 2', - settings: { - color: { - duotone: [ - { - name: 'Boom', - colors: [ '#000000', '#7f7f7f' ], - slug: 'boom', - }, - { - name: 'Gray to white', - colors: [ '#000000', '#ffffff' ], - slug: 'gray-to-white', - }, - { - name: 'Whatever to whatever', - colors: [ '#8c00b7', '#fcff41' ], - slug: 'whatever-to-whatever', - }, - ], - gradients: [ - { - name: 'Jam in the office', - gradient: - 'linear-gradient(135deg,rgba(6,147,227,1) 0%,rgb(155,81,224) 100%)', - slug: 'jam-in-the-office', - }, - { - name: 'Open source', - gradient: - 'linear-gradient(135deg,rgb(122,220,180) 0%,rgb(0,208,130) 100%)', - slug: 'open-source', - }, - { - name: 'Here to there', - gradient: - 'linear-gradient(135deg,rgba(252,185,0,1) 0%,rgba(255,105,0,1) 100%)', - slug: 'here-to-there', - }, - ], - palette: [ - { - name: 'Chunky Bacon', - slug: 'chunky-bacon', - color: '#cf2e2e', - }, - { - name: 'Burrito', - slug: 'burrito', - color: '#ff6900', - }, - { - name: 'Dinosaur', - slug: 'dinosaur', - color: '#fcb900', - }, - ], - }, - }, - styles: { - color: { - backgroundColor: 'red', - text: 'orange', - }, - blocks: { - 'core/paragraph': { - color: { - text: 'purple', - background: 'green', - }, - }, - }, - }, - }, - ] ); - } ); - - it( 'should merge the user styles and settings with the supplied variation, but only for the specified property', () => { - const { result } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations: [ mockVariations[ 0 ] ], - property: 'typography', - baseVariation: mockBaseVariation, - } ) - ); - - expect( result.current ).toEqual( [ - { - title: 'Title 1', - description: 'Description 1', - settings: { - typography: { - fluid: true, - fontFamilies: { - theme: [ - { - name: 'Inter san-serif', - fontFamily: 'Inter san-serif', - slug: 'inter-san-serif', - fontFace: [ - { - src: 'inter-san-serif.woff2', - fontWeight: '400', - fontStyle: 'italic', - fontFamily: 'Inter san-serif', - }, - ], - }, - ], - custom: [ - { - name: 'ADLaM Display', - fontFamily: 'ADLaM Display, system-ui', - slug: 'adlam-display', - fontFace: [ - { - src: 'adlam.woff2', - fontWeight: '400', - fontStyle: 'normal', - fontFamily: 'ADLaM Display', - }, - ], - }, - ], - }, - fontSizes: [ - { - name: 'Small', - slug: 'small', - size: '13px', - }, - { - name: 'Medium', - slug: 'medium', - size: '20px', - }, - { - name: 'Large', - slug: 'large', - size: '36px', - }, - ], - }, - color: { - palette: { - custom: [ - { - color: '#c42727', - name: 'Color 1', - slug: 'custom-color-1', - }, - { - color: '#3b0f0f', - name: 'Color 2', - slug: 'custom-color-2', - }, - ], - }, - }, - layout: { - wideSize: '1137px', - contentSize: '400px', - }, - }, - styles: { - color: { - backgroundColor: 'cheese', - color: 'lettuce', - }, - typography: { - fontSize: '12px', - letterSpacing: '3px', - lineHeight: '1.5', - }, - blocks: { - 'core/quote': { - color: { - text: 'hello', - background: 'dolly', - }, - typography: { - fontSize: '20px', - }, - }, - 'core/group': { - typography: { - fontFamily: - 'var:preset|font-family|system-sans-serif', - }, - }, - }, - }, - }, - ] ); - } ); - - it( 'should filter the output and return only variations that match filter', () => { - const { result } = renderHook( () => - useThemeStyleVariationsByProperty( { - variations: mockVariations, - property: 'typography', - filter: ( variation ) => - !! variation?.settings?.typography?.fontFamilies?.theme - ?.length, - } ) - ); - expect( result.current ).toEqual( [ - { - title: 'Title 1', - description: 'Description 1', - settings: { - typography: { - fluid: true, - fontFamilies: { - theme: [ - { - name: 'Inter san-serif', - fontFamily: 'Inter san-serif', - slug: 'inter-san-serif', - fontFace: [ - { - src: 'inter-san-serif.woff2', - fontWeight: '400', - fontStyle: 'italic', - fontFamily: 'Inter san-serif', - }, - ], - }, - ], - }, - fontSizes: [ - { - name: 'Small', - slug: 'small', - size: '13px', - }, - { - name: 'Medium', - slug: 'medium', - size: '20px', - }, - { - name: 'Large', - slug: 'large', - size: '36px', - }, - ], - }, - }, - styles: { - typography: { - letterSpacing: '3px', - }, - blocks: { - 'core/quote': { - typography: { - fontSize: '20px', - }, - }, - }, - }, - }, - ] ); - } ); -} ); - describe( 'removePropertyFromObject', () => { const mockBaseVariation = { settings: { diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 8f5088fc6b2fa..35928ad774e46 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -120,98 +120,6 @@ export const filterObjectByProperty = ( object, property ) => { return newObject; }; -/** - * Returns a new object with only the properties specified in `property`. - * Optional merges the baseVariation object with the variation object. - * Note: this function will only overwrite the specified property in baseVariation if it exists. - * The baseVariation will not be otherwise modified. To strip a property from the baseVariation object, use `removePropertyFromObject`. - * See useCurrentMergeThemeStyleVariationsWithUserConfig for an example of how to use this function. - * - * @param {Object} props Object of hook args. - * @param {Object[]} props.variations The theme style variations to filter. - * @param {string} props.property The property to filter by. - * @param {Function} props.filter Optional. The filter function to apply to the variations. - * @param {Object} props.baseVariation Optional. Base or user settings to be updated with variation properties. - * @return {Object[]|*} The merged object. - */ -export default function useThemeStyleVariationsByProperty( { - variations, - property, - filter, - baseVariation, -} ) { - return useMemo( () => { - if ( ! property || ! variations || variations?.length === 0 ) { - return variations; - } - - const clonedBaseVariation = - typeof baseVariation === 'object' && - Object.keys( baseVariation ).length > 0 - ? cloneDeep( baseVariation ) - : null; - - let processedStyleVariations = variations.reduce( - ( accumulator, variation ) => { - const variationFilteredByProperty = filterObjectByProperty( - cloneDeep( variation ), - property - ); - - // Remove variations that are empty once the property is filtered out. - if ( - variation.title !== __( 'Default' ) && - Object.keys( variationFilteredByProperty ).length === 0 - ) { - return accumulator; - } - - let result = { - ...variationFilteredByProperty, - title: variation?.title, - description: variation?.description, - }; - - if ( clonedBaseVariation ) { - /* - * Overwrites all baseVariation object `styleProperty` properties - * with the theme variation `styleProperty` properties. - */ - result = mergeBaseAndUserConfigs( - clonedBaseVariation, - result - ); - } - - // Detect if this is a duplicate variation. - const isDuplicate = accumulator.some( ( item ) => { - return ( - JSON.stringify( item.styles ) === - JSON.stringify( result?.styles ) && - JSON.stringify( item.settings ) === - JSON.stringify( result?.settings ) - ); - } ); - if ( isDuplicate ) { - return accumulator; - } - - // If the variation is not a duplicate, add it to the accumulator. - accumulator.push( result ); - return accumulator; - }, - [] // Initial accumulator value. - ); - - if ( 'function' === typeof filter ) { - processedStyleVariations = - processedStyleVariations.filter( filter ); - } - - return processedStyleVariations; - }, [ variations, property, baseVariation, filter ] ); -} - /** * Compares a style variation to the same variation filtered by a single property. * Returns true if the variation contains only the property specified. From 6f91bb7cd89bc90b52ab66e184602c822729ec78 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Thu, 13 Jun 2024 12:54:13 +0100 Subject: [PATCH 05/11] update comment --- .../use-theme-style-variations-by-property.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 35928ad774e46..497a05298913c 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -48,8 +48,8 @@ export function removePropertyFromObject( object, property ) { } /** - * A convenience wrapper for `useThemeStyleVariationsByProperty()` that fetches the current theme style variations, - * and user-defined global style/settings object. + * Fetches the current theme style variations that contain only the specified property + * and merges them with the user config. * * @param {Object} props Object of hook args. * @param {string} props.property The property to filter by. From 9059e3b101db408933f3acc541e2120e766bf127 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Thu, 13 Jun 2024 13:13:17 +0100 Subject: [PATCH 06/11] fix typo --- .../use-theme-style-variations-by-property.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 497a05298913c..b351451821fd7 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -69,14 +69,14 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { }; }, [] ); const { user: baseVariation } = useContext( GlobalStylesContext ); - const clondedBaseVariation = cloneDeep( baseVariation ); + const clonedBaseVariation = cloneDeep( baseVariation ); const variationsWithSinglePropertyAndBase = variationsFromTheme .filter( ( variation ) => { return isVariationWithSingleProperty( variation, property ); } ) .map( ( variation ) => { - return mergeBaseAndUserConfigs( clondedBaseVariation, variation ); + return mergeBaseAndUserConfigs( clonedBaseVariation, variation ); } ); return useMemo( () => { From fc86da8727c295db4b44eacd116425ae7bcc5b15 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Thu, 13 Jun 2024 15:52:20 +0100 Subject: [PATCH 07/11] Add the user settings to the base variation --- .../use-theme-style-variations-by-property.js | 37 ++++++++++--------- 1 file changed, 19 insertions(+), 18 deletions(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index b351451821fd7..d8d1d662ba2b1 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -69,26 +69,27 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { }; }, [] ); const { user: baseVariation } = useContext( GlobalStylesContext ); - const clonedBaseVariation = cloneDeep( baseVariation ); - - const variationsWithSinglePropertyAndBase = variationsFromTheme - .filter( ( variation ) => { - return isVariationWithSingleProperty( variation, property ); - } ) - .map( ( variation ) => { - return mergeBaseAndUserConfigs( clonedBaseVariation, variation ); - } ); return useMemo( () => { - return [ - { - title: __( 'Default' ), - settings: {}, - styles: {}, - }, - ...variationsWithSinglePropertyAndBase, - ]; - }, [ variationsWithSinglePropertyAndBase ] ); + const clonedBaseVariation = cloneDeep( baseVariation ); + + // Get user variation and remove the color settings. + const userVariation = removePropertyFromObject( + clonedBaseVariation, + property + ); + userVariation.title = __( 'Default' ); + + const variationsWithSinglePropertyAndBase = variationsFromTheme + .filter( ( variation ) => { + return isVariationWithSingleProperty( variation, property ); + } ) + .map( ( variation ) => { + return mergeBaseAndUserConfigs( baseVariation, variation ); + } ); + + return [ userVariation, ...variationsWithSinglePropertyAndBase ]; + }, [ baseVariation, variationsFromTheme ] ); } /** From 621465737b01a1d1cb94638ece5f1e405d08ee53 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Fri, 14 Jun 2024 10:59:40 +0100 Subject: [PATCH 08/11] Update packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js Co-authored-by: Alex Lende --- .../use-theme-style-variations-by-property.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index d8d1d662ba2b1..7c5deb7577df9 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -89,7 +89,7 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { } ); return [ userVariation, ...variationsWithSinglePropertyAndBase ]; - }, [ baseVariation, variationsFromTheme ] ); + }, [ property, baseVariation, variationsFromTheme ] ); } /** From e626ce167c1b42044d488deb15f986b439c5dfea Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Fri, 14 Jun 2024 10:59:47 +0100 Subject: [PATCH 09/11] Update packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js Co-authored-by: Alex Lende --- .../use-theme-style-variations-by-property.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 7c5deb7577df9..3419cf512ee0a 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -73,7 +73,7 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { return useMemo( () => { const clonedBaseVariation = cloneDeep( baseVariation ); - // Get user variation and remove the color settings. + // Get user variation and remove the settings for the given property. const userVariation = removePropertyFromObject( clonedBaseVariation, property From e8827a7579aef93dab16172fe85765ee876cb3f2 Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Fri, 14 Jun 2024 13:37:48 +0100 Subject: [PATCH 10/11] Remove the property from the user variation before merginng it with the new variation --- .../use-theme-style-variations-by-property.js | 20 ++++++++++++------- 1 file changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 3419cf512ee0a..6449b5b342332 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -68,14 +68,14 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { variationsFromTheme: _variationsFromTheme || [], }; }, [] ); - const { user: baseVariation } = useContext( GlobalStylesContext ); + const { user: userVariation } = useContext( GlobalStylesContext ); return useMemo( () => { - const clonedBaseVariation = cloneDeep( baseVariation ); + const clonedUserVariation = cloneDeep( userVariation ); // Get user variation and remove the settings for the given property. - const userVariation = removePropertyFromObject( - clonedBaseVariation, + const userVariationWithoutProperty = removePropertyFromObject( + clonedUserVariation, property ); userVariation.title = __( 'Default' ); @@ -85,11 +85,17 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { return isVariationWithSingleProperty( variation, property ); } ) .map( ( variation ) => { - return mergeBaseAndUserConfigs( baseVariation, variation ); + return mergeBaseAndUserConfigs( + userVariationWithoutProperty, + variation + ); } ); - return [ userVariation, ...variationsWithSinglePropertyAndBase ]; - }, [ property, baseVariation, variationsFromTheme ] ); + return [ + userVariationWithoutProperty, + ...variationsWithSinglePropertyAndBase, + ]; + }, [ property, userVariation, variationsFromTheme ] ); } /** From b3388de2f837ea21785a97fd3c432f767fb769cf Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Fri, 14 Jun 2024 13:40:17 +0100 Subject: [PATCH 11/11] apply the name to the correct object --- .../use-theme-style-variations-by-property.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index 6449b5b342332..401a7e1a71dfd 100644 --- a/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -78,7 +78,7 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { clonedUserVariation, property ); - userVariation.title = __( 'Default' ); + userVariationWithoutProperty.title = __( 'Default' ); const variationsWithSinglePropertyAndBase = variationsFromTheme .filter( ( variation ) => {