From d0a9d128e492cd1b7a52b180052a78e818f04916 Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 8 Feb 2024 15:51:58 +1100 Subject: [PATCH 01/10] First commit Pulling out the utils from https://github.com/WordPress/gutenberg/pull/56622 so we test separately --- .../use-theme-style-variations-by-property.js | 125 ++++++++++++++++++ .../push-changes-to-global-styles/index.js | 5 +- packages/edit-site/src/utils/clone-deep.js | 8 ++ 3 files changed, 134 insertions(+), 4 deletions(-) create mode 100644 packages/edit-site/src/components/global-styles/use-theme-style-variations-by-property.js create mode 100644 packages/edit-site/src/utils/clone-deep.js diff --git a/packages/edit-site/src/components/global-styles/use-theme-style-variations-by-property.js b/packages/edit-site/src/components/global-styles/use-theme-style-variations-by-property.js new file mode 100644 index 00000000000000..f7305561680c1c --- /dev/null +++ b/packages/edit-site/src/components/global-styles/use-theme-style-variations-by-property.js @@ -0,0 +1,125 @@ +/** + * WordPress dependencies + */ +import { useSelect } from '@wordpress/data'; +import { store as coreStore } from '@wordpress/core-data'; +import { useContext, useMemo } from '@wordpress/element'; +import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; + +/** + * Internal dependencies + */ +import { unlock } from '../../lock-unlock'; +import cloneDeep from '../../utils/clone-deep'; +import { mergeBaseAndUserConfigs } from './global-styles-provider'; +/** + * Returns a new object with only the properties specified in `properties`. + * + * @param {Object} object The object to filter + * @param {Object} property The property to filter by + * @return {Object} The merged object. + */ +export const filterObjectByProperty = ( object, property ) => { + if ( ! object ) { + return {}; + } + + const newObject = {}; + Object.keys( object ).forEach( ( key ) => { + if ( key === property ) { + newObject[ key ] = object[ key ]; + } else if ( typeof object[ key ] === 'object' ) { + const newFilter = filterObjectByProperty( object[ key ], property ); + if ( Object.keys( newFilter ).length ) { + newObject[ key ] = newFilter; + } + } + } ); + return newObject; +}; + +/** + * Removes all instances of a property from an object. + * + * @param {Object} object + * @param {string} property + * @return {Object} The modified object. + */ +const removePropertyFromObject = ( object, property ) => { + for ( const key in object ) { + if ( key === property ) { + delete object[ key ]; + } else if ( typeof object[ key ] === 'object' ) { + removePropertyFromObject( object[ key ], property ); + } + } + return object; +}; + +/** + * Return style variations with all properties removed except for the one specified in `type`. + * + * @param {Object} user The user variation. + * @param {Array} variations The other style variations. + * @param {string} property The property to filter by. + * + * @return {Array} The style variation with only the specified property filtered. + */ +export const getVariationsByProperty = ( user, variations, property ) => { + const userSettingsWithoutProperty = removePropertyFromObject( + cloneDeep( user ), + property + ); + + const variationsWithOnlyProperty = variations.map( ( variation ) => { + return { + ...filterObjectByProperty( variation, property ), + // Add variation title and description to every variation item. + title: variation?.title, + description: variation?.description, + }; + } ); + + return variationsWithOnlyProperty.map( ( variation ) => + mergeBaseAndUserConfigs( userSettingsWithoutProperty, variation ) + ); +}; + +const { GlobalStylesContext } = unlock( blockEditorPrivateApis ); + +export default function useThemeStyleVariationsByProperty( { + styleProperty, + filter, +} ) { + const variations = useSelect( ( select ) => { + return select( + coreStore + ).__experimentalGetCurrentThemeGlobalStylesVariations(); + }, [] ); + const { user } = useContext( GlobalStylesContext ); + + return useMemo( () => { + if ( ! styleProperty || ! variations.length ) { + return []; + } + /* + @TODO: + For colors, should also get filter? + Memoize/cache all this better? E.g., should we memoize the variations? + Test with empty theme + Test with 2022 - typography font families bork for some reason + + */ + let styleVariations = getVariationsByProperty( + user, + variations, + styleProperty + ); + + if ( 'function' === typeof filter ) { + styleVariations = styleVariations.filter( filter ); + } + + return styleVariations; + }, [ styleProperty, variations, filter ] ); +} diff --git a/packages/edit-site/src/hooks/push-changes-to-global-styles/index.js b/packages/edit-site/src/hooks/push-changes-to-global-styles/index.js index 4844ec6d03eb5e..21f19201b75108 100644 --- a/packages/edit-site/src/hooks/push-changes-to-global-styles/index.js +++ b/packages/edit-site/src/hooks/push-changes-to-global-styles/index.js @@ -26,6 +26,7 @@ import { store as coreStore } from '@wordpress/core-data'; */ import { useSupportedStyles } from '../../components/global-styles/hooks'; import { unlock } from '../../lock-unlock'; +import cloneDeep from '../../utils/clone-deep'; const { cleanEmptyObject, GlobalStylesContext } = unlock( blockEditorPrivateApis @@ -275,10 +276,6 @@ function setNestedValue( object, path, value ) { return object; } -function cloneDeep( object ) { - return ! object ? {} : JSON.parse( JSON.stringify( object ) ); -} - function PushChangesToGlobalStylesControl( { name, attributes, diff --git a/packages/edit-site/src/utils/clone-deep.js b/packages/edit-site/src/utils/clone-deep.js new file mode 100644 index 00000000000000..149e1df2408ea7 --- /dev/null +++ b/packages/edit-site/src/utils/clone-deep.js @@ -0,0 +1,8 @@ +/** + * Makes a copy of an object without storing any references to the original object. + * @param {Object} object + * @return {Object} The cloned object. + */ +export default function cloneDeep( object ) { + return ! object ? {} : JSON.parse( JSON.stringify( object ) ); +} From 73c62b951666cd901c06dcd21335ab0fd889541a Mon Sep 17 00:00:00 2001 From: ramon Date: Thu, 8 Feb 2024 17:26:08 +1100 Subject: [PATCH 02/10] Adding failing tests :) --- .../use-theme-style-variations-by-property.js | 105 ++++++++++++++++++ .../use-theme-style-variations-by-property.js | 3 +- 2 files changed, 107 insertions(+), 1 deletion(-) create mode 100644 packages/edit-site/src/hooks/use-theme-style-variations/test/use-theme-style-variations-by-property.js rename packages/edit-site/src/{components/global-styles => hooks/use-theme-style-variations}/use-theme-style-variations-by-property.js (97%) 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 new file mode 100644 index 00000000000000..de6ba83e40e480 --- /dev/null +++ b/packages/edit-site/src/hooks/use-theme-style-variations/test/use-theme-style-variations-by-property.js @@ -0,0 +1,105 @@ +/** + * External dependencies + */ +import { renderHook } from '@testing-library/react'; + +/** + * WordPress dependencies + */ +import { useSelect } from '@wordpress/data'; + +/** + * Internal dependencies + */ +import useThemeStyleVariationsByProperty, { + getVariationsByProperty, +} from '../use-theme-style-variations-by-property'; + +jest.mock( '@wordpress/data/src/components/use-select', () => jest.fn() ); +jest.mock( '@wordpress/element', () => { + return { + __esModule: true, + ...jest.requireActual( '@wordpress/element' ), + useContext: jest.fn().mockImplementation( () => ( { + user: { + styles: { + typography: { + 'font-size': '12px', + 'line-height': '1.5', + }, + color: { + 'background-color': 'red', + color: 'blue', + }, + }, + settings: { + typography: { + 'font-size': '14px', + 'line-height': '1.6', + }, + color: { + 'background-color': 'green', + color: 'yellow', + }, + }, + }, + } ) ), + }; +} ); + +describe( 'useThemeStyleVariationsByProperty', () => { + const mockVariations = [ + { + title: 'Title 1', + description: 'Description 1', + settings: {}, + styles: { + typography: { + 'letter-spacing': '3px', + }, + color: { + 'background-color': 'red', + color: 'orange', + }, + }, + }, + { + title: 'Title 2', + description: 'Description 2', + settings: {}, + styles: { + typography: { + 'letter-spacing': '1px', + }, + color: { + color: 'pink', + }, + }, + }, + ]; + + it( "should return the variation's typography properties", () => { + useSelect.mockImplementation( () => mockVariations ); + const { result } = renderHook( () => + useThemeStyleVariationsByProperty( { styleProperty: 'typography' } ) + ); + expect( result.current ).toEqual( [ + { + description: 'Description 1', + settings: {}, + styles: { + typography: { 'letter-spacing': '3px' }, + }, + title: 'Title 1', + }, + { + description: 'Description 2', + settings: {}, + styles: { + typography: { 'letter-spacing': '1px' }, + }, + title: 'Title 2', + }, + ] ); + } ); +} ); diff --git a/packages/edit-site/src/components/global-styles/use-theme-style-variations-by-property.js b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js similarity index 97% rename from packages/edit-site/src/components/global-styles/use-theme-style-variations-by-property.js rename to packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js index f7305561680c1c..55c59b7ff96399 100644 --- a/packages/edit-site/src/components/global-styles/use-theme-style-variations-by-property.js +++ b/packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js @@ -11,7 +11,7 @@ import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; */ import { unlock } from '../../lock-unlock'; import cloneDeep from '../../utils/clone-deep'; -import { mergeBaseAndUserConfigs } from './global-styles-provider'; +import { mergeBaseAndUserConfigs } from '../../components/global-styles/global-styles-provider'; /** * Returns a new object with only the properties specified in `properties`. * @@ -96,6 +96,7 @@ export default function useThemeStyleVariationsByProperty( { coreStore ).__experimentalGetCurrentThemeGlobalStylesVariations(); }, [] ); + const { user } = useContext( GlobalStylesContext ); return useMemo( () => { From e97054aee203882033258d8c26c4b76ac21824d8 Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Feb 2024 13:14:55 +1100 Subject: [PATCH 03/10] Adds working tests Removes extra functions - now internal to the hook --- .../use-theme-style-variations-by-property.js | 811 ++++++++++++++++-- .../use-theme-style-variations-by-property.js | 116 +-- 2 files changed, 797 insertions(+), 130 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 de6ba83e40e480..c22a3c8e9dbd9a 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 @@ -3,102 +3,807 @@ */ import { renderHook } from '@testing-library/react'; -/** - * WordPress dependencies - */ -import { useSelect } from '@wordpress/data'; - /** * Internal dependencies */ -import useThemeStyleVariationsByProperty, { - getVariationsByProperty, -} from '../use-theme-style-variations-by-property'; - -jest.mock( '@wordpress/data/src/components/use-select', () => jest.fn() ); -jest.mock( '@wordpress/element', () => { - return { - __esModule: true, - ...jest.requireActual( '@wordpress/element' ), - useContext: jest.fn().mockImplementation( () => ( { - user: { - styles: { - typography: { - 'font-size': '12px', - 'line-height': '1.5', - }, - color: { - 'background-color': 'red', - color: 'blue', - }, - }, - settings: { - typography: { - 'font-size': '14px', - 'line-height': '1.6', - }, - color: { - 'background-color': 'green', - color: 'yellow', - }, - }, - }, - } ) ), - }; -} ); +import useThemeStyleVariationsByProperty from '../use-theme-style-variations-by-property'; describe( 'useThemeStyleVariationsByProperty', () => { const mockVariations = [ { title: 'Title 1', description: 'Description 1', - settings: {}, + 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: { - 'letter-spacing': '3px', + letterSpacing: '3px', }, color: { - 'background-color': 'red', + 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: {}, + 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: { - 'letter-spacing': '1px', + letterSpacing: '3px', }, color: { - color: 'pink', + 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 the variation's typography properties", () => { - useSelect.mockImplementation( () => mockVariations ); const { result } = renderHook( () => - useThemeStyleVariationsByProperty( { styleProperty: 'typography' } ) + useThemeStyleVariationsByProperty( { + styleVariations: mockVariations, + styleProperty: 'typography', + } ) ); + expect( result.current ).toEqual( [ { + title: 'Title 1', description: 'Description 1', - settings: {}, + 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: { 'letter-spacing': '3px' }, + typography: { + letterSpacing: '3px', + }, + blocks: { + 'core/quote': { + typography: { + fontSize: '20px', + }, + }, + }, }, - title: 'Title 1', }, { + title: 'Title 2', description: 'Description 2', - settings: {}, + 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( { + styleVariations: mockVariations, + styleProperty: '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: { - typography: { 'letter-spacing': '1px' }, + 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( { + styleVariations: [ mockVariations[ 0 ] ], + styleProperty: 'typography', + mergeWith: 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( { + styleVariations: mockVariations, + styleProperty: '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', + }, + }, + }, + }, }, ] ); } ); 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 55c59b7ff96399..1d5a257fa7d91d 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 @@ -1,17 +1,14 @@ /** * WordPress dependencies */ -import { useSelect } from '@wordpress/data'; -import { store as coreStore } from '@wordpress/core-data'; -import { useContext, useMemo } from '@wordpress/element'; -import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; +import { useMemo } from '@wordpress/element'; /** * Internal dependencies */ -import { unlock } from '../../lock-unlock'; -import cloneDeep from '../../utils/clone-deep'; import { mergeBaseAndUserConfigs } from '../../components/global-styles/global-styles-provider'; +import cloneDeep from '../../utils/clone-deep'; + /** * Returns a new object with only the properties specified in `properties`. * @@ -39,88 +36,53 @@ export const filterObjectByProperty = ( object, property ) => { }; /** - * Removes all instances of a property from an object. - * - * @param {Object} object - * @param {string} property - * @return {Object} The modified object. - */ -const removePropertyFromObject = ( object, property ) => { - for ( const key in object ) { - if ( key === property ) { - delete object[ key ]; - } else if ( typeof object[ key ] === 'object' ) { - removePropertyFromObject( object[ key ], property ); - } - } - return object; -}; - -/** - * Return style variations with all properties removed except for the one specified in `type`. - * - * @param {Object} user The user variation. - * @param {Array} variations The other style variations. - * @param {string} property The property to filter by. + * Returns a new object with only the properties specified in `properties`. * - * @return {Array} The style variation with only the specified property filtered. + * @param {Object} props Object of hook args. + * @param {Object[]} props.styleVariations The theme style variations to filter. + * @param {string} props.styleProperty 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 const getVariationsByProperty = ( user, variations, property ) => { - const userSettingsWithoutProperty = removePropertyFromObject( - cloneDeep( user ), - property - ); - - const variationsWithOnlyProperty = variations.map( ( variation ) => { - return { - ...filterObjectByProperty( variation, property ), - // Add variation title and description to every variation item. - title: variation?.title, - description: variation?.description, - }; - } ); - - return variationsWithOnlyProperty.map( ( variation ) => - mergeBaseAndUserConfigs( userSettingsWithoutProperty, variation ) - ); -}; - -const { GlobalStylesContext } = unlock( blockEditorPrivateApis ); - export default function useThemeStyleVariationsByProperty( { + styleVariations, styleProperty, filter, + baseVariation, } ) { - const variations = useSelect( ( select ) => { - return select( - coreStore - ).__experimentalGetCurrentThemeGlobalStylesVariations(); - }, [] ); - - const { user } = useContext( GlobalStylesContext ); - return useMemo( () => { - if ( ! styleProperty || ! variations.length ) { - return []; + if ( ! styleProperty || styleVariations?.length === 0 ) { + return styleVariations; } - /* - @TODO: - For colors, should also get filter? - Memoize/cache all this better? E.g., should we memoize the variations? - Test with empty theme - Test with 2022 - typography font families bork for some reason - */ - let styleVariations = getVariationsByProperty( - user, - variations, - styleProperty - ); + let processedStyleVariations = styleVariations.map( ( variation ) => ( { + ...filterObjectByProperty( variation, styleProperty ), + // Add variation title and description to every variation item. + title: variation?.title, + description: variation?.description, + } ) ); + + if ( + typeof baseVariation === 'object' && + Object.keys( baseVariation ).length > 0 + ) { + /* + * Overwrites all baseVariation object `styleProperty` properties + * with the theme variation `styleProperty` properties. + */ + const clonedBaseVariation = cloneDeep( baseVariation ); + processedStyleVariations = processedStyleVariations.map( + ( variation ) => + mergeBaseAndUserConfigs( clonedBaseVariation, variation ) + ); + } if ( 'function' === typeof filter ) { - styleVariations = styleVariations.filter( filter ); + processedStyleVariations = + processedStyleVariations.filter( filter ); } - return styleVariations; - }, [ styleProperty, variations, filter ] ); + return processedStyleVariations; + }, [ styleVariations, styleProperty, baseVariation, filter ] ); } From 28b15645bc9fe61d9c0dc9d693261e13bb8de0af Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Feb 2024 13:33:11 +1100 Subject: [PATCH 04/10] Adds more tests --- .../use-theme-style-variations-by-property.js | 49 +++++++++++++++---- .../use-theme-style-variations-by-property.js | 26 +++++----- 2 files changed, 53 insertions(+), 22 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 c22a3c8e9dbd9a..52594fffd915c4 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 @@ -338,11 +338,42 @@ describe( 'useThemeStyleVariationsByProperty', () => { }, }; + 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 the variation's typography properties", () => { const { result } = renderHook( () => useThemeStyleVariationsByProperty( { - styleVariations: mockVariations, - styleProperty: 'typography', + variations: mockVariations, + property: 'typography', } ) ); @@ -452,8 +483,8 @@ describe( 'useThemeStyleVariationsByProperty', () => { it( "should return the variation's color properties", () => { const { result } = renderHook( () => useThemeStyleVariationsByProperty( { - styleVariations: mockVariations, - styleProperty: 'color', + variations: mockVariations, + property: 'color', } ) ); @@ -623,9 +654,9 @@ describe( 'useThemeStyleVariationsByProperty', () => { it( 'should merge the user styles and settings with the supplied variation, but only for the specified property', () => { const { result } = renderHook( () => useThemeStyleVariationsByProperty( { - styleVariations: [ mockVariations[ 0 ] ], - styleProperty: 'typography', - mergeWith: mockBaseVariation, + variations: [ mockVariations[ 0 ] ], + property: 'typography', + baseVariation: mockBaseVariation, } ) ); @@ -742,8 +773,8 @@ describe( 'useThemeStyleVariationsByProperty', () => { it( 'should filter the output and return only variations that match filter', () => { const { result } = renderHook( () => useThemeStyleVariationsByProperty( { - styleVariations: mockVariations, - styleProperty: 'typography', + variations: mockVariations, + property: 'typography', filter: ( variation ) => !! variation?.settings?.typography?.fontFamilies?.theme ?.length, 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 1d5a257fa7d91d..3c1da551852b2f 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 @@ -38,26 +38,26 @@ export const filterObjectByProperty = ( object, property ) => { /** * Returns a new object with only the properties specified in `properties`. * - * @param {Object} props Object of hook args. - * @param {Object[]} props.styleVariations The theme style variations to filter. - * @param {string} props.styleProperty 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. + * @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( { - styleVariations, - styleProperty, + variations, + property, filter, baseVariation, } ) { return useMemo( () => { - if ( ! styleProperty || styleVariations?.length === 0 ) { - return styleVariations; + if ( ! property || ! variations || variations?.length === 0 ) { + return variations; } - let processedStyleVariations = styleVariations.map( ( variation ) => ( { - ...filterObjectByProperty( variation, styleProperty ), + let processedStyleVariations = variations.map( ( variation ) => ( { + ...filterObjectByProperty( variation, property ), // Add variation title and description to every variation item. title: variation?.title, description: variation?.description, @@ -84,5 +84,5 @@ export default function useThemeStyleVariationsByProperty( { } return processedStyleVariations; - }, [ styleVariations, styleProperty, baseVariation, filter ] ); + }, [ variations, property, baseVariation, filter ] ); } From d80fa65f2dadf6b253fbf7f3751d032054034f2f Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Feb 2024 15:02:12 +1100 Subject: [PATCH 05/10] Adds filterObjectByProperty tests --- .../use-theme-style-variations-by-property.js | 62 ++++++++++++++++++- 1 file changed, 61 insertions(+), 1 deletion(-) 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 52594fffd915c4..d493ab84e91ec6 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 @@ -6,7 +6,67 @@ import { renderHook } from '@testing-library/react'; /** * Internal dependencies */ -import useThemeStyleVariationsByProperty from '../use-theme-style-variations-by-property'; +import useThemeStyleVariationsByProperty, { + filterObjectByProperty, +} from '../use-theme-style-variations-by-property'; + +describe( 'filterObjectByProperty', () => { + test.each( [ + { + object: { + foo: 'bar', + array: [ 1, 3, 4 ], + }, + property: 'array', + expected: { array: [ 1, 3, 4 ] }, + }, + { + object: { + foo: 'bar', + }, + property: 'does-not-exist', + expected: {}, + }, + { + object: { + foo: 'bar', + }, + property: false, + expected: {}, + }, + { + object: [], + property: 'something', + expected: {}, + }, + { + object: {}, + property: undefined, + expected: {}, + }, + { + object: { + 'nested-object': { + foo: 'bar', + array: [ 1, 3, 4 ], + }, + }, + property: 'nested-object', + expected: { + 'nested-object': { + foo: 'bar', + array: [ 1, 3, 4 ], + }, + }, + }, + ] )( + 'should filter object by $property', + ( { expected, object, property } ) => { + const result = filterObjectByProperty( object, property ); + expect( result ).toEqual( expected ); + } + ); +} ); describe( 'useThemeStyleVariationsByProperty', () => { const mockVariations = [ From 5fbca49a4f0717ea152412074cf5f17a4bbf80e4 Mon Sep 17 00:00:00 2001 From: Ramon Date: Tue, 13 Feb 2024 15:02:45 +1100 Subject: [PATCH 06/10] Update packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js Co-authored-by: Andrew Serong <14988353+andrewserong@users.noreply.github.com> --- .../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 3c1da551852b2f..0556605bcc51aa 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 @@ -36,7 +36,7 @@ export const filterObjectByProperty = ( object, property ) => { }; /** - * Returns a new object with only the properties specified in `properties`. + * Returns a new object with only the properties specified in `property`. * * @param {Object} props Object of hook args. * @param {Object[]} props.variations The theme style variations to filter. From ee36d45b6e3dd1b1bea732e66606e7e484f85cc2 Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Feb 2024 15:13:40 +1100 Subject: [PATCH 07/10] Merge map callbacks --- .../use-theme-style-variations-by-property.js | 39 ++++++++++--------- 1 file changed, 20 insertions(+), 19 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 0556605bcc51aa..0495042d38eec3 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 @@ -56,27 +56,28 @@ export default function useThemeStyleVariationsByProperty( { return variations; } - let processedStyleVariations = variations.map( ( variation ) => ( { - ...filterObjectByProperty( variation, property ), - // Add variation title and description to every variation item. - title: variation?.title, - description: variation?.description, - } ) ); - - if ( + const clonedBaseVariation = typeof baseVariation === 'object' && Object.keys( baseVariation ).length > 0 - ) { - /* - * Overwrites all baseVariation object `styleProperty` properties - * with the theme variation `styleProperty` properties. - */ - const clonedBaseVariation = cloneDeep( baseVariation ); - processedStyleVariations = processedStyleVariations.map( - ( variation ) => - mergeBaseAndUserConfigs( clonedBaseVariation, variation ) - ); - } + ? cloneDeep( baseVariation ) + : null; + + let processedStyleVariations = variations.map( ( variation ) => { + let result = { + ...filterObjectByProperty( variation, property ), + title: variation?.title, + description: variation?.description, + }; + + if ( clonedBaseVariation ) { + /* + * Overwrites all baseVariation object `styleProperty` properties + * with the theme variation `styleProperty` properties. + */ + result = mergeBaseAndUserConfigs( clonedBaseVariation, result ); + } + return result; + } ); if ( 'function' === typeof filter ) { processedStyleVariations = From 1ef39b88fef351e22e8d6fa55eb999a600809eb8 Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Feb 2024 15:23:59 +1100 Subject: [PATCH 08/10] More tests, adding a `null` check too --- .../use-theme-style-variations-by-property.js | 34 ++++++++++++++++--- 1 file changed, 30 insertions(+), 4 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 d493ab84e91ec6..a49050e37515e7 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 @@ -11,6 +11,7 @@ import useThemeStyleVariationsByProperty, { } from '../use-theme-style-variations-by-property'; describe( 'filterObjectByProperty', () => { + const noop = () => {}; test.each( [ { object: { @@ -34,6 +35,32 @@ describe( 'filterObjectByProperty', () => { property: false, expected: {}, }, + { + object: { + dig: { + deeper: { + null: null, + }, + }, + }, + property: 'null', + expected: { + dig: { + deeper: { + null: null, + }, + }, + }, + }, + { + object: { + function: noop, + }, + property: 'function', + expected: { + function: noop, + }, + }, { object: [], property: 'something', @@ -47,15 +74,14 @@ describe( 'filterObjectByProperty', () => { { object: { 'nested-object': { - foo: 'bar', + 'nested-object-foo': 'bar', array: [ 1, 3, 4 ], }, }, - property: 'nested-object', + property: 'nested-object-foo', expected: { 'nested-object': { - foo: 'bar', - array: [ 1, 3, 4 ], + 'nested-object-foo': 'bar', }, }, }, From 3fa7504fd4d4f062e7756abbaee8638b791338ea Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Feb 2024 15:47:51 +1100 Subject: [PATCH 09/10] Update comment to reflect functionality --- .../use-theme-style-variations-by-property.js | 5 ++++- 1 file changed, 4 insertions(+), 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 0495042d38eec3..13f5e64b47a4bd 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 @@ -10,7 +10,10 @@ import { mergeBaseAndUserConfigs } from '../../components/global-styles/global-s import cloneDeep from '../../utils/clone-deep'; /** - * Returns a new object with only the properties specified in `properties`. + * Returns a new object, with properties specified in `property`, + * maintain the original object tree structure. + * The function is recursive, so it will perform a deep search for the given property. + * E.g., the function will return `{ a: { b: { c: { test: 1 } } } }` if the property is `test`. * * @param {Object} object The object to filter * @param {Object} property The property to filter by From 43c2cd3e9881ea76b1427698c5bade739c338a0c Mon Sep 17 00:00:00 2001 From: ramon Date: Tue, 13 Feb 2024 16:06:12 +1100 Subject: [PATCH 10/10] Update test to ensure we don't reference original object --- .../use-theme-style-variations-by-property.js | 37 +++++++++++++++++++ .../use-theme-style-variations-by-property.js | 2 +- 2 files changed, 38 insertions(+), 1 deletion(-) 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 a49050e37515e7..ced849f8fb1213 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 @@ -455,6 +455,43 @@ describe( 'useThemeStyleVariationsByProperty', () => { 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( { 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 13f5e64b47a4bd..b9c1b40ec7c1d3 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 @@ -67,7 +67,7 @@ export default function useThemeStyleVariationsByProperty( { let processedStyleVariations = variations.map( ( variation ) => { let result = { - ...filterObjectByProperty( variation, property ), + ...filterObjectByProperty( cloneDeep( variation ), property ), title: variation?.title, description: variation?.description, };