From e95e45e26fe86b3f29717d8c9f49cbd73c55e7d4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 22 Apr 2021 19:14:01 +0300 Subject: [PATCH 1/2] Block editor: fix focus handler in Safari --- .../block-list/use-block-props/index.js | 4 +- .../use-block-props/use-focus-handler.js | 56 ++++++++++++++++++ ...s => use-selected-block-event-handlers.js} | 57 +++++-------------- 3 files changed, 72 insertions(+), 45 deletions(-) create mode 100644 packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js rename packages/block-editor/src/components/block-list/use-block-props/{use-event-handlers.js => use-selected-block-event-handlers.js} (66%) diff --git a/packages/block-editor/src/components/block-list/use-block-props/index.js b/packages/block-editor/src/components/block-list/use-block-props/index.js index 91c27c025b4a6..627f848ebf841 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/index.js +++ b/packages/block-editor/src/components/block-list/use-block-props/index.js @@ -26,7 +26,8 @@ import { useBlockClassNames } from './use-block-class-names'; import { useBlockDefaultClassName } from './use-block-default-class-name'; import { useBlockCustomClassName } from './use-block-custom-class-name'; import { useBlockMovingModeClassNames } from './use-block-moving-mode-class-names'; -import { useEventHandlers } from './use-event-handlers'; +import { useFocusHandler } from './use-focus-handler'; +import { useEventHandlers } from './use-selected-block-event-handlers'; import { useNavModeExit } from './use-nav-mode-exit'; import { useBlockNodes } from './use-block-nodes'; import { useScrollIntoView } from './use-scroll-into-view'; @@ -106,6 +107,7 @@ export function useBlockProps( props = {}, { __unstableIsHtml } = {} ) { // Must happen after focus because we check for focus in the block. useScrollIntoView( clientId ), useBlockNodes( clientId ), + useFocusHandler( clientId ), useEventHandlers( clientId ), useNavModeExit( clientId ), useIsHovered(), diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js b/packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js new file mode 100644 index 0000000000000..4946cc40a26c9 --- /dev/null +++ b/packages/block-editor/src/components/block-list/use-block-props/use-focus-handler.js @@ -0,0 +1,56 @@ +/** + * WordPress dependencies + */ +import { useSelect, useDispatch } from '@wordpress/data'; +import { useRefEffect } from '@wordpress/compose'; + +/** + * Internal dependencies + */ +import { isInsideRootBlock } from '../../../utils/dom'; +import { store as blockEditorStore } from '../../../store'; + +/** + * Selects the block if it receives focus. + * + * @param {string} clientId Block client ID. + */ +export function useFocusHandler( clientId ) { + const { isBlockSelected } = useSelect( blockEditorStore ); + const { selectBlock } = useDispatch( blockEditorStore ); + + return useRefEffect( + ( node ) => { + /** + * Marks the block as selected when focused and not already + * selected. This specifically handles the case where block does not + * set focus on its own (via `setFocus`), typically if there is no + * focusable input in the block. + * + * @param {FocusEvent} event Focus event. + */ + function onFocus( event ) { + // Check synchronously because a non-selected block might be + // getting data through `useSelect` asynchronously. + if ( isBlockSelected( clientId ) ) { + return; + } + + // If an inner block is focussed, that block is resposible for + // setting the selected block. + if ( ! isInsideRootBlock( node, event.target ) ) { + return; + } + + selectBlock( clientId ); + } + + node.addEventListener( 'focusin', onFocus ); + + return () => { + node.removeEventListener( 'focusin', onFocus ); + }; + }, + [ isBlockSelected, selectBlock ] + ); +} diff --git a/packages/block-editor/src/components/block-list/use-block-props/use-event-handlers.js b/packages/block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js similarity index 66% rename from packages/block-editor/src/components/block-list/use-block-props/use-event-handlers.js rename to packages/block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js index 844a6862fd8eb..d5ee93fe3f988 100644 --- a/packages/block-editor/src/components/block-list/use-block-props/use-event-handlers.js +++ b/packages/block-editor/src/components/block-list/use-block-props/use-selected-block-event-handlers.js @@ -10,13 +10,11 @@ import { useRefEffect } from '@wordpress/compose'; /** * Internal dependencies */ -import { isInsideRootBlock } from '../../../utils/dom'; import { SelectionStart } from '../../writing-flow'; import { store as blockEditorStore } from '../../../store'; /** * Adds block behaviour: - * - Selects the block if it receives focus. * - Removes the block on BACKSPACE. * - Inserts a default block on ENTER. * - Initiates selection start for multi-selection. @@ -26,52 +24,19 @@ import { store as blockEditorStore } from '../../../store'; */ export function useEventHandlers( clientId ) { const onSelectionStart = useContext( SelectionStart ); - const { isSelected, rootClientId, index } = useSelect( - ( select ) => { - const { - isBlockSelected, - getBlockRootClientId, - getBlockIndex, - } = select( blockEditorStore ); - - return { - isSelected: isBlockSelected( clientId ), - rootClientId: getBlockRootClientId( clientId ), - index: getBlockIndex( clientId ), - }; - }, + const isSelected = useSelect( + ( select ) => select( blockEditorStore ).isBlockSelected( clientId ), [ clientId ] ); - const { insertDefaultBlock, removeBlock, selectBlock } = useDispatch( + const { getBlockRootClientId, getBlockIndex } = useSelect( blockEditorStore ); + const { insertDefaultBlock, removeBlock } = useDispatch( blockEditorStore ); return useRefEffect( ( node ) => { if ( ! isSelected ) { - /** - * Marks the block as selected when focused and not already - * selected. This specifically handles the case where block does not - * set focus on its own (via `setFocus`), typically if there is no - * focusable input in the block. - * - * @param {FocusEvent} event Focus event. - */ - function onFocus( event ) { - // If an inner block is focussed, that block is resposible for - // setting the selected block. - if ( ! isInsideRootBlock( node, event.target ) ) { - return; - } - - selectBlock( clientId ); - } - - node.addEventListener( 'focusin', onFocus ); - - return () => { - node.removeEventListener( 'focusin', onFocus ); - }; + return; } /** @@ -101,7 +66,11 @@ export function useEventHandlers( clientId ) { event.preventDefault(); if ( keyCode === ENTER ) { - insertDefaultBlock( {}, rootClientId, index + 1 ); + insertDefaultBlock( + {}, + getBlockRootClientId( clientId ), + getBlockIndex( clientId ) + 1 + ); } else { removeBlock( clientId ); } @@ -136,13 +105,13 @@ export function useEventHandlers( clientId ) { }; }, [ + clientId, isSelected, - rootClientId, - index, + getBlockRootClientId, + getBlockIndex, onSelectionStart, insertDefaultBlock, removeBlock, - selectBlock, ] ); } From 57953f4b3d733626db22a2746e8640c382f0c400 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ella=20van=C2=A0Durpe?= Date: Thu, 22 Apr 2021 20:07:44 +0300 Subject: [PATCH 2/2] Attempt to fix e2e tests --- .../block-editor/src/components/block-navigation/dropdown.js | 3 +-- packages/e2e-tests/specs/editor/blocks/list.test.js | 1 + 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/block-editor/src/components/block-navigation/dropdown.js b/packages/block-editor/src/components/block-navigation/dropdown.js index b72d879d6762e..6ffe00fc93f8d 100644 --- a/packages/block-editor/src/components/block-navigation/dropdown.js +++ b/packages/block-editor/src/components/block-navigation/dropdown.js @@ -80,9 +80,8 @@ function BlockNavigationDropdown( isEnabled={ isEnabled } /> ) } - renderContent={ ( { onClose } ) => ( + renderContent={ () => ( ) } diff --git a/packages/e2e-tests/specs/editor/blocks/list.test.js b/packages/e2e-tests/specs/editor/blocks/list.test.js index 57d9fc40e7a3f..ddc115e2195c4 100644 --- a/packages/e2e-tests/specs/editor/blocks/list.test.js +++ b/packages/e2e-tests/specs/editor/blocks/list.test.js @@ -113,6 +113,7 @@ describe( 'List', () => { await clickBlockAppender(); await page.keyboard.press( 'Enter' ); await page.keyboard.type( '* ' ); + await page.evaluate( () => new Promise( window.requestIdleCallback ) ); await page.keyboard.press( 'ArrowUp' ); await page.keyboard.press( 'ArrowDown' ); await page.keyboard.press( 'Backspace' );