From 9a53c4733f8607e5c8317f06514b4f884ea8fd6b Mon Sep 17 00:00:00 2001 From: Ben Dwyer Date: Fri, 5 Jul 2024 09:08:17 +0100 Subject: [PATCH] Global Styles: Simplify code to fetch color and typography variation (#62827) * Simplify style variation code * Update packages/edit-site/src/components/global-styles/variations/variations-typography.js * Update packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js Co-authored-by: Jerry Jones * Update the function to actually check if the variation is empty * move the filter inside the useMemo * Update packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js Co-authored-by: Jerry Jones * Update packages/edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js Co-authored-by: Jerry Jones * updawte variable name * fix rebase issues * refactor dependecy to a const * use the new function in the sidebar --------- Co-authored-by: Jerry Jones Co-authored-by: scruffian Co-authored-by: jeryj Co-authored-by: ramonjd --- .../src/components/global-styles/hooks.js | 41 ------------------- .../variations/variations-color.js | 8 ++-- .../variations/variations-typography.js | 10 +++-- .../index.js | 13 +++--- .../use-theme-style-variations-by-property.js | 39 ++++++++++++++---- 5 files changed, 51 insertions(+), 60 deletions(-) diff --git a/packages/edit-site/src/components/global-styles/hooks.js b/packages/edit-site/src/components/global-styles/hooks.js index 5f1c63b600c515..ccae419be98c00 100644 --- a/packages/edit-site/src/components/global-styles/hooks.js +++ b/packages/edit-site/src/components/global-styles/hooks.js @@ -9,12 +9,10 @@ import a11yPlugin from 'colord/plugins/a11y'; */ import { store as blocksStore } from '@wordpress/blocks'; import { privateApis as blockEditorPrivateApis } from '@wordpress/block-editor'; -import { __ } from '@wordpress/i18n'; /** * Internal dependencies */ -import { useCurrentMergeThemeStyleVariationsWithUserConfig } from '../../hooks/use-theme-style-variations/use-theme-style-variations-by-property'; import { unlock } from '../../lock-unlock'; import { useSelect } from '@wordpress/data'; @@ -111,42 +109,3 @@ export function useSupportedStyles( name, element ) { return supportedPanels; } - -export function useColorVariations() { - const colorVariations = useCurrentMergeThemeStyleVariationsWithUserConfig( { - properties: [ 'color' ], - } ); - /* - * Filter out variations with no settings or styles. - */ - return colorVariations?.length - ? colorVariations.filter( ( variation ) => { - const { settings, styles, title } = variation; - return ( - title === __( 'Default' ) || // Always preseve the default variation. - Object.keys( settings ).length > 0 || - Object.keys( styles ).length > 0 - ); - } ) - : []; -} - -export function useTypographyVariations() { - const typographyVariations = - useCurrentMergeThemeStyleVariationsWithUserConfig( { - properties: [ 'typography' ], - } ); - /* - * Filter out variations with no settings or styles. - */ - return typographyVariations?.length - ? typographyVariations.filter( ( variation ) => { - const { settings, styles, title } = variation; - return ( - title === __( 'Default' ) || // Always preseve the default variation. - Object.keys( settings ).length > 0 || - Object.keys( styles ).length > 0 - ); - } ) - : []; -} diff --git a/packages/edit-site/src/components/global-styles/variations/variations-color.js b/packages/edit-site/src/components/global-styles/variations/variations-color.js index aa680d7083325f..e74d2171c7fa75 100644 --- a/packages/edit-site/src/components/global-styles/variations/variations-color.js +++ b/packages/edit-site/src/components/global-styles/variations/variations-color.js @@ -10,12 +10,14 @@ import { * Internal dependencies */ import StylesPreviewColors from '../preview-colors'; -import { useColorVariations } from '../hooks'; +import { useCurrentMergeThemeStyleVariationsWithUserConfig } from '../../../hooks/use-theme-style-variations/use-theme-style-variations-by-property'; import Subtitle from '../subtitle'; import Variation from './variation'; export default function ColorVariations( { title, gap = 2 } ) { - const colorVariations = useColorVariations(); + const propertiesToFilter = [ 'color' ]; + const colorVariations = + useCurrentMergeThemeStyleVariationsWithUserConfig( propertiesToFilter ); // Return null if there is only one variation (the default). if ( colorVariations?.length <= 1 ) { @@ -31,7 +33,7 @@ export default function ColorVariations( { title, gap = 2 } ) { key={ index } variation={ variation } isPill - properties={ [ 'color' ] } + properties={ propertiesToFilter } showTooltip > { () => } diff --git a/packages/edit-site/src/components/global-styles/variations/variations-typography.js b/packages/edit-site/src/components/global-styles/variations/variations-typography.js index ee00ca51e65760..ac9e16c6262faf 100644 --- a/packages/edit-site/src/components/global-styles/variations/variations-typography.js +++ b/packages/edit-site/src/components/global-styles/variations/variations-typography.js @@ -9,13 +9,17 @@ import { /** * Internal dependencies */ + import StylesPreviewTypography from '../preview-typography'; -import { useTypographyVariations } from '../hooks'; +import { useCurrentMergeThemeStyleVariationsWithUserConfig } from '../../../hooks/use-theme-style-variations/use-theme-style-variations-by-property'; import Variation from './variation'; import Subtitle from '../subtitle'; export default function TypographyVariations( { title, gap = 2 } ) { - const typographyVariations = useTypographyVariations(); + const propertiesToFilter = [ 'typography' ]; + const typographyVariations = + useCurrentMergeThemeStyleVariationsWithUserConfig( propertiesToFilter ); + // Return null if there is only one variation (the default). if ( typographyVariations?.length <= 1 ) { return null; @@ -34,7 +38,7 @@ export default function TypographyVariations( { title, gap = 2 } ) { { () => ( diff --git a/packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js b/packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js index 29ce24cfb3b7b1..8c5692655a0fc3 100644 --- a/packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js +++ b/packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js @@ -26,10 +26,7 @@ import useGlobalStylesRevisions from '../global-styles/screen-revisions/use-glob import SidebarNavigationScreenDetailsFooter from '../sidebar-navigation-screen-details-footer'; import ColorVariations from '../global-styles/variations/variations-color'; import TypographyVariations from '../global-styles/variations/variations-typography'; -import { - useColorVariations, - useTypographyVariations, -} from '../global-styles/hooks'; +import { useCurrentMergeThemeStyleVariationsWithUserConfig } from '../../hooks/use-theme-style-variations/use-theme-style-variations-by-property'; const noop = () => {}; @@ -75,8 +72,12 @@ function SidebarNavigationScreenGlobalStylesContent() { }; }, [] ); - const colorVariations = useColorVariations(); - const typographyVariations = useTypographyVariations(); + const colorVariations = useCurrentMergeThemeStyleVariationsWithUserConfig( [ + 'color', + ] ); + const typographyVariations = + useCurrentMergeThemeStyleVariationsWithUserConfig( [ 'typography' ] ); + const gap = 3; // Wrap in a BlockEditorProvider to ensure that the Iframe's dependencies are 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 d3edfea53a0ded..9486f15f802899 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 @@ -49,17 +49,33 @@ export function removePropertiesFromObject( object, properties ) { return object; } +/** + * Checks whether a style variation is empty. + * + * @param {Object} variation A style variation object. + * @param {string} variation.title The title of the variation. + * @param {Object} variation.settings The settings of the variation. + * @param {Object} variation.styles The styles of the variation. + * @return {boolean} Whether the variation is empty. + */ +function hasThemeVariation( { title, settings, styles } ) { + return ( + title === __( 'Default' ) || // Always preserve the default variation. + Object.keys( settings ).length > 0 || + Object.keys( styles ).length > 0 + ); +} + /** * Fetches the current theme style variations that contain only the specified properties * and merges them with the user config. * - * @param {Object} props Object of hook args. - * @param {string[]} props.properties The properties to filter by. + * @param {string[]} properties The properties to filter by. * @return {Object[]|*} The merged object. */ -export function useCurrentMergeThemeStyleVariationsWithUserConfig( { - properties = [], -} ) { +export function useCurrentMergeThemeStyleVariationsWithUserConfig( + properties = [] +) { const { variationsFromTheme } = useSelect( ( select ) => { const _variationsFromTheme = select( @@ -72,6 +88,8 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { }, [] ); const { user: userVariation } = useContext( GlobalStylesContext ); + const propertiesAsString = properties.toString(); + return useMemo( () => { const clonedUserVariation = cloneDeep( userVariation ); @@ -93,11 +111,18 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( { ); } ); - return [ + const variationsByProperties = [ userVariationWithoutProperties, ...variationsWithPropertiesAndBase, ]; - }, [ properties.toString(), userVariation, variationsFromTheme ] ); + + /* + * Filter out variations with no settings or styles. + */ + return variationsByProperties?.length + ? variationsByProperties.filter( hasThemeVariation ) + : []; + }, [ propertiesAsString, userVariation, variationsFromTheme ] ); } /**