Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Global Styles: Simplify code to fetch color and typography variation #62827

Merged
merged 11 commits into from
Jul 5, 2024
Merged

Global Styles: Simplify code to fetch color and typography variation #62827

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
1d8c7c4
Simplify style variation code
scruffian Jun 25, 2024
642616e
Update packages/edit-site/src/components/global-styles/variations/var…
scruffian Jun 25, 2024
b1304b4
Update packages/edit-site/src/hooks/use-theme-style-variations/use-th…
scruffian Jul 2, 2024
392ca03
Update the function to actually check if the variation is empty
scruffian Jul 2, 2024
1cf7b08
move the filter inside the useMemo
scruffian Jul 2, 2024
8872d0f
Update packages/edit-site/src/hooks/use-theme-style-variations/use-th…
scruffian Jul 2, 2024
a33eb22
Update packages/edit-site/src/hooks/use-theme-style-variations/use-th…
scruffian Jul 2, 2024
16cc9ea
updawte variable name
scruffian Jul 4, 2024
e5dafe6
fix rebase issues
scruffian Jul 4, 2024
002915c
refactor dependecy to a const
scruffian Jul 4, 2024
90d3084
use the new function in the sidebar
scruffian Jul 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
41 changes: 0 additions & 41 deletions packages/edit-site/src/components/global-styles/hooks.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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
);
} )
: [];
}
8 changes: 5 additions & 3 deletions packages/edit-site/src/components/global-styles/variations/variations-color.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {
Expand All @@ -31,7 +33,7 @@ export default function ColorVariations( { title, gap = 2 } ) {
key={ index }
variation={ variation }
isPill
properties={ [ 'color' ] }
properties={ propertiesToFilter }
showTooltip
>
{ () => <StylesPreviewColors /> }
Expand Down
10 changes: 7 additions & 3 deletions packages/edit-site/src/components/global-styles/variations/variations-typography.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -34,7 +38,7 @@ export default function TypographyVariations( { title, gap = 2 } ) {
<Variation
key={ index }
variation={ variation }
properties={ [ 'typography' ] }
properties={ propertiesToFilter }
showTooltip
>
{ () => (
Expand Down
13 changes: 7 additions & 6 deletions packages/edit-site/src/components/sidebar-navigation-screen-global-styles/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 = () => {};

Expand Down Expand Up @@ -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
Expand Down
39 changes: 32 additions & 7 deletions .../edit-site/src/hooks/use-theme-style-variations/use-theme-style-variations-by-property.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Comment on lines +64 to +65
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not familiar with how theme.json settings get applied. From looking at the structure, I'm wondering if this should be an AND:

( Object.keys( settings ).length > 0 && Object.keys( styles ).length > 0 )

It looks like it could be a valid theme style while having a setting or style? Or does it need both?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah the problem here is actually the name of the function. It is returning true if the variation is NOT empty!

);
}

/**
* 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(
Expand All @@ -72,6 +88,8 @@ export function useCurrentMergeThemeStyleVariationsWithUserConfig( {
}, [] );
const { user: userVariation } = useContext( GlobalStylesContext );

const propertiesAsString = properties.toString();

return useMemo( () => {
const clonedUserVariation = cloneDeep( userVariation );

Expand All @@ -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 ] );
}

/**
Expand Down
Loading