From 21b6a3717c1b09c376f7a5d914b0953aa2cdcddd Mon Sep 17 00:00:00 2001 From: Andrew Serong <14988353+andrewserong@users.noreply.github.com> Date: Thu, 14 Sep 2023 11:18:08 +1000 Subject: [PATCH] BlockSettingsMenu: Ensure only one block settings menu is open at a time (#54083) * BlockSettingsMenu and DropdownMenu: Ensure only one BlockSettingsMenu is open at a time * Update Readme files * Update reducer to fall back to undefined Co-authored-by: Ramon * Rename isOpen to open * Try switching to a controlled component * Ensure menu is closed when blocks are inserted before/after via the menu, help stabilise test * Simplify useSelect Co-authored by: Ramon * Add an explanatory comment describing the current solution as a temporary solution * Update action, reducer, and selectors for consistency, add tests --------- Co-authored-by: Ramon Co-authored-by: Marin Atanasov <8436925+tyxla@users.noreply.github.com> --- .../block-settings-dropdown.js | 44 +++++++++++++++++++ .../block-editor/src/store/private-actions.js | 13 ++++++ .../src/store/private-selectors.js | 10 +++++ packages/block-editor/src/store/reducer.js | 16 +++++++ .../src/store/test/private-actions.js | 17 +++++++ .../block-editor/src/store/test/reducer.js | 25 +++++++++++ .../specs/editor/various/list-view.spec.js | 4 ++ 7 files changed, 129 insertions(+) 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 ce059d7720a9ff..bb8e00d0c11db9 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 @@ -31,6 +31,7 @@ import BlockHTMLConvertButton from './block-html-convert-button'; import __unstableBlockSettingsMenuFirstItem from './block-settings-menu-first-item'; import BlockSettingsMenuControls from '../block-settings-menu-controls'; import { store as blockEditorStore } from '../../store'; +import { unlock } from '../../lock-unlock'; import { useShowHoveredOrFocusedGestures } from '../block-toolbar/utils'; const POPOVER_PROPS = { @@ -47,12 +48,15 @@ function CopyMenuItem( { blocks, onCopy, label } ) { } export function BlockSettingsDropdown( { + block, clientIds, __experimentalSelectBlock, children, __unstableDisplayLocation, ...props } ) { + // Get the client id of the current block for this menu, if one is set. + const currentClientId = block?.clientId; const blockClientIds = Array.isArray( clientIds ) ? clientIds : [ clientIds ]; @@ -102,6 +106,16 @@ export function BlockSettingsDropdown( { const { getBlockOrder, getSelectedBlockClientIds } = useSelect( blockEditorStore ); + const openedBlockSettingsMenu = useSelect( + ( select ) => + unlock( select( blockEditorStore ) ).getOpenedBlockSettingsMenu(), + [] + ); + + const { setOpenedBlockSettingsMenu } = unlock( + useDispatch( blockEditorStore ) + ); + const shortcuts = useSelect( ( select ) => { const { getShortcutRepresentation } = select( keyboardShortcutsStore ); return { @@ -174,6 +188,32 @@ export function BlockSettingsDropdown( { const parentBlockIsSelected = selectedBlockClientIds?.includes( firstParentClientId ); + // 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. + // This is a temporary solution to work around an issue with `onFocusOutside` + // where it does not allow a dropdown to be closed if focus was never within + // the dropdown to begin with. Examples include a user either CMD+Clicking or + // right clicking into an inactive window. + // See: https://github.com/WordPress/gutenberg/pull/54083 + const open = ! currentClientId + ? undefined + : openedBlockSettingsMenu === currentClientId || false; + + const onToggle = useCallback( + ( localOpen ) => { + if ( localOpen && openedBlockSettingsMenu !== currentClientId ) { + setOpenedBlockSettingsMenu( currentClientId ); + } else if ( + ! localOpen && + openedBlockSettingsMenu && + openedBlockSettingsMenu === currentClientId + ) { + setOpenedBlockSettingsMenu( undefined ); + } + }, + [ currentClientId, openedBlockSettingsMenu, setOpenedBlockSettingsMenu ] + ); + return ( { @@ -78,4 +79,20 @@ describe( 'private actions', () => { } ); } ); } ); + + describe( 'setOpenedBlockSettingsMenu', () => { + it( 'should return the SET_OPENED_BLOCK_SETTINGS_MENU action', () => { + expect( setOpenedBlockSettingsMenu() ).toEqual( { + clientId: undefined, + type: 'SET_OPENED_BLOCK_SETTINGS_MENU', + } ); + } ); + + it( 'should return the SET_OPENED_BLOCK_SETTINGS_MENU action with client id if provided', () => { + expect( setOpenedBlockSettingsMenu( 'abcd' ) ).toEqual( { + clientId: 'abcd', + type: 'SET_OPENED_BLOCK_SETTINGS_MENU', + } ); + } ); + } ); } ); diff --git a/packages/block-editor/src/store/test/reducer.js b/packages/block-editor/src/store/test/reducer.js index 9d886f9aa37bbc..8c82d1c3092b16 100644 --- a/packages/block-editor/src/store/test/reducer.js +++ b/packages/block-editor/src/store/test/reducer.js @@ -33,6 +33,7 @@ import { lastBlockAttributesChange, lastBlockInserted, blockEditingModes, + openedBlockSettingsMenu, } from '../reducer'; const noop = () => {}; @@ -3415,4 +3416,28 @@ describe( 'state', () => { ); } ); } ); + + describe( 'openedBlockSettingsMenu', () => { + it( 'should return null by default', () => { + expect( openedBlockSettingsMenu( undefined, {} ) ).toBe( null ); + } ); + + it( 'should set client id for opened block settings menu', () => { + const state = openedBlockSettingsMenu( null, { + type: 'SET_OPENED_BLOCK_SETTINGS_MENU', + clientId: '14501cc2-90a6-4f52-aa36-ab6e896135d1', + } ); + expect( state ).toBe( '14501cc2-90a6-4f52-aa36-ab6e896135d1' ); + } ); + + it( 'should clear the state when no client id is passed', () => { + const state = openedBlockSettingsMenu( + '14501cc2-90a6-4f52-aa36-ab6e896135d1', + { + type: 'SET_OPENED_BLOCK_SETTINGS_MENU', + } + ); + expect( state ).toBe( null ); + } ); + } ); } ); diff --git a/test/e2e/specs/editor/various/list-view.spec.js b/test/e2e/specs/editor/various/list-view.spec.js index 130bd9b7952fde..e0ac030d1fd24b 100644 --- a/test/e2e/specs/editor/various/list-view.spec.js +++ b/test/e2e/specs/editor/various/list-view.spec.js @@ -784,6 +784,10 @@ test.describe( 'List View', () => { ).toBeHidden(); await optionsForFileToggle.click(); + await expect( + optionsForFileMenu, + 'Pressing Space should also open the menu dropdown' + ).toBeVisible(); await pageUtils.pressKeys( 'access+z' ); // Keyboard shortcut for Delete. await expect .poll(