From df3451b97e31d8e25f2cd44aab650e0bb2d6f267 Mon Sep 17 00:00:00 2001
From: Mitchell Austin <mr.fye@oneandthesame.net>
Date: Wed, 17 Apr 2024 14:48:40 -0700
Subject: [PATCH] Trigger `onDeselect`/`onSelect` callbacks directly

Instead of via an effect hook.
---
 .../src/tools-panel/tools-panel-item/hook.ts  | 38 +++----------------
 .../src/tools-panel/tools-panel/hook.ts       | 19 ++++++++--
 packages/components/src/tools-panel/types.ts  | 14 +++----
 3 files changed, 28 insertions(+), 43 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 1e33e7c6740ded..7949ffa22ad2c1 100644
--- a/packages/components/src/tools-panel/tools-panel-item/hook.ts
+++ b/packages/components/src/tools-panel/tools-panel-item/hook.ts
@@ -44,7 +44,6 @@ export function useToolsPanelItem(
 		registerPanelItem,
 		deregisterPanelItem,
 		flagItemCustomization,
-		isResetting,
 		shouldRenderPlaceholderItems: shouldRenderPlaceholder,
 		firstDisplayedItem,
 		lastDisplayedItem,
@@ -78,6 +77,8 @@ export function useToolsPanelItem(
 				isShownByDefault,
 				label,
 				panelId,
+				onDeselect,
+				onSelect,
 			} );
 		}
 
@@ -99,6 +100,8 @@ export function useToolsPanelItem(
 		previousPanelId,
 		registerPanelItem,
 		deregisterPanelItem,
+		onDeselect,
+		onSelect,
 	] );
 
 	useEffect( () => {
@@ -121,7 +124,6 @@ export function useToolsPanelItem(
 	// `ToolsPanel`.
 	const menuGroup = isShownByDefault ? 'default' : 'optional';
 	const isMenuItemChecked = menuItems?.[ menuGroup ]?.[ label ];
-	const wasMenuItemChecked = usePrevious( isMenuItemChecked );
 	const isRegistered = menuItems?.[ menuGroup ]?.[ label ] !== undefined;
 
 	const isValueSet = hasValue();
@@ -141,40 +143,10 @@ export function useToolsPanelItem(
 		isShownByDefault,
 	] );
 
-	// Determine if the panel item's corresponding menu is being toggled and
-	// trigger appropriate callback if it is.
-	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
-		if ( ! isRegistered || isResetting || ! hasMatchingPanel ) {
-			return;
-		}
-
-		if ( isMenuItemChecked && ! isValueSet && ! wasMenuItemChecked ) {
-			onSelect?.();
-		}
-
-		if ( ! isMenuItemChecked && isValueSet && wasMenuItemChecked ) {
-			onDeselect?.();
-		}
-	}, [
-		hasMatchingPanel,
-		isMenuItemChecked,
-		isRegistered,
-		isResetting,
-		isValueSet,
-		wasMenuItemChecked,
-		onSelect,
-		onDeselect,
-	] );
-
 	// The item is shown if it is a default control regardless of whether it
 	// has a value. Optional items are shown when they are checked or have
 	// a value.
-	const isShown = isShownByDefault
-		? menuItems?.[ menuGroup ]?.[ label ] !== undefined
-		: isMenuItemChecked;
+	const isShown = isShownByDefault ? isRegistered : isMenuItemChecked;
 
 	const cx = useCx();
 	const classes = useMemo( () => {
diff --git a/packages/components/src/tools-panel/tools-panel/hook.ts b/packages/components/src/tools-panel/tools-panel/hook.ts
index 583a079ab20026..03f7758e4ee3d5 100644
--- a/packages/components/src/tools-panel/tools-panel/hook.ts
+++ b/packages/components/src/tools-panel/tools-panel/hook.ts
@@ -357,9 +357,22 @@ export function useToolsPanel(
 
 	// Toggle the checked state of a menu item which is then used to determine
 	// display of the item within the panel.
-	const toggleItem = useCallback( ( label: string ) => {
-		panelDispatch( { type: 'TOGGLE_VALUE', label } );
-	}, [] );
+	const toggleItem = useCallback(
+		( label: string ) => {
+			panelDispatch( { type: 'TOGGLE_VALUE', label } );
+			const currentItem = panelItems.find(
+				( item ) => item.label === label
+			);
+			if ( currentItem ) {
+				const { isShownByDefault, onDeselect, onSelect } = currentItem;
+				const menuGroup = isShownByDefault ? 'default' : 'optional';
+				const hasValue = menuItems[ menuGroup ][ label ];
+				const callback = hasValue ? onDeselect : onSelect;
+				callback?.();
+			}
+		},
+		[ menuItems, panelItems ]
+	);
 
 	// Resets display of children and executes resetAll callback if available.
 	const resetAllItems = useCallback( () => {
diff --git a/packages/components/src/tools-panel/types.ts b/packages/components/src/tools-panel/types.ts
index e8e2f950de9a30..a5308f7dc05ff7 100644
--- a/packages/components/src/tools-panel/types.ts
+++ b/packages/components/src/tools-panel/types.ts
@@ -133,13 +133,6 @@ export type ToolsPanelItem = {
 	 * from a shared source.
 	 */
 	panelId?: string | null;
-};
-
-export type ToolsPanelItemProps = ToolsPanelItem & {
-	/**
-	 * The child elements.
-	 */
-	children?: ReactNode;
 	/**
 	 * Called when this item is deselected in the `ToolsPanel` menu. This is
 	 * normally used to reset the panel item control's value.
@@ -150,6 +143,13 @@ export type ToolsPanelItemProps = ToolsPanelItem & {
 	 * menu.
 	 */
 	onSelect?: () => void;
+};
+
+export type ToolsPanelItemProps = ToolsPanelItem & {
+	/**
+	 * The child elements.
+	 */
+	children?: ReactNode;
 
 	/**
 	 * A `ToolsPanel` will collect each item's `resetAllFilter` and pass an