From a7151edee5fec4ad6c694a16537f422cab78f05c Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 25 Aug 2023 12:12:34 +0100 Subject: [PATCH 01/18] Make the shortcut provider optional --- .../src/components/customize-widgets/index.js | 28 ++++++------- .../test/index.js | 9 ++--- packages/edit-post/src/editor.js | 37 ++++++++---------- .../edit-site/src/components/app/index.js | 21 +++++----- .../index.js | 29 +++++++------- packages/keyboard-shortcuts/README.md | 2 +- .../src/components/shortcut-provider.js | 2 + packages/keyboard-shortcuts/src/context.js | 9 ++++- storybook/stories/playground/index.story.js | 39 +++++++++---------- .../helpers/integration-test-editor.js | 29 +++++++------- 10 files changed, 96 insertions(+), 109 deletions(-) diff --git a/packages/customize-widgets/src/components/customize-widgets/index.js b/packages/customize-widgets/src/components/customize-widgets/index.js index 3306e438f80e9..d206108398283 100644 --- a/packages/customize-widgets/src/components/customize-widgets/index.js +++ b/packages/customize-widgets/src/components/customize-widgets/index.js @@ -3,7 +3,6 @@ */ import { useState, useEffect, useRef, createPortal } from '@wordpress/element'; import { SlotFillProvider, Popover } from '@wordpress/components'; -import { ShortcutProvider } from '@wordpress/keyboard-shortcuts'; /** * Internal dependencies @@ -68,21 +67,16 @@ export default function CustomizeWidgets( { ); return ( - - - - - { activeSidebar } - { popover } - - - - + + + + { activeSidebar } + { popover } + + + ); } diff --git a/packages/edit-post/src/components/keyboard-shortcut-help-modal/test/index.js b/packages/edit-post/src/components/keyboard-shortcut-help-modal/test/index.js index 6d8982501d137..0380e648a1733 100644 --- a/packages/edit-post/src/components/keyboard-shortcut-help-modal/test/index.js +++ b/packages/edit-post/src/components/keyboard-shortcut-help-modal/test/index.js @@ -7,7 +7,6 @@ import { render, screen } from '@testing-library/react'; * WordPress dependencies */ import { EditorKeyboardShortcutsRegister } from '@wordpress/editor'; -import { ShortcutProvider } from '@wordpress/keyboard-shortcuts'; /** * Internal dependencies @@ -19,10 +18,10 @@ const noop = () => {}; describe( 'KeyboardShortcutHelpModal', () => { it( 'should match snapshot when the modal is active', () => { render( - + <> - + ); expect( @@ -34,13 +33,13 @@ describe( 'KeyboardShortcutHelpModal', () => { it( 'should not render the modal when inactive', () => { render( - + <> - + ); expect( diff --git a/packages/edit-post/src/editor.js b/packages/edit-post/src/editor.js index 00bc911dd7626..6668001b76773 100644 --- a/packages/edit-post/src/editor.js +++ b/packages/edit-post/src/editor.js @@ -12,7 +12,6 @@ import { import { useMemo } from '@wordpress/element'; import { SlotFillProvider } from '@wordpress/components'; import { store as coreStore } from '@wordpress/core-data'; -import { ShortcutProvider } from '@wordpress/keyboard-shortcuts'; import { store as preferencesStore } from '@wordpress/preferences'; import { CommandMenu } from '@wordpress/commands'; import { privateApis as coreCommandsPrivateApis } from '@wordpress/core-commands'; @@ -156,25 +155,23 @@ function Editor( { postId, postType, settings, initialEdits, ...props } ) { } return ( - - - - - - - - - - - - + + + + + + + + + + ); } diff --git a/packages/edit-site/src/components/app/index.js b/packages/edit-site/src/components/app/index.js index ca1893c3c1775..cad76b3ea1fb8 100644 --- a/packages/edit-site/src/components/app/index.js +++ b/packages/edit-site/src/components/app/index.js @@ -3,7 +3,6 @@ */ import { SlotFillProvider } from '@wordpress/components'; import { UnsavedChangesWarning } from '@wordpress/editor'; -import { ShortcutProvider } from '@wordpress/keyboard-shortcuts'; import { store as noticesStore } from '@wordpress/notices'; import { useDispatch } from '@wordpress/data'; import { __, sprintf } from '@wordpress/i18n'; @@ -35,16 +34,14 @@ export default function App() { } return ( - - - - - - - - - - - + + + + + + + + + ); } diff --git a/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js b/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js index 899cc02766480..f3a7b84d5ded6 100644 --- a/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js +++ b/packages/edit-widgets/src/components/widget-areas-block-editor-provider/index.js @@ -15,7 +15,6 @@ import { privateApis as blockEditorPrivateApis, } from '@wordpress/block-editor'; import { privateApis as editPatternsPrivateApis } from '@wordpress/patterns'; -import { ShortcutProvider } from '@wordpress/keyboard-shortcuts'; import { store as preferencesStore } from '@wordpress/preferences'; /** @@ -98,21 +97,19 @@ export default function WidgetAreasBlockEditorProvider( { ); return ( - + - - - { children } - - - - + + { children } + + + ); } diff --git a/packages/keyboard-shortcuts/README.md b/packages/keyboard-shortcuts/README.md index 4138b44a8e685..281ef698526e7 100644 --- a/packages/keyboard-shortcuts/README.md +++ b/packages/keyboard-shortcuts/README.md @@ -18,7 +18,7 @@ _This package assumes that your code will run in an **ES2015+** environment. If ### ShortcutProvider -Handles callbacks added to context by `useShortcut`. +Handles callbacks added to context by `useShortcut`. Adding a provider allows to register contextual shortcuts that are only active when a certain part of the UI is focused. _Parameters_ diff --git a/packages/keyboard-shortcuts/src/components/shortcut-provider.js b/packages/keyboard-shortcuts/src/components/shortcut-provider.js index bebad586268a7..9fabf392738f6 100644 --- a/packages/keyboard-shortcuts/src/components/shortcut-provider.js +++ b/packages/keyboard-shortcuts/src/components/shortcut-provider.js @@ -12,6 +12,8 @@ const { Provider } = context; /** * Handles callbacks added to context by `useShortcut`. + * Adding a provider allows to register contextual shortcuts + * that are only active when a certain part of the UI is focused. * * @param {Object} props Props to pass to `div`. * diff --git a/packages/keyboard-shortcuts/src/context.js b/packages/keyboard-shortcuts/src/context.js index bb885af5c721f..32a535e9c1f06 100644 --- a/packages/keyboard-shortcuts/src/context.js +++ b/packages/keyboard-shortcuts/src/context.js @@ -3,4 +3,11 @@ */ import { createContext } from '@wordpress/element'; -export const context = createContext(); +const globalShortcuts = { current: new Set() }; +document.body.addEventListener( 'keydown', ( event ) => { + for ( const keyboardShortcut of globalShortcuts.current ) { + keyboardShortcut( event ); + } +} ); + +export const context = createContext( globalShortcuts ); diff --git a/storybook/stories/playground/index.story.js b/storybook/stories/playground/index.story.js index 380a95f4b79a9..c95da2b4c365b 100644 --- a/storybook/stories/playground/index.story.js +++ b/storybook/stories/playground/index.story.js @@ -10,7 +10,6 @@ import { WritingFlow, } from '@wordpress/block-editor'; import { registerCoreBlocks } from '@wordpress/block-library'; -import { ShortcutProvider } from '@wordpress/keyboard-shortcuts'; import '@wordpress/format-library'; /** @@ -35,26 +34,24 @@ function App() { return (
- - -
- -
-
- -
- - - -
- -
- - + +
+ +
+
+ +
+ + + +
+ +
+
); } diff --git a/test/integration/helpers/integration-test-editor.js b/test/integration/helpers/integration-test-editor.js index 7c2fba15060b4..c8694f88f1fb5 100644 --- a/test/integration/helpers/integration-test-editor.js +++ b/test/integration/helpers/integration-test-editor.js @@ -16,7 +16,6 @@ import { WritingFlow, } from '@wordpress/block-editor'; import { registerCoreBlocks } from '@wordpress/block-library'; -import { ShortcutProvider } from '@wordpress/keyboard-shortcuts'; import '@wordpress/format-library'; import { createBlock, @@ -66,21 +65,19 @@ export function Editor( { testBlocks, settings = {} } ) { }, [] ); return ( - - - - - - - - - - + + + + + + + + ); } From 88b69b23ca848549f17d5d8c31f9daf7d6d9020c Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 25 Aug 2023 16:38:24 +0100 Subject: [PATCH 02/18] Bubble keydown events --- packages/block-editor/src/components/iframe/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 19c1b61058016..14139547ca947 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -70,7 +70,7 @@ function bubbleEvents( doc ) { } } - const eventTypes = [ 'dragover', 'mousemove' ]; + const eventTypes = [ 'dragover', 'mousemove', 'keydown' ]; for ( const name of eventTypes ) { doc.addEventListener( name, bubbleEvent ); From ab7c07c217299d5fc14b1948de25e213783e594e Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 25 Aug 2023 17:22:17 +0100 Subject: [PATCH 03/18] Prevent double region navigation --- .../components/src/higher-order/navigate-regions/index.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/components/src/higher-order/navigate-regions/index.tsx b/packages/components/src/higher-order/navigate-regions/index.tsx index f2b9c31bfbe95..320f1f7d67908 100644 --- a/packages/components/src/higher-order/navigate-regions/index.tsx +++ b/packages/components/src/higher-order/navigate-regions/index.tsx @@ -70,7 +70,6 @@ export function useNavigateRegions( shortcuts: Shortcuts = defaultShortcuts ) { nextIndex = nextIndex === regions.length ? 0 : nextIndex; nextRegion = regions[ nextIndex ]; } - nextRegion.focus(); setIsFocusingRegions( true ); } @@ -99,12 +98,14 @@ export function useNavigateRegions( shortcuts: Shortcuts = defaultShortcuts ) { return isKeyboardEvent[ modifier ]( event, character ); } ) ) { + event.stopPropagation(); focusRegion( -1 ); } else if ( shortcuts.next.some( ( { modifier, character } ) => { return isKeyboardEvent[ modifier ]( event, character ); } ) ) { + event.stopPropagation(); focusRegion( 1 ); } }, From 2a6247cbddb8f81442eed73e0131cb642490fc3b Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 25 Aug 2023 17:53:37 +0100 Subject: [PATCH 04/18] Prevent bubbling when navigation mode is handled --- packages/block-editor/src/components/writing-flow/use-tab-nav.js | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/block-editor/src/components/writing-flow/use-tab-nav.js b/packages/block-editor/src/components/writing-flow/use-tab-nav.js index 616da1bc75813..0089488e21278 100644 --- a/packages/block-editor/src/components/writing-flow/use-tab-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-tab-nav.js @@ -89,6 +89,7 @@ export default function useTabNav() { if ( event.keyCode === ESCAPE && ! hasMultiSelection() ) { event.preventDefault(); + event.stopPropagation(); setNavigationMode( true ); return; } From a94eb2dde45d2006fdb7c47d0b3d6d505542f76f Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Mon, 28 Aug 2023 09:54:01 +0100 Subject: [PATCH 05/18] Wait to register the event handler --- packages/keyboard-shortcuts/src/context.js | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/keyboard-shortcuts/src/context.js b/packages/keyboard-shortcuts/src/context.js index 32a535e9c1f06..dfc9a34db2fcc 100644 --- a/packages/keyboard-shortcuts/src/context.js +++ b/packages/keyboard-shortcuts/src/context.js @@ -4,10 +4,12 @@ import { createContext } from '@wordpress/element'; const globalShortcuts = { current: new Set() }; -document.body.addEventListener( 'keydown', ( event ) => { - for ( const keyboardShortcut of globalShortcuts.current ) { - keyboardShortcut( event ); - } +document.addEventListener( 'DOMContentLoaded', () => { + document.body.addEventListener( 'keydown', ( event ) => { + for ( const keyboardShortcut of globalShortcuts.current ) { + keyboardShortcut( event ); + } + } ); } ); export const context = createContext( globalShortcuts ); From 0138f1367e4b6cfd864192c4539638dfa63c4280 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 29 Aug 2023 10:43:33 +0100 Subject: [PATCH 06/18] SSR support for keyboard shortcuts --- packages/keyboard-shortcuts/src/context.js | 31 +++++++++++++++------- 1 file changed, 22 insertions(+), 9 deletions(-) diff --git a/packages/keyboard-shortcuts/src/context.js b/packages/keyboard-shortcuts/src/context.js index dfc9a34db2fcc..5954e74405939 100644 --- a/packages/keyboard-shortcuts/src/context.js +++ b/packages/keyboard-shortcuts/src/context.js @@ -3,13 +3,26 @@ */ import { createContext } from '@wordpress/element'; -const globalShortcuts = { current: new Set() }; -document.addEventListener( 'DOMContentLoaded', () => { - document.body.addEventListener( 'keydown', ( event ) => { - for ( const keyboardShortcut of globalShortcuts.current ) { - keyboardShortcut( event ); - } - } ); -} ); +const globalShortcuts = new Set(); +const globalListener = ( event ) => { + for ( const keyboardShortcut of globalShortcuts ) { + keyboardShortcut( event ); + } +}; -export const context = createContext( globalShortcuts ); +export const context = createContext( { + current: { + add: ( shortcut ) => { + if ( globalShortcuts.size === 0 ) { + document.addEventListener( 'keydown', globalListener ); + } + globalShortcuts.add( shortcut ); + }, + remove: ( shortcut ) => { + globalShortcuts.delete( shortcut ); + if ( globalShortcuts.size === 0 ) { + document.removeEventListener( 'keydown', globalListener ); + } + }, + }, +} ); From 93537de3ad04d6ce4fd4987662568112ab3333c9 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 29 Aug 2023 14:48:59 +0100 Subject: [PATCH 07/18] Small improvements --- packages/keyboard-shortcuts/src/context.js | 2 +- packages/keyboard-shortcuts/src/hooks/use-shortcut.js | 11 +++++++++-- 2 files changed, 10 insertions(+), 3 deletions(-) diff --git a/packages/keyboard-shortcuts/src/context.js b/packages/keyboard-shortcuts/src/context.js index 5954e74405939..f11990a510dbb 100644 --- a/packages/keyboard-shortcuts/src/context.js +++ b/packages/keyboard-shortcuts/src/context.js @@ -18,7 +18,7 @@ export const context = createContext( { } globalShortcuts.add( shortcut ); }, - remove: ( shortcut ) => { + delete: ( shortcut ) => { globalShortcuts.delete( shortcut ); if ( globalShortcuts.size === 0 ) { document.removeEventListener( 'keydown', globalListener ); diff --git a/packages/keyboard-shortcuts/src/hooks/use-shortcut.js b/packages/keyboard-shortcuts/src/hooks/use-shortcut.js index c60f84cc58da7..231dae6ed9c98 100644 --- a/packages/keyboard-shortcuts/src/hooks/use-shortcut.js +++ b/packages/keyboard-shortcuts/src/hooks/use-shortcut.js @@ -17,11 +17,18 @@ import { context } from '../context'; * @param {Object} options Shortcut options. * @param {boolean} options.isDisabled Whether to disable to shortut. */ -export default function useShortcut( name, callback, { isDisabled } = {} ) { +export default function useShortcut( + name, + callback, + { isDisabled = false } = {} +) { const shortcuts = useContext( context ); const isMatch = useShortcutEventMatch(); const callbackRef = useRef(); - callbackRef.current = callback; + + useEffect( () => { + callbackRef.current = callback; + } ); useEffect( () => { if ( isDisabled ) { From 82a856bbaf1854b49d1167ca690a790db21d4e07 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 29 Aug 2023 14:50:30 +0100 Subject: [PATCH 08/18] Small improvements --- packages/keyboard-shortcuts/src/context.js | 24 +++++++++---------- .../src/hooks/use-shortcut.js | 6 ++--- 2 files changed, 14 insertions(+), 16 deletions(-) diff --git a/packages/keyboard-shortcuts/src/context.js b/packages/keyboard-shortcuts/src/context.js index f11990a510dbb..f2f8846751291 100644 --- a/packages/keyboard-shortcuts/src/context.js +++ b/packages/keyboard-shortcuts/src/context.js @@ -11,18 +11,16 @@ const globalListener = ( event ) => { }; export const context = createContext( { - current: { - add: ( shortcut ) => { - if ( globalShortcuts.size === 0 ) { - document.addEventListener( 'keydown', globalListener ); - } - globalShortcuts.add( shortcut ); - }, - delete: ( shortcut ) => { - globalShortcuts.delete( shortcut ); - if ( globalShortcuts.size === 0 ) { - document.removeEventListener( 'keydown', globalListener ); - } - }, + add: ( shortcut ) => { + if ( globalShortcuts.size === 0 ) { + document.addEventListener( 'keydown', globalListener ); + } + globalShortcuts.add( shortcut ); + }, + delete: ( shortcut ) => { + globalShortcuts.delete( shortcut ); + if ( globalShortcuts.size === 0 ) { + document.removeEventListener( 'keydown', globalListener ); + } }, } ); diff --git a/packages/keyboard-shortcuts/src/hooks/use-shortcut.js b/packages/keyboard-shortcuts/src/hooks/use-shortcut.js index 231dae6ed9c98..9cac7edc92e9d 100644 --- a/packages/keyboard-shortcuts/src/hooks/use-shortcut.js +++ b/packages/keyboard-shortcuts/src/hooks/use-shortcut.js @@ -41,9 +41,9 @@ export default function useShortcut( } } - shortcuts.current.add( _callback ); + shortcuts.add( _callback ); return () => { - shortcuts.current.delete( _callback ); + shortcuts.delete( _callback ); }; - }, [ name, isDisabled ] ); + }, [ name, isDisabled, shortcuts ] ); } From 687c6532ec06d8fc9cbccca3d7a946fca5dcf782 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 29 Aug 2023 15:26:40 +0100 Subject: [PATCH 09/18] Small update --- packages/keyboard-shortcuts/src/hooks/use-shortcut.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/keyboard-shortcuts/src/hooks/use-shortcut.js b/packages/keyboard-shortcuts/src/hooks/use-shortcut.js index 9cac7edc92e9d..b6252ebc3680c 100644 --- a/packages/keyboard-shortcuts/src/hooks/use-shortcut.js +++ b/packages/keyboard-shortcuts/src/hooks/use-shortcut.js @@ -28,7 +28,7 @@ export default function useShortcut( useEffect( () => { callbackRef.current = callback; - } ); + }, [ callback ] ); useEffect( () => { if ( isDisabled ) { From 96d46840458466a8b23a95041bb083aa166307af Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Tue, 29 Aug 2023 16:54:28 +0100 Subject: [PATCH 10/18] Fix e2e tests --- packages/block-editor/src/components/block-tools/index.js | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index 8e3b240838fd0..b804ad6dd013f 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -71,6 +71,7 @@ export default function BlockTools( { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); + event.stopPropagation(); const rootClientId = getBlockRootClientId( clientIds[ 0 ] ); moveBlocksUp( clientIds, rootClientId ); } @@ -78,6 +79,7 @@ export default function BlockTools( { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); + event.stopPropagation(); const rootClientId = getBlockRootClientId( clientIds[ 0 ] ); moveBlocksDown( clientIds, rootClientId ); } @@ -85,30 +87,35 @@ export default function BlockTools( { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); + event.stopPropagation(); duplicateBlocks( clientIds ); } } else if ( isMatch( 'core/block-editor/remove', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); + event.stopPropagation(); removeBlocks( clientIds ); } } else if ( isMatch( 'core/block-editor/insert-after', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); + event.stopPropagation(); insertAfterBlock( clientIds[ clientIds.length - 1 ] ); } } else if ( isMatch( 'core/block-editor/insert-before', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); + event.stopPropagation(); insertBeforeBlock( clientIds[ 0 ] ); } } else if ( isMatch( 'core/block-editor/unselect', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); + event.stopPropagation(); // If there is more than one block selected, select the first // block so that focus is directed back to the beginning of the selection. From e10d9d26e3b07d68fd0a8c120cfb2dbab9fae21c Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 31 Aug 2023 05:21:56 +0100 Subject: [PATCH 11/18] Prevent the iframe event from bubbling twice --- packages/block-editor/src/components/block-tools/index.js | 7 ------- packages/block-editor/src/components/iframe/index.js | 5 ++++- .../src/components/writing-flow/use-tab-nav.js | 1 - .../components/src/higher-order/navigate-regions/index.tsx | 2 -- 4 files changed, 4 insertions(+), 11 deletions(-) diff --git a/packages/block-editor/src/components/block-tools/index.js b/packages/block-editor/src/components/block-tools/index.js index b804ad6dd013f..8e3b240838fd0 100644 --- a/packages/block-editor/src/components/block-tools/index.js +++ b/packages/block-editor/src/components/block-tools/index.js @@ -71,7 +71,6 @@ export default function BlockTools( { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); - event.stopPropagation(); const rootClientId = getBlockRootClientId( clientIds[ 0 ] ); moveBlocksUp( clientIds, rootClientId ); } @@ -79,7 +78,6 @@ export default function BlockTools( { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); - event.stopPropagation(); const rootClientId = getBlockRootClientId( clientIds[ 0 ] ); moveBlocksDown( clientIds, rootClientId ); } @@ -87,35 +85,30 @@ export default function BlockTools( { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); - event.stopPropagation(); duplicateBlocks( clientIds ); } } else if ( isMatch( 'core/block-editor/remove', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); - event.stopPropagation(); removeBlocks( clientIds ); } } else if ( isMatch( 'core/block-editor/insert-after', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); - event.stopPropagation(); insertAfterBlock( clientIds[ clientIds.length - 1 ] ); } } else if ( isMatch( 'core/block-editor/insert-before', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); - event.stopPropagation(); insertBeforeBlock( clientIds[ 0 ] ); } } else if ( isMatch( 'core/block-editor/unselect', event ) ) { const clientIds = getSelectedBlockClientIds(); if ( clientIds.length ) { event.preventDefault(); - event.stopPropagation(); // If there is more than one block selected, select the first // block so that focus is directed back to the beginning of the selection. diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 14139547ca947..d2dbdbfa04801 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -62,6 +62,9 @@ function bubbleEvents( doc ) { init.clientY += rect.top; } + // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event + // which would result in two React events being bubbled throught the iframe. + event.stopPropagation(); const newEvent = new Constructor( event.type, init ); const cancelled = ! frameElement.dispatchEvent( newEvent ); @@ -73,7 +76,7 @@ function bubbleEvents( doc ) { const eventTypes = [ 'dragover', 'mousemove', 'keydown' ]; for ( const name of eventTypes ) { - doc.addEventListener( name, bubbleEvent ); + doc.body.addEventListener( name, bubbleEvent ); } } diff --git a/packages/block-editor/src/components/writing-flow/use-tab-nav.js b/packages/block-editor/src/components/writing-flow/use-tab-nav.js index 0089488e21278..616da1bc75813 100644 --- a/packages/block-editor/src/components/writing-flow/use-tab-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-tab-nav.js @@ -89,7 +89,6 @@ export default function useTabNav() { if ( event.keyCode === ESCAPE && ! hasMultiSelection() ) { event.preventDefault(); - event.stopPropagation(); setNavigationMode( true ); return; } diff --git a/packages/components/src/higher-order/navigate-regions/index.tsx b/packages/components/src/higher-order/navigate-regions/index.tsx index 320f1f7d67908..83b473b1ba633 100644 --- a/packages/components/src/higher-order/navigate-regions/index.tsx +++ b/packages/components/src/higher-order/navigate-regions/index.tsx @@ -98,14 +98,12 @@ export function useNavigateRegions( shortcuts: Shortcuts = defaultShortcuts ) { return isKeyboardEvent[ modifier ]( event, character ); } ) ) { - event.stopPropagation(); focusRegion( -1 ); } else if ( shortcuts.next.some( ( { modifier, character } ) => { return isKeyboardEvent[ modifier ]( event, character ); } ) ) { - event.stopPropagation(); focusRegion( 1 ); } }, From 42015f02f7cb47ed2a0a3d2dea0a26b3316a5ca1 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 31 Aug 2023 17:10:37 +0100 Subject: [PATCH 12/18] Remove the event propagation --- packages/block-editor/src/components/iframe/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index d2dbdbfa04801..48eb367107830 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -64,7 +64,6 @@ function bubbleEvents( doc ) { // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event // which would result in two React events being bubbled throught the iframe. - event.stopPropagation(); const newEvent = new Constructor( event.type, init ); const cancelled = ! frameElement.dispatchEvent( newEvent ); From dde64472cfed2119d244ac653958b534df93cf35 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Thu, 31 Aug 2023 17:44:01 +0100 Subject: [PATCH 13/18] Fixes --- packages/block-editor/src/components/iframe/index.js | 1 + packages/block-editor/src/components/writing-flow/use-tab-nav.js | 1 + 2 files changed, 2 insertions(+) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 48eb367107830..d2dbdbfa04801 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -64,6 +64,7 @@ function bubbleEvents( doc ) { // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event // which would result in two React events being bubbled throught the iframe. + event.stopPropagation(); const newEvent = new Constructor( event.type, init ); const cancelled = ! frameElement.dispatchEvent( newEvent ); diff --git a/packages/block-editor/src/components/writing-flow/use-tab-nav.js b/packages/block-editor/src/components/writing-flow/use-tab-nav.js index 616da1bc75813..0089488e21278 100644 --- a/packages/block-editor/src/components/writing-flow/use-tab-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-tab-nav.js @@ -89,6 +89,7 @@ export default function useTabNav() { if ( event.keyCode === ESCAPE && ! hasMultiSelection() ) { event.preventDefault(); + event.stopPropagation(); setNavigationMode( true ); return; } From 7ff56bfcb734752a9896e8474153445726bac243 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 1 Sep 2023 10:07:09 +0100 Subject: [PATCH 14/18] Fix event listener --- .../src/components/iframe/index.js | 68 +++++++++++-------- 1 file changed, 38 insertions(+), 30 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index d2dbdbfa04801..32180642dc320 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -39,45 +39,53 @@ import { store as blockEditorStore } from '../../store'; * should be context dependent, e.g. actions on blocks like Cmd+A should not * work globally outside the block editor. * - * @param {Document} doc Document to attach listeners to. + * @param {Document} iframeDocument Document to attach listeners to. */ -function bubbleEvents( doc ) { - const { defaultView } = doc; - const { frameElement } = defaultView; +function useBubbleEvents( iframeDocument ) { + return useRefEffect( ( body ) => { + const { defaultView } = iframeDocument; + const { frameElement } = defaultView; - function bubbleEvent( event ) { - const prototype = Object.getPrototypeOf( event ); - const constructorName = prototype.constructor.name; - const Constructor = window[ constructorName ]; + function bubbleEvent( event ) { + const prototype = Object.getPrototypeOf( event ); + const constructorName = prototype.constructor.name; + const Constructor = window[ constructorName ]; - const init = {}; + const init = {}; - for ( const key in event ) { - init[ key ] = event[ key ]; - } + for ( const key in event ) { + init[ key ] = event[ key ]; + } - if ( event instanceof defaultView.MouseEvent ) { - const rect = frameElement.getBoundingClientRect(); - init.clientX += rect.left; - init.clientY += rect.top; - } + if ( event instanceof defaultView.MouseEvent ) { + const rect = frameElement.getBoundingClientRect(); + init.clientX += rect.left; + init.clientY += rect.top; + } - // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event - // which would result in two React events being bubbled throught the iframe. - event.stopPropagation(); - const newEvent = new Constructor( event.type, init ); - const cancelled = ! frameElement.dispatchEvent( newEvent ); + // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event + // which would result in two React events being bubbled throught the iframe. + event.stopPropagation(); + const newEvent = new Constructor( event.type, init ); + const cancelled = ! frameElement.dispatchEvent( newEvent ); - if ( cancelled ) { - event.preventDefault(); + if ( cancelled ) { + event.preventDefault(); + } } - } - const eventTypes = [ 'dragover', 'mousemove', 'keydown' ]; + const eventTypes = [ 'dragover', 'mousemove', 'keydown' ]; - for ( const name of eventTypes ) { - doc.body.addEventListener( name, bubbleEvent ); - } + for ( const name of eventTypes ) { + body.addEventListener( name, bubbleEvent ); + } + + return () => { + for ( const name of eventTypes ) { + body.removeEventListener( name, bubbleEvent ); + } + }; + } ); } function Iframe( { @@ -120,7 +128,6 @@ function Iframe( { const { documentElement } = contentDocument; iFrameDocument = contentDocument; - bubbleEvents( contentDocument ); clearerRef( documentElement ); // Ideally ALL classes that are added through get_body_class should @@ -185,6 +192,7 @@ function Iframe( { const disabledRef = useDisabled( { isDisabled: ! readonly } ); const bodyRef = useMergeRefs( [ + useBubbleEvents( iframeDocument ), contentRef, clearerRef, writingFlowRef, From f4193888ad2a28deea98261a875f323b13e8f646 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Fri, 1 Sep 2023 10:45:36 +0100 Subject: [PATCH 15/18] Change how we prevent bubbling iframe events --- packages/block-editor/src/components/iframe/index.js | 11 ++++++++--- .../src/higher-order/navigate-regions/index.tsx | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 32180642dc320..4ba715baf525a 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -63,9 +63,6 @@ function useBubbleEvents( iframeDocument ) { init.clientY += rect.top; } - // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event - // which would result in two React events being bubbled throught the iframe. - event.stopPropagation(); const newEvent = new Constructor( event.type, init ); const cancelled = ! frameElement.dispatchEvent( newEvent ); @@ -262,6 +259,9 @@ function Iframe( { > { iframeDocument && createPortal( + // We want to prevent React events from bubbling throught the iframe + // we bubble these manually. + /* eslint-disable-next-line jsx-a11y/no-noninteractive-element-interactions */ { + // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event + // which would result in two React events being bubbled throught the iframe. + event.stopPropagation(); + } } > { contentResizeListener } diff --git a/packages/components/src/higher-order/navigate-regions/index.tsx b/packages/components/src/higher-order/navigate-regions/index.tsx index 83b473b1ba633..f2b9c31bfbe95 100644 --- a/packages/components/src/higher-order/navigate-regions/index.tsx +++ b/packages/components/src/higher-order/navigate-regions/index.tsx @@ -70,6 +70,7 @@ export function useNavigateRegions( shortcuts: Shortcuts = defaultShortcuts ) { nextIndex = nextIndex === regions.length ? 0 : nextIndex; nextRegion = regions[ nextIndex ]; } + nextRegion.focus(); setIsFocusingRegions( true ); } From 39b2f746ed04e3d33e409883696314f9c5cbf841 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Mon, 4 Sep 2023 09:49:28 +0100 Subject: [PATCH 16/18] Fix more e2e tests --- .../src/components/iframe/index.js | 68 +++++++++++-------- .../components/src/autocomplete/index.tsx | 2 + 2 files changed, 40 insertions(+), 30 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 4ba715baf525a..4213141636760 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -31,6 +31,27 @@ import { useWritingFlow } from '../writing-flow'; import { useCompatibilityStyles } from './use-compatibility-styles'; import { store as blockEditorStore } from '../../store'; +function bubbleEvent( event, Constructor, frame ) { + const init = {}; + + for ( const key in event ) { + init[ key ] = event[ key ]; + } + + if ( event instanceof frame.ownerDocument.defaultView.MouseEvent ) { + const rect = frame.getBoundingClientRect(); + init.clientX += rect.left; + init.clientY += rect.top; + } + + const newEvent = new Constructor( event.type, init ); + const cancelled = ! frame.dispatchEvent( newEvent ); + + if ( cancelled ) { + event.preventDefault(); + } +} + /** * Bubbles some event types (keydown, keypress, and dragover) to parent document * document to ensure that the keyboard shortcuts and drag and drop work. @@ -45,41 +66,21 @@ function useBubbleEvents( iframeDocument ) { return useRefEffect( ( body ) => { const { defaultView } = iframeDocument; const { frameElement } = defaultView; - - function bubbleEvent( event ) { - const prototype = Object.getPrototypeOf( event ); - const constructorName = prototype.constructor.name; - const Constructor = window[ constructorName ]; - - const init = {}; - - for ( const key in event ) { - init[ key ] = event[ key ]; - } - - if ( event instanceof defaultView.MouseEvent ) { - const rect = frameElement.getBoundingClientRect(); - init.clientX += rect.left; - init.clientY += rect.top; - } - - const newEvent = new Constructor( event.type, init ); - const cancelled = ! frameElement.dispatchEvent( newEvent ); - - if ( cancelled ) { - event.preventDefault(); - } - } - - const eventTypes = [ 'dragover', 'mousemove', 'keydown' ]; - + const eventTypes = [ 'dragover', 'mousemove' ]; + const handlers = {}; for ( const name of eventTypes ) { - body.addEventListener( name, bubbleEvent ); + handlers[ name ] = ( event ) => { + const prototype = Object.getPrototypeOf( event ); + const constructorName = prototype.constructor.name; + const Constructor = window[ constructorName ]; + bubbleEvent( event, Constructor, frameElement ); + }; + body.addEventListener( name, handlers[ name ] ); } return () => { for ( const name of eventTypes ) { - body.removeEventListener( name, bubbleEvent ); + body.removeEventListener( name, handlers[ name ] ); } }; } ); @@ -273,6 +274,13 @@ function Iframe( { // This stopPropagation call ensures React doesn't create a syncthetic event to bubble this event // which would result in two React events being bubbled throught the iframe. event.stopPropagation(); + const { defaultView } = iframeDocument; + const { frameElement } = defaultView; + bubbleEvent( + event, + window.KeyboardEvent, + frameElement + ); } } > { contentResizeListener } diff --git a/packages/components/src/autocomplete/index.tsx b/packages/components/src/autocomplete/index.tsx index 7825526fe34a5..e0850bdf707c3 100644 --- a/packages/components/src/autocomplete/index.tsx +++ b/packages/components/src/autocomplete/index.tsx @@ -182,6 +182,8 @@ export function useAutocomplete( { setAutocompleter( null ); setAutocompleterUI( null ); event.preventDefault(); + // This prevents the block editor from handling the escape key to unselect the block. + event.stopPropagation(); break; case 'Enter': From 95e360789449a1bbc8f8e0522bfc7119515313e7 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Mon, 4 Sep 2023 10:27:31 +0100 Subject: [PATCH 17/18] Escape command shouldn't trigger when dialogs are open --- packages/compose/src/hooks/use-dialog/index.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/compose/src/hooks/use-dialog/index.ts b/packages/compose/src/hooks/use-dialog/index.ts index dd7e47422eb56..d822e264763e8 100644 --- a/packages/compose/src/hooks/use-dialog/index.ts +++ b/packages/compose/src/hooks/use-dialog/index.ts @@ -76,6 +76,7 @@ function useDialog( options: DialogOptions ): useDialogReturn { currentOptions.current?.onClose ) { event.preventDefault(); + event.stopPropagation(); currentOptions.current.onClose(); } } ); From cdc70bd0b511fd463be55c42762edb0ae2dbe604 Mon Sep 17 00:00:00 2001 From: Riad Benguella Date: Mon, 4 Sep 2023 15:07:28 +0100 Subject: [PATCH 18/18] Retain defaultPrevented value --- packages/block-editor/src/components/iframe/index.js | 3 +++ .../block-editor/src/components/writing-flow/use-tab-nav.js | 1 - packages/components/src/autocomplete/index.tsx | 2 -- packages/compose/src/hooks/use-dialog/index.ts | 1 - 4 files changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/block-editor/src/components/iframe/index.js b/packages/block-editor/src/components/iframe/index.js index 4213141636760..67b01a4963b0c 100644 --- a/packages/block-editor/src/components/iframe/index.js +++ b/packages/block-editor/src/components/iframe/index.js @@ -45,6 +45,9 @@ function bubbleEvent( event, Constructor, frame ) { } const newEvent = new Constructor( event.type, init ); + if ( init.defaultPrevented ) { + newEvent.preventDefault(); + } const cancelled = ! frame.dispatchEvent( newEvent ); if ( cancelled ) { diff --git a/packages/block-editor/src/components/writing-flow/use-tab-nav.js b/packages/block-editor/src/components/writing-flow/use-tab-nav.js index 0089488e21278..616da1bc75813 100644 --- a/packages/block-editor/src/components/writing-flow/use-tab-nav.js +++ b/packages/block-editor/src/components/writing-flow/use-tab-nav.js @@ -89,7 +89,6 @@ export default function useTabNav() { if ( event.keyCode === ESCAPE && ! hasMultiSelection() ) { event.preventDefault(); - event.stopPropagation(); setNavigationMode( true ); return; } diff --git a/packages/components/src/autocomplete/index.tsx b/packages/components/src/autocomplete/index.tsx index e0850bdf707c3..7825526fe34a5 100644 --- a/packages/components/src/autocomplete/index.tsx +++ b/packages/components/src/autocomplete/index.tsx @@ -182,8 +182,6 @@ export function useAutocomplete( { setAutocompleter( null ); setAutocompleterUI( null ); event.preventDefault(); - // This prevents the block editor from handling the escape key to unselect the block. - event.stopPropagation(); break; case 'Enter': diff --git a/packages/compose/src/hooks/use-dialog/index.ts b/packages/compose/src/hooks/use-dialog/index.ts index d822e264763e8..dd7e47422eb56 100644 --- a/packages/compose/src/hooks/use-dialog/index.ts +++ b/packages/compose/src/hooks/use-dialog/index.ts @@ -76,7 +76,6 @@ function useDialog( options: DialogOptions ): useDialogReturn { currentOptions.current?.onClose ) { event.preventDefault(); - event.stopPropagation(); currentOptions.current.onClose(); } } );