From 3f8062c5371b5d8f56e0896d19d2480d2caca9c6 Mon Sep 17 00:00:00 2001 From: Aaron Robertshaw <60436221+aaronrobertshaw@users.noreply.github.com> Date: Mon, 5 Feb 2024 12:40:24 +1000 Subject: [PATCH] Try and minimize style override order changes --- packages/block-editor/src/hooks/utils.js | 47 ++++++- packages/block-editor/src/hooks/variation.js | 130 +++++++++++++------ 2 files changed, 132 insertions(+), 45 deletions(-) diff --git a/packages/block-editor/src/hooks/utils.js b/packages/block-editor/src/hooks/utils.js index 2f7a8f3a81f19..afb62dd01b761 100644 --- a/packages/block-editor/src/hooks/utils.js +++ b/packages/block-editor/src/hooks/utils.js @@ -3,7 +3,7 @@ */ import { getBlockSupport } from '@wordpress/blocks'; import { memo, useMemo, useEffect, useId, useState } from '@wordpress/element'; -import { useDispatch } from '@wordpress/data'; +import { useDispatch, useSelect } from '@wordpress/data'; import { createHigherOrderComponent } from '@wordpress/compose'; import { addFilter } from '@wordpress/hooks'; @@ -133,21 +133,58 @@ export function shouldSkipSerialization( return skipSerialization; } -export function useStyleOverride( { id, css, assets, __unstableType } = {} ) { +export function useStyleOverride( { + id, + css, + assets, + __unstableType, + childOverrideIds, +} = {} ) { const { setStyleOverride, deleteStyleOverride } = unlock( useDispatch( blockEditorStore ) ); + + const getStyleOverrides = useSelect( + ( select ) => unlock( select( blockEditorStore ) ).getStyleOverrides, + [] + ); + const fallbackId = useId(); useEffect( () => { // Unmount if there is CSS and assets are empty. - if ( ! css && ! assets ) return; + if ( ! css && ! assets && __unstableType !== 'variation' ) return; + const _id = id || fallbackId; + setStyleOverride( _id, { id, css, assets, __unstableType, } ); + + // To maintain style order for block style variations such that parent + // block style overrides are loaded before children. Remove the child + // overrides specified in `childOverrideIds` and re-add them so they are + // stored in the underlying Map after the parent in the insertion order. + // + // If this hook only prevented the cleanup of a variation's style + // overrides, when a new application of a block style variation was made + // to a block containing inner blocks with their own variations applied, + // the parent's would be loaded after which isn't desired. + if ( __unstableType === 'variation' && childOverrideIds?.length ) { + const overrides = getStyleOverrides(); + + childOverrideIds.forEach( ( childOverrideId ) => { + const override = overrides.get( childOverrideId ); + + if ( override ) { + deleteStyleOverride( childOverrideId ); + setStyleOverride( childOverrideId, override ); + } + } ); + } + return () => { deleteStyleOverride( _id ); }; @@ -157,8 +194,10 @@ export function useStyleOverride( { id, css, assets, __unstableType } = {} ) { assets, __unstableType, fallbackId, + getStyleOverrides, setStyleOverride, deleteStyleOverride, + childOverrideIds, ] ); } @@ -500,7 +539,7 @@ export function createBlockListBlockFilter( features ) { useBlockProps, } = feature; - const neededProps = {}; + const neededProps = { clientId: props.clientId }; for ( const key of attributeKeys ) { if ( props.attributes[ key ] ) { neededProps[ key ] = props.attributes[ key ]; diff --git a/packages/block-editor/src/hooks/variation.js b/packages/block-editor/src/hooks/variation.js index 6383456023449..8ea7fecf79c28 100644 --- a/packages/block-editor/src/hooks/variation.js +++ b/packages/block-editor/src/hooks/variation.js @@ -2,7 +2,6 @@ * WordPress dependencies */ import { getBlockTypes, store as blocksStore } from '@wordpress/blocks'; -import { useInstanceId } from '@wordpress/compose'; import { useSelect } from '@wordpress/data'; import { useContext, useMemo } from '@wordpress/element'; @@ -20,60 +19,104 @@ import { store as blockEditorStore } from '../store'; export default { hasSupport: () => true, // TODO: Work out what the eligibility here should be. attributeKeys: [ 'style' ], + passChildren: true, useBlockProps, }; -function useBlockSyleVariation( name, variation, id ) { +function hasVariationClass( className ) { + return /\bis-style-(?!default)\b/.test( className ); +} + +function findInnerVariations( innerBlocks ) { + const variations = []; + + innerBlocks.forEach( ( innerBlock ) => { + if ( ! innerBlock ) { + return; + } + + const { innerBlocks: nestedBlocks, clientId } = innerBlock; + + if ( hasVariationClass( innerBlock.attributes?.className ) ) { + variations.push( `variation-${ clientId }` ); + } + + // Recursively check children of current child for variations. + if ( nestedBlocks ) { + variations.push( ...findInnerVariations( nestedBlocks ) ); + } + } ); + + return variations; +} + +function useBlockSyleVariation( name, variation, clientId ) { const { user: userStyles } = useContext( GlobalStylesContext ); - const globalStyles = useSelect( ( select ) => { - const { getSettings } = select( blockEditorStore ); - return { - settings: getSettings().__experimentalFeatures, - styles: getSettings().__experimentalStyles, - }; - }, [] ); + const { globalSettings, globalStyles, block } = useSelect( + ( select ) => { + const { getSettings, getBlock } = select( blockEditorStore ); + return { + globalSettings: getSettings().__experimentalFeatures, + globalStyles: getSettings().__experimentalStyles, + block: getBlock( clientId ), + }; + }, + [ clientId ] + ); + + return useMemo( () => { + const styles = userStyles?.styles ?? globalStyles; + const variationStyles = + styles?.blocks?.[ name ]?.variations?.[ variation ]; - if ( ! variation ) { - return {}; - } - - const settings = userStyles?.settings ?? globalStyles?.settings; - const styles = userStyles?.styles ?? globalStyles?.styles; - - // The variation style data is all that is needed to generate - // the styles for the current application to a block. The variation - // name is updated to match the instance specific class name. - const variationStyles = styles?.blocks?.[ name ]?.variations?.[ variation ]; - const variationOnlyStyles = { - blocks: { - [ name ]: { - variations: { - [ `${ variation }-${ id }` ]: variationStyles, + return { + settings: userStyles?.settings ?? globalSettings, + // The variation style data is all that is needed to generate + // the styles for the current application to a block. The variation + // name is updated to match the instance specific class name. + styles: { + blocks: { + [ name ]: { + variations: { + [ `${ variation }-${ clientId }` ]: variationStyles, + }, + }, }, }, - }, - }; - - return { - settings, - styles: variationOnlyStyles, - }; + // Collect any inner blocks that have variations applied as their style + // overrides need to be moved to after this block so the CSS cascade is + // in the correct order. + childVariationIds: block?.innerBlocks?.length + ? findInnerVariations( block.innerBlocks ) + : [], + }; + }, [ + userStyles, + globalSettings, + globalStyles, + block, + variation, + clientId, + name, + ] ); } -function useBlockProps( { name, style } ) { +// Rather than leveraging `useInstanceId` here, the `clientId` is used. +// This is so that the variation style override's ID is predictable when +// searching for inner blocks that have a variation applied and need those +// styles to come after the parent's. +function useBlockProps( { name, style, clientId } ) { const variation = style?.variation; - const id = useInstanceId( useBlockProps ); - const className = `is-style-${ variation }-${ id }`; + const className = `is-style-${ variation }-${ clientId }`; const getBlockStyles = useSelect( ( select ) => { return select( blocksStore ).getBlockStyles; }, [] ); - const { settings, styles } = useBlockSyleVariation( + const { settings, styles, childVariationIds } = useBlockSyleVariation( name, variation, - id, - getBlockStyles() + clientId ); const variationStyles = useMemo( () => { @@ -85,7 +128,7 @@ function useBlockProps( { name, style } ) { const blockSelectors = getBlockSelectors( getBlockTypes(), getBlockStyles, - id + clientId ); const hasBlockGapSupport = false; const hasFallbackGapSupport = true; @@ -108,9 +151,14 @@ function useBlockProps( { name, style } ) { rootPadding: false, } ); - }, [ variation, settings, styles, getBlockStyles, id ] ); + }, [ variation, settings, styles, getBlockStyles, clientId ] ); - useStyleOverride( { css: variationStyles } ); + useStyleOverride( { + id: `variation-${ clientId }`, + css: variationStyles, + __unstableType: 'variation', + childOverrideIds: childVariationIds, + } ); return variation ? { className } : {}; }