From 23d973fd82743fe828296f8b2b162ac10c8a4723 Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Fri, 8 Sep 2023 10:57:34 +1000 Subject: [PATCH] Try switching to a controlled component --- .../block-settings-dropdown.js | 27 +++++----- .../components/src/dropdown-menu/README.md | 10 +++- .../components/src/dropdown-menu/index.tsx | 9 ++-- .../components/src/dropdown-menu/types.ts | 23 +++++---- packages/components/src/dropdown/README.md | 6 +++ packages/components/src/dropdown/index.tsx | 51 ++++++------------- packages/components/src/dropdown/types.ts | 4 ++ 7 files changed, 66 insertions(+), 64 deletions(-) diff --git a/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js b/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js index 1bd6ce620b1ac..01d78c49814c8 100644 --- a/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js +++ b/packages/block-editor/src/components/block-settings-menu/block-settings-dropdown.js @@ -191,26 +191,25 @@ export function BlockSettingsDropdown( { const parentBlockIsSelected = selectedBlockClientIds?.includes( firstParentClientId ); - // Only override the `open` prop if the current block is not the one that - // opened the menu. The logic here is to ensure that non-current - // block menus are automatically closed when a new block menu is opened. - // This is useful for cases where focus is not present in the current window. - // All other behavior of the drop down menu should be otherwise unaffected. - const open = - ! currentClientId || openedBlockSettingsMenu === currentClientId - ? undefined - : false; + // When a currentClientId is in use, treat the menu as a controlled component. + // This ensures that only one block settings menu is open at a time. + const open = ! currentClientId + ? undefined + : openedBlockSettingsMenu === currentClientId || false; const onToggle = useCallback( ( localOpen ) => { - // When the current menu is opened, update the state to reflect - // the new current menu. This allows all other instances of the - // menu to close if they already open. - if ( localOpen ) { + if ( localOpen && openedBlockSettingsMenu !== currentClientId ) { setOpenedBlockSettingsMenu( currentClientId ); + } else if ( + ! localOpen && + openedBlockSettingsMenu && + openedBlockSettingsMenu === currentClientId + ) { + setOpenedBlockSettingsMenu( undefined ); } }, - [ currentClientId, setOpenedBlockSettingsMenu ] + [ currentClientId, openedBlockSettingsMenu, setOpenedBlockSettingsMenu ] ); return ( diff --git a/packages/components/src/dropdown-menu/README.md b/packages/components/src/dropdown-menu/README.md index 8805ebd20862e..5dec609b918cf 100644 --- a/packages/components/src/dropdown-menu/README.md +++ b/packages/components/src/dropdown-menu/README.md @@ -171,13 +171,19 @@ A class name to apply to the dropdown menu's toggle element wrapper. - Required: No -### `open`: `boolean` +#### `defaultOpen`: `boolean` + +The initial open state of the dropdown. + +- Required: No + +#### `open`: `boolean` Control whether the dropdown is open or not. - Required: No -### `onToggle`: `( willOpen: boolean ) => void` +#### `onToggle`: `( willOpen: boolean ) => void` A callback invoked when the state of the popover changes from open to closed and vice versa. diff --git a/packages/components/src/dropdown-menu/index.tsx b/packages/components/src/dropdown-menu/index.tsx index 9995040a05d64..5925608bef49d 100644 --- a/packages/components/src/dropdown-menu/index.tsx +++ b/packages/components/src/dropdown-menu/index.tsx @@ -49,16 +49,18 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) { className, controls, icon = menu, - open: openProp, label, popoverProps, toggleProps, - onToggle: onToggleProp, menuProps, disableOpenOnArrowDown = false, text, noIcons, + open, + defaultOpen, + onToggle: onToggleProp, + // Context variant, } = useContextSystem< DropdownMenuProps & DropdownMenuInternalContext >( @@ -94,7 +96,8 @@ function UnconnectedDropdownMenu( dropdownMenuProps: DropdownMenuProps ) { return ( { diff --git a/packages/components/src/dropdown-menu/types.ts b/packages/components/src/dropdown-menu/types.ts index 808826eeea076..46fb8b0e694f3 100644 --- a/packages/components/src/dropdown-menu/types.ts +++ b/packages/components/src/dropdown-menu/types.ts @@ -68,10 +68,7 @@ export type DropdownMenuProps = { * @default "menu" */ icon?: IconProps[ 'icon' ] | null; - /** - * Whether the dropdown is opened or not. - */ - open?: boolean; + /** * A human-readable label to present as accessibility text on the focused * collapsed menu button. @@ -90,11 +87,6 @@ export type DropdownMenuProps = { * set with `position` prop. */ popoverProps?: DropdownProps[ 'popoverProps' ]; - /** - * A function that is called any time the menu is toggled from its closed - * state to its opened state, or vice versa. - */ - onToggle?: ( next: boolean ) => void; /** * Properties of `toggleProps` object will be passed as props to the nested * `Button` component in the `renderToggle` implementation of the `Dropdown` @@ -149,6 +141,19 @@ export type DropdownMenuProps = { * A valid DropdownMenu must specify a `controls` or `children` prop, or both. */ controls?: DropdownOption[] | DropdownOption[][]; + /** + * Whether the dropdown is opened or not. + */ + open?: boolean; + /** + * A function that is called any time the menu is toggled from its closed + * state to its opened state, or vice versa. + */ + onToggle?: ( willOpen: boolean ) => void; + /** + * The initial open state of the dropdown. + */ + defaultOpen?: boolean; }; export type DropdownMenuInternalContext = { diff --git a/packages/components/src/dropdown/README.md b/packages/components/src/dropdown/README.md index 987b0dcfb218c..ca2a3614ab4c9 100644 --- a/packages/components/src/dropdown/README.md +++ b/packages/components/src/dropdown/README.md @@ -68,6 +68,12 @@ Set this to customize the text that is shown in the dropdown's header when it is - Required: No +### `defaultOpen`: `boolean` + +The initial open state of the dropdown. + +- Required: No + ### `open`: `boolean` Control whether the dropdown is open or not. diff --git a/packages/components/src/dropdown/index.tsx b/packages/components/src/dropdown/index.tsx index 13be6bdada3f9..e591bbe1f1f14 100644 --- a/packages/components/src/dropdown/index.tsx +++ b/packages/components/src/dropdown/index.tsx @@ -7,7 +7,7 @@ import type { ForwardedRef } from 'react'; /** * WordPress dependencies */ -import { useEffect, useRef, useState } from '@wordpress/element'; +import { useRef, useState } from '@wordpress/element'; import { useMergeRefs } from '@wordpress/compose'; import deprecated from '@wordpress/deprecated'; @@ -15,25 +15,10 @@ import deprecated from '@wordpress/deprecated'; * Internal dependencies */ import { contextConnect, useContextSystem } from '../ui/context'; +import { useControlledValue } from '../utils/hooks'; import Popover from '../popover'; import type { DropdownProps, DropdownInternalContext } from './types'; -function useObservableState( - initialState: boolean, - onStateChange?: ( newState: boolean ) => void -) { - const [ state, setState ] = useState( initialState ); - return [ - state, - ( value: boolean ) => { - setState( value ); - if ( onStateChange ) { - onStateChange( value ); - } - }, - ] as const; -} - const UnconnectedDropdown = ( props: DropdownProps, forwardedRef: ForwardedRef< any > @@ -50,7 +35,9 @@ const UnconnectedDropdown = ( onClose, onToggle, style, - open: openProp, + + open, + defaultOpen, // Deprecated props position, @@ -75,23 +62,11 @@ const UnconnectedDropdown = ( const [ fallbackPopoverAnchor, setFallbackPopoverAnchor ] = useState< HTMLDivElement | null >( null ); const containerRef = useRef< HTMLDivElement >(); - const [ isOpenState, setIsOpen ] = useObservableState( false, onToggle ); - - // Allow provided `isOpen` prop to override internal state. - const isOpen = openProp ?? isOpenState; - - useEffect( - () => () => { - if ( onToggle && isOpen ) { - onToggle( false ); - } - }, - [ onToggle, isOpen ] - ); - - function toggle() { - setIsOpen( ! isOpen ); - } + const [ isOpen, setIsOpen ] = useControlledValue( { + defaultValue: defaultOpen, + value: open, + onChange: onToggle, + } ); /** * Closes the popover when focus leaves it unless the toggle was pressed or @@ -122,7 +97,11 @@ const UnconnectedDropdown = ( setIsOpen( false ); } - const args = { isOpen, onToggle: toggle, onClose: close }; + const args = { + isOpen: !! isOpen, + onToggle: () => setIsOpen( ! isOpen ), + onClose: close, + }; const popoverPropsHaveAnchor = !! popoverProps?.anchor || // Note: `anchorRef`, `getAnchorRect` and `anchorRect` are deprecated and diff --git a/packages/components/src/dropdown/types.ts b/packages/components/src/dropdown/types.ts index 291a40bdef53f..e498f6bc50c03 100644 --- a/packages/components/src/dropdown/types.ts +++ b/packages/components/src/dropdown/types.ts @@ -35,6 +35,10 @@ export type DropdownProps = { * as a child of the container node. */ contentClassName?: string; + /** + * The initial open state of the dropdown. + */ + defaultOpen?: boolean; /** * Opt-in prop to show popovers fullscreen on mobile. *