From eaf2258cdeed35a4d3c690d66892981a85c4d9ed Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 20 Sep 2024 15:19:34 +0100 Subject: [PATCH] =?UTF-8?q?Revert=20"useToolsPanel:=20calculate=20menuItem?= =?UTF-8?q?s=20in=20layout=20effect=20to=20avoid=20painting=E2=80=A6"=20(#?= =?UTF-8?q?65533)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 5e37a1316d0a39bc880bf6efff9b9f26db381690. --- packages/components/CHANGELOG.md | 4 - .../src/tools-panel/tools-panel-item/hook.ts | 13 +- .../src/tools-panel/tools-panel/hook.ts | 131 ++++++++++-------- 3 files changed, 80 insertions(+), 68 deletions(-) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 03d6eb30e336b0..6607700b2d2c25 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -2,10 +2,6 @@ ## Unreleased -### Bug Fixes - -- `ToolsPanel`: avoid paining intermediate unfinished states ([#65494](https://github.com/WordPress/gutenberg/pull/65494)). - ## 28.8.0 (2024-09-19) ### Bug Fixes diff --git a/packages/components/src/tools-panel/tools-panel-item/hook.ts b/packages/components/src/tools-panel/tools-panel-item/hook.ts index 27a0ceb27e7ce7..1e33e7c6740ded 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts @@ -2,7 +2,12 @@ * WordPress dependencies */ import { usePrevious } from '@wordpress/compose'; -import { useCallback, useLayoutEffect, useMemo } from '@wordpress/element'; +import { + useCallback, + useEffect, + useLayoutEffect, + useMemo, +} from '@wordpress/element'; /** * Internal dependencies @@ -96,7 +101,7 @@ export function useToolsPanelItem( deregisterPanelItem, ] ); - useLayoutEffect( () => { + useEffect( () => { if ( hasMatchingPanel ) { registerResetAllFilter( resetAllFilterCallback ); } @@ -122,7 +127,7 @@ export function useToolsPanelItem( const isValueSet = hasValue(); // Notify the panel when an item's value has changed except for optional // items without value because the item should not cause itself to hide. - useLayoutEffect( () => { + useEffect( () => { if ( ! isShownByDefault && ! isValueSet ) { return; } @@ -138,7 +143,7 @@ export function useToolsPanelItem( // Determine if the panel item's corresponding menu is being toggled and // trigger appropriate callback if it is. - useLayoutEffect( () => { + useEffect( () => { // We check whether this item is currently registered as items rendered // via fills can persist through the parent panel being remounted. // See: https://github.com/WordPress/gutenberg/pull/45673 diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts index d67d732d4df671..931bf2494e6e34 100644 --- a/packages/components/src/tools-panel/tools-panel/hook.ts +++ b/packages/components/src/tools-panel/tools-panel/hook.ts @@ -3,7 +3,7 @@ */ import { useCallback, - useLayoutEffect, + useEffect, useMemo, useRef, useState, @@ -101,7 +101,7 @@ export function useToolsPanel( // the resetAll task. Without this, the flag is cleared after the first // control updates and forces a rerender with subsequent controls then // believing they need to reset, unfortunately using stale data. - useLayoutEffect( () => { + useEffect( () => { if ( wasResetting ) { isResettingRef.current = false; } @@ -114,66 +114,76 @@ export function useToolsPanel( ResetAllFilter[] >( [] ); - const registerPanelItem = useCallback( ( item: ToolsPanelItem ) => { - // Add item to panel items. - setPanelItems( ( items ) => { - const newItems = [ ...items ]; - // If an item with this label has already been registered, remove it - // first. This can happen when an item is moved between the default - // and optional groups. - const existingIndex = newItems.findIndex( - ( oldItem ) => oldItem.label === item.label - ); - if ( existingIndex !== -1 ) { - newItems.splice( existingIndex, 1 ); - } - return [ ...newItems, item ]; - } ); + const registerPanelItem = useCallback( + ( item: ToolsPanelItem ) => { + // Add item to panel items. + setPanelItems( ( items ) => { + const newItems = [ ...items ]; + // If an item with this label has already been registered, remove it + // first. This can happen when an item is moved between the default + // and optional groups. + const existingIndex = newItems.findIndex( + ( oldItem ) => oldItem.label === item.label + ); + if ( existingIndex !== -1 ) { + newItems.splice( existingIndex, 1 ); + } + return [ ...newItems, item ]; + } ); - // Track the initial order of item registration. This is used for - // maintaining menu item order later. - setMenuItemOrder( ( items ) => { - if ( items.includes( item.label ) ) { - return items; - } + // Track the initial order of item registration. This is used for + // maintaining menu item order later. + setMenuItemOrder( ( items ) => { + if ( items.includes( item.label ) ) { + return items; + } - return [ ...items, item.label ]; - } ); - }, [] ); + return [ ...items, item.label ]; + } ); + }, + [ setPanelItems, setMenuItemOrder ] + ); // Panels need to deregister on unmount to avoid orphans in menu state. // This is an issue when panel items are being injected via SlotFills. - const deregisterPanelItem = useCallback( ( label: string ) => { - // When switching selections between components injecting matching - // controls, e.g. both panels have a "padding" control, the - // deregistration of the first panel doesn't occur until after the - // registration of the next. - setPanelItems( ( items ) => { - const newItems = [ ...items ]; - const index = newItems.findIndex( - ( item ) => item.label === label - ); - if ( index !== -1 ) { - newItems.splice( index, 1 ); - } - return newItems; - } ); - }, [] ); + const deregisterPanelItem = useCallback( + ( label: string ) => { + // When switching selections between components injecting matching + // controls, e.g. both panels have a "padding" control, the + // deregistration of the first panel doesn't occur until after the + // registration of the next. + setPanelItems( ( items ) => { + const newItems = [ ...items ]; + const index = newItems.findIndex( + ( item ) => item.label === label + ); + if ( index !== -1 ) { + newItems.splice( index, 1 ); + } + return newItems; + } ); + }, + [ setPanelItems ] + ); const registerResetAllFilter = useCallback( ( newFilter: ResetAllFilter ) => { - setResetAllFilters( ( filters ) => [ ...filters, newFilter ] ); + setResetAllFilters( ( filters ) => { + return [ ...filters, newFilter ]; + } ); }, - [] + [ setResetAllFilters ] ); const deregisterResetAllFilter = useCallback( ( filterToRemove: ResetAllFilter ) => { - setResetAllFilters( ( filters ) => - filters.filter( ( filter ) => filter !== filterToRemove ) - ); + setResetAllFilters( ( filters ) => { + return filters.filter( + ( filter ) => filter !== filterToRemove + ); + } ); }, - [] + [ setResetAllFilters ] ); // Manage and share display state of menu items representing child controls. @@ -183,16 +193,17 @@ export function useToolsPanel( } ); // Setup menuItems state as panel items register themselves. - useLayoutEffect( () => { - setMenuItems( ( currentMenuItems ) => - generateMenuItems( { + useEffect( () => { + setMenuItems( ( prevState ) => { + const items = generateMenuItems( { panelItems, shouldReset: false, - currentMenuItems, + currentMenuItems: prevState, menuItemOrder, - } ) - ); - }, [ panelItems, menuItemOrder ] ); + } ); + return items; + } ); + }, [ panelItems, setMenuItems, menuItemOrder ] ); // Updates the status of the panel’s menu items. For default items the // value represents whether it differs from the default and for optional @@ -214,7 +225,7 @@ export function useToolsPanel( return newState; } ); }, - [] + [ setMenuItems ] ); // Whether all optional menu items are hidden or not must be tracked @@ -224,7 +235,7 @@ export function useToolsPanel( const [ areAllOptionalControlsHidden, setAreAllOptionalControlsHidden ] = useState( false ); - useLayoutEffect( () => { + useEffect( () => { if ( isMenuItemTypeEmpty( menuItems?.default ) && ! isMenuItemTypeEmpty( menuItems?.optional ) @@ -234,7 +245,7 @@ export function useToolsPanel( ).some( ( [ , isSelected ] ) => isSelected ); setAreAllOptionalControlsHidden( allControlsHidden ); } - }, [ menuItems ] ); + }, [ menuItems, setAreAllOptionalControlsHidden ] ); const cx = useCx(); const classes = useMemo( () => { @@ -286,7 +297,7 @@ export function useToolsPanel( setMenuItems( newMenuItems ); }, - [ menuItems, panelItems ] + [ menuItems, panelItems, setMenuItems ] ); // Resets display of children and executes resetAll callback if available. @@ -303,7 +314,7 @@ export function useToolsPanel( shouldReset: true, } ); setMenuItems( resetMenuItems ); - }, [ panelItems, resetAllFilters, resetAll, menuItemOrder ] ); + }, [ panelItems, resetAllFilters, resetAll, setMenuItems, menuItemOrder ] ); // Assist ItemGroup styling when there are potentially hidden placeholder // items by identifying first & last items that are toggled on for display.