From 0861b0d089d6ce65cdad1a7e2ddbed7c588e6dca Mon Sep 17 00:00:00 2001 From: Ella Date: Tue, 5 Dec 2023 01:22:17 +0200 Subject: [PATCH 1/5] Components: ToolsPanel: fix deregister/register on type --- packages/components/src/tools-panel/tools-panel-item/hook.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 244349b6379ea..8a248f085ccbc 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts @@ -52,7 +52,8 @@ export function useToolsPanelItem( __experimentalLastVisibleItemClass, } = useToolsPanelContext(); - const hasValueCallback = useCallback( hasValue, [ panelId, hasValue ] ); + // eslint-disable-next-line react-hooks/exhaustive-deps + const hasValueCallback = useCallback( hasValue, [ panelId ] ); const resetAllFilterCallback = useCallback( resetAllFilter, [ panelId, resetAllFilter, From 17a7dcb28ac9c3e9a83f2830b0dfa64acd94206c Mon Sep 17 00:00:00 2001 From: Ella Date: Tue, 5 Dec 2023 01:33:56 +0200 Subject: [PATCH 2/5] Fix for registerResetAllFilter --- .../components/src/tools-panel/tools-panel-item/hook.ts | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 8a248f085ccbc..229b9256488bf 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts @@ -54,10 +54,8 @@ export function useToolsPanelItem( // eslint-disable-next-line react-hooks/exhaustive-deps const hasValueCallback = useCallback( hasValue, [ panelId ] ); - const resetAllFilterCallback = useCallback( resetAllFilter, [ - panelId, - resetAllFilter, - ] ); + // eslint-disable-next-line react-hooks/exhaustive-deps + const resetAllFilterCallback = useCallback( resetAllFilter, [ panelId ] ); const previousPanelId = usePrevious( currentPanelId ); const hasMatchingPanel = From 086accc523ec2901ce235ee8c3b57aebf025b3fc Mon Sep 17 00:00:00 2001 From: Ella Date: Tue, 5 Dec 2023 11:26:23 +0200 Subject: [PATCH 3/5] Fix unit test + add comment --- .../src/tools-panel/tools-panel-item/hook.ts | 17 ++++++----------- 1 file changed, 6 insertions(+), 11 deletions(-) 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 229b9256488bf..d64aecbed28d3 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts @@ -52,8 +52,12 @@ export function useToolsPanelItem( __experimentalLastVisibleItemClass, } = useToolsPanelContext(); + // hasValue is a new function on every render, so do not add it as a + // dependency to the useCallback hook! If needed, we should use a ref. // eslint-disable-next-line react-hooks/exhaustive-deps const hasValueCallback = useCallback( hasValue, [ panelId ] ); + // resetAllFilter is a new function on every render, so do not add it as a + // dependency to the useCallback hook! If needed, we should use a ref. // eslint-disable-next-line react-hooks/exhaustive-deps const resetAllFilterCallback = useCallback( resetAllFilter, [ panelId ] ); const previousPanelId = usePrevious( currentPanelId ); @@ -135,17 +139,8 @@ export function useToolsPanelItem( return; } - if ( isShownByDefault || currentPanelId === null ) { - flagItemCustomization( label, menuGroup ); - } - }, [ - currentPanelId, - newValueSet, - isShownByDefault, - menuGroup, - label, - flagItemCustomization, - ] ); + flagItemCustomization( label, menuGroup ); + }, [ newValueSet, menuGroup, label, flagItemCustomization ] ); // Determine if the panel item's corresponding menu is being toggled and // trigger appropriate callback if it is. From a010c662492d25fe058e1469385deb40e536a7eb Mon Sep 17 00:00:00 2001 From: Ella Date: Tue, 5 Dec 2023 11:44:06 +0200 Subject: [PATCH 4/5] Changelog --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index f3019c250a6c4..435a9bf22d9b2 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -11,6 +11,7 @@ ### Bug Fix - `ToggleGroupControl`: react correctly to external controlled updates ([#56678](https://github.com/WordPress/gutenberg/pull/56678)). +- `ToolsPanel`: fix a performance issue ([#56770](https://github.com/WordPress/gutenberg/pull/56770)). ## 25.13.0 (2023-11-29) From c601a63189d4326940710335aac9928ca500a1f4 Mon Sep 17 00:00:00 2001 From: Ella Date: Wed, 6 Dec 2023 08:29:55 +0200 Subject: [PATCH 5/5] Adjust comment --- packages/components/src/tools-panel/tools-panel-item/hook.ts | 5 ----- 1 file changed, 5 deletions(-) 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 d64aecbed28d3..fe415b8723a88 100644 --- a/packages/components/src/tools-panel/tools-panel-item/hook.ts +++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts @@ -129,11 +129,6 @@ export function useToolsPanelItem( const newValueSet = isValueSet && ! wasValueSet; // Notify the panel when an item's value has been set. - // - // 1. For default controls, this is so "reset" appears beside its menu item. - // 2. For optional controls, when the panel ID is `null`, it allows the - // panel to ensure the item is toggled on for display in the menu, given the - // value has been set external to the control. useEffect( () => { if ( ! newValueSet ) { return;