From 791f5d698da4a7d20f1fb447d8043c85d3816cc8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 24 Jan 2020 15:29:42 -0500 Subject: [PATCH 1/4] Block Editor: WritingFlow: Consider key events only from DOM descendents --- .../src/components/writing-flow/index.js | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/packages/block-editor/src/components/writing-flow/index.js b/packages/block-editor/src/components/writing-flow/index.js index 475be097c9132..aa8ddad199ed1 100644 --- a/packages/block-editor/src/components/writing-flow/index.js +++ b/packages/block-editor/src/components/writing-flow/index.js @@ -308,6 +308,19 @@ export default function WritingFlow( { children } ) { function onKeyDown( event ) { const { keyCode, target } = event; + + // Handle only if the event occurred within the same DOM hierarchy as + // the rendered container. This is used to distinguish between events + // which bubble through React's virtual event system from those which + // strictly occur in the DOM created by the component. + // + // The implication here is: If it's not desirable for a bubbled event to + // be considered by WritingFlow, it can be avoided by rendering to a + // distinct place in the DOM (e.g. using Slot/Fill). + if ( ! container.current.contains( target ) ) { + return; + } + const isUp = keyCode === UP; const isDown = keyCode === DOWN; const isLeft = keyCode === LEFT; From ab1b8643d9dfcb28b8c0078935fa9534c4352cf8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 24 Jan 2020 15:30:14 -0500 Subject: [PATCH 2/4] Block Editor: ObserveTyping: Consider events only as DOM descendents --- .../src/components/observe-typing/index.js | 37 ++++++++++++++++--- 1 file changed, 31 insertions(+), 6 deletions(-) diff --git a/packages/block-editor/src/components/observe-typing/index.js b/packages/block-editor/src/components/observe-typing/index.js index 0e9c176e251f2..c3ca2db40d478 100644 --- a/packages/block-editor/src/components/observe-typing/index.js +++ b/packages/block-editor/src/components/observe-typing/index.js @@ -20,6 +20,8 @@ import { } from '@wordpress/keycodes'; import { withSafeTimeout } from '@wordpress/compose'; +/** @typedef {import('@wordpress/element').WPSyntheticEvent} WPSyntheticEvent */ + /** * Set of key codes upon which typing is to be initiated on a keydown event. * @@ -45,6 +47,7 @@ function ObserveTyping( { children, setTimeout: setSafeTimeout, } ) { + const node = useRef(); const lastMouseMove = useRef(); const isTyping = useSelect( ( select ) => select( 'core/block-editor' ).isTyping() ); const { startTyping, stopTyping } = useDispatch( 'core/block-editor' ); @@ -53,6 +56,27 @@ function ObserveTyping( { return () => toggleEventBindings( false ); }, [ isTyping ] ); + /** + * Higher-order function which, given an event handler, calls the event + * handler only if the event occurred within the same DOM hierarchy as the + * rendered element. This is used to distinguish between events which bubble + * through React's virtual event system from those which strictly occur in + * the DOM created by the component. + * + * The implication here is: If it's not desirable for a bubbled event to be + * considered by ObserveTyping, it's possible to avoid this by rendering to + * a distinct place in the DOM (e.g. using Slot/Fill). + * + * @param {(event:WPSyntheticEvent)=>void} handler Original event handler. + * + * @return {(event:WPSyntheticEvent)=>void} Enhanced event handler. + */ + const ifDOMDescendentEventTarget = ( handler ) => ( event ) => { + if ( node.current.contains( event.target ) ) { + handler( event ); + } + }; + /** * Bind or unbind events to the document when typing has started or stopped * respectively, or when component has become unmounted. @@ -107,18 +131,18 @@ function ObserveTyping( { * * @param {KeyboardEvent} event Keypress or keydown event to interpret. */ - function stopTypingOnEscapeKey( event ) { + const stopTypingOnEscapeKey = ifDOMDescendentEventTarget( ( event ) => { if ( isTyping && event.keyCode === ESCAPE ) { stopTyping(); } - } + } ); /** * Handles a keypress or keydown event to infer intention to start typing. * * @param {KeyboardEvent} event Keypress or keydown event to interpret. */ - function startTypingInTextField( event ) { + const startTypingInTextField = ifDOMDescendentEventTarget( ( event ) => { const { type, target } = event; // Abort early if already typing, or key press is incurred outside a @@ -136,14 +160,14 @@ function ObserveTyping( { } startTyping(); - } + } ); /** * Stops typing when focus transitions to a non-text field element. * * @param {FocusEvent} event Focus event. */ - function stopTypingOnNonTextField( event ) { + const stopTypingOnNonTextField = ifDOMDescendentEventTarget( ( event ) => { const { target } = event; // Since focus to a non-text field via arrow key will trigger before @@ -154,7 +178,7 @@ function ObserveTyping( { stopTyping(); } } ); - } + } ); // Disable reason: This component is responsible for capturing bubbled // keyboard events which are interpreted as typing intent. @@ -162,6 +186,7 @@ function ObserveTyping( { /* eslint-disable jsx-a11y/no-static-element-interactions */ return (
Date: Fri, 24 Jan 2020 15:36:18 -0500 Subject: [PATCH 3/4] Components: Remove redundant stopPropagation for WritingFlow, ObserveTyping Originally necessary to avoid events being interprted by ObserveTyping, WritingFlow. These components now explicitly consider whether the event occurred in the same DOM hierarchy, where the elements rendered here would presumably be rendered outside (Popover, Slot/Fill, etc). --- .../src/components/inserter/menu.js | 18 ++-------- .../components/link-control/search-input.js | 33 ------------------- .../components/media-replace-flow/index.js | 22 +------------ .../src/components/url-input/index.js | 7 ---- .../url-popover/image-url-input-ui.js | 26 --------------- packages/format-library/src/image/index.js | 13 -------- packages/format-library/src/link/inline.js | 14 -------- 7 files changed, 3 insertions(+), 130 deletions(-) diff --git a/packages/block-editor/src/components/inserter/menu.js b/packages/block-editor/src/components/inserter/menu.js index 45936b20db2e5..0f901e2f0e6f6 100644 --- a/packages/block-editor/src/components/inserter/menu.js +++ b/packages/block-editor/src/components/inserter/menu.js @@ -39,7 +39,6 @@ import { } from '@wordpress/blocks'; import { withDispatch, withSelect } from '@wordpress/data'; import { withInstanceId, compose, withSafeTimeout } from '@wordpress/compose'; -import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes'; import { addQueryArgs } from '@wordpress/url'; /** @@ -54,8 +53,6 @@ import { searchItems } from './search-items'; const MAX_SUGGESTED_ITEMS = 9; -const stopKeyPropagation = ( event ) => event.stopPropagation(); - const getBlockNamespace = ( item ) => item.name.split( '/' )[ 0 ]; export class InserterMenu extends Component { @@ -220,13 +217,6 @@ export class InserterMenu extends Component { debouncedSpeak( resultsFoundMessage ); } - onKeyDown( event ) { - if ( includes( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ], event.keyCode ) ) { - // Stop the key event from propagating up to ObserveTyping.startTypingInTextField. - event.stopPropagation(); - } - } - render() { const { categories, collections, instanceId, onSelect, rootClientId, showInserterHelpPanel } = this.props; const { @@ -247,16 +237,12 @@ export class InserterMenu extends Component { // Disable reason (no-autofocus): The inserter menu is a modal display, not one which // is always visible, and one which already incurs this behavior of autoFocus via // Popover's focusOnMount. - // Disable reason (no-static-element-interactions): Navigational key-presses within - // the menu are prevented from triggering WritingFlow and ObserveTyping interactions. - /* eslint-disable jsx-a11y/no-autofocus, jsx-a11y/no-static-element-interactions */ + /* eslint-disable jsx-a11y/no-autofocus */ return (
); - /* eslint-enable jsx-a11y/no-autofocus, jsx-a11y/no-static-element-interactions */ + /* eslint-enable jsx-a11y/no-autofocus */ } } diff --git a/packages/block-editor/src/components/link-control/search-input.js b/packages/block-editor/src/components/link-control/search-input.js index 4bda15f1f1eb7..18812351cf736 100644 --- a/packages/block-editor/src/components/link-control/search-input.js +++ b/packages/block-editor/src/components/link-control/search-input.js @@ -4,38 +4,12 @@ import { useState } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; import { Button } from '@wordpress/components'; -import { LEFT, - RIGHT, - UP, - DOWN, - BACKSPACE, - ENTER, -} from '@wordpress/keycodes'; /** * Internal dependencies */ import { URLInput } from '../'; -const handleLinkControlOnKeyDown = ( event ) => { - const { keyCode } = event; - - if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( keyCode ) > -1 ) { - // Stop the key event from propagating up to ObserveTyping.startTypingInTextField. - event.stopPropagation(); - } -}; - -const handleLinkControlOnKeyPress = ( event ) => { - const { keyCode } = event; - - event.stopPropagation(); - - if ( keyCode === ENTER ) { - - } -}; - const LinkControlSearchInput = ( { value, onChange, @@ -67,13 +41,6 @@ const LinkControlSearchInput = ( { className="block-editor-link-control__search-input" value={ value } onChange={ selectItemHandler } - onKeyDown={ ( event ) => { - if ( event.keyCode === ENTER ) { - return; - } - handleLinkControlOnKeyDown( event ); - } } - onKeyPress={ handleLinkControlOnKeyPress } placeholder={ __( 'Search or type url' ) } __experimentalRenderSuggestions={ renderSuggestions } __experimentalFetchLinkSuggestions={ fetchSuggestions } diff --git a/packages/block-editor/src/components/media-replace-flow/index.js b/packages/block-editor/src/components/media-replace-flow/index.js index 8e2c09567d018..b810da686e5dc 100644 --- a/packages/block-editor/src/components/media-replace-flow/index.js +++ b/packages/block-editor/src/components/media-replace-flow/index.js @@ -13,14 +13,7 @@ import { Dropdown, withNotices, } from '@wordpress/components'; -import { - LEFT, - RIGHT, - UP, - DOWN, - BACKSPACE, - ENTER, -} from '@wordpress/keycodes'; +import { DOWN } from '@wordpress/keycodes'; import { useSelect } from '@wordpress/data'; import { compose } from '@wordpress/compose'; @@ -51,17 +44,6 @@ const MediaReplaceFlow = ( }, [] ); const editMediaButtonRef = createRef(); - const stopPropagation = ( event ) => { - event.stopPropagation(); - }; - - const stopPropagationRelevantKeys = ( event ) => { - if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( event.keyCode ) > -1 ) { - // Stop the key event from propagating up to ObserveTyping.startTypingInTextField. - event.stopPropagation(); - } - }; - const selectMedia = ( media ) => { onSelect( media ); setMediaURLValue( media.url ); @@ -99,8 +81,6 @@ const MediaReplaceFlow = ( if ( showEditURLInput ) { urlInputUIContent = ( event.stopPropagation(); - class URLInput extends Component { constructor( props ) { super( props ); @@ -346,7 +340,6 @@ class URLInput extends Component { required value={ value } onChange={ this.onChange } - onInput={ stopEventPropagation } placeholder={ placeholder } onKeyDown={ this.onKeyDown } role="combobox" diff --git a/packages/block-editor/src/components/url-popover/image-url-input-ui.js b/packages/block-editor/src/components/url-popover/image-url-input-ui.js index f8fabb072b38c..60fc882caffc6 100644 --- a/packages/block-editor/src/components/url-popover/image-url-input-ui.js +++ b/packages/block-editor/src/components/url-popover/image-url-input-ui.js @@ -17,14 +17,6 @@ import { SVG, Path, } from '@wordpress/components'; -import { - LEFT, - RIGHT, - UP, - DOWN, - BACKSPACE, - ENTER, -} from '@wordpress/keycodes'; /** * Internal dependencies @@ -60,17 +52,6 @@ const ImageURLInputUI = ( { const autocompleteRef = useRef( null ); - const stopPropagation = ( event ) => { - event.stopPropagation(); - }; - - const stopPropagationRelevantKeys = ( event ) => { - if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( event.keyCode ) > -1 ) { - // Stop the key event from propagating up to ObserveTyping.startTypingInTextField. - event.stopPropagation(); - } - }; - const startEditLink = useCallback( () => { if ( linkDestination === LINK_DESTINATION_MEDIA || linkDestination === LINK_DESTINATION_ATTACHMENT @@ -223,14 +204,10 @@ const ImageURLInputUI = ( { label={ __( 'Link Rel' ) } value={ removeNewTabRel( rel ) || '' } onChange={ onSetLinkRel } - onKeyPress={ stopPropagation } - onKeyDown={ stopPropagationRelevantKeys } /> @@ -279,8 +256,6 @@ const ImageURLInputUI = ( { className="block-editor-format-toolbar__link-container-content" value={ linkEditorValue } onChangeInputValue={ setUrlInput } - onKeyDown={ stopPropagationRelevantKeys } - onKeyPress={ stopPropagation } onSubmit={ onSubmitLinkChange() } autocompleteRef={ autocompleteRef } /> @@ -289,7 +264,6 @@ const ImageURLInputUI = ( { <> event.stopPropagation(); - function getRange() { const selection = window.getSelection(); return selection.rangeCount ? selection.getRangeAt( 0 ) : null; @@ -37,7 +34,6 @@ export const image = { constructor() { super( ...arguments ); this.onChange = this.onChange.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); this.openModal = this.openModal.bind( this ); this.closeModal = this.closeModal.bind( this ); this.state = { @@ -69,13 +65,6 @@ export const image = { this.setState( { width } ); } - onKeyDown( event ) { - if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( event.keyCode ) > -1 ) { - // Stop the key event from propagating up to ObserveTyping.startTypingInTextField. - event.stopPropagation(); - } - } - openModal() { this.setState( { modal: true } ); } @@ -126,8 +115,6 @@ export const image = { /* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ }
{ const newReplacements = value.replacements.slice(); diff --git a/packages/format-library/src/link/inline.js b/packages/format-library/src/link/inline.js index 72e075255aa04..8d290c8dccec4 100644 --- a/packages/format-library/src/link/inline.js +++ b/packages/format-library/src/link/inline.js @@ -7,7 +7,6 @@ import { ToggleControl, withSpokenMessages, } from '@wordpress/components'; -import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes'; import { prependHTTP } from '@wordpress/url'; import { create, @@ -24,8 +23,6 @@ import { URLPopover } from '@wordpress/block-editor'; */ import { createLinkFormat, isValidHref } from './utils'; -const stopKeyPropagation = ( event ) => event.stopPropagation(); - function isShowingInput( props, state ) { return props.addingLink || state.editLink; } @@ -69,7 +66,6 @@ class InlineLinkUI extends Component { this.editLink = this.editLink.bind( this ); this.submitLink = this.submitLink.bind( this ); - this.onKeyDown = this.onKeyDown.bind( this ); this.onChangeInputValue = this.onChangeInputValue.bind( this ); this.setLinkTarget = this.setLinkTarget.bind( this ); this.onFocusOutside = this.onFocusOutside.bind( this ); @@ -101,13 +97,6 @@ class InlineLinkUI extends Component { return null; } - onKeyDown( event ) { - if ( [ LEFT, DOWN, RIGHT, UP, BACKSPACE, ENTER ].indexOf( event.keyCode ) > -1 ) { - // Stop the key event from propagating up to ObserveTyping.startTypingInTextField. - event.stopPropagation(); - } - } - onChangeInputValue( inputValue ) { this.setState( { inputValue } ); } @@ -216,15 +205,12 @@ class InlineLinkUI extends Component { className="block-editor-format-toolbar__link-container-content" value={ inputValue } onChangeInputValue={ this.onChangeInputValue } - onKeyDown={ this.onKeyDown } - onKeyPress={ stopKeyPropagation } onSubmit={ this.submitLink } autocompleteRef={ this.autocompleteRef } /> ) : ( Date: Mon, 27 Jan 2020 13:28:21 -0500 Subject: [PATCH 4/4] Block Editor: Use Event#currentTarget to detect contains --- packages/block-editor/src/components/observe-typing/index.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/block-editor/src/components/observe-typing/index.js b/packages/block-editor/src/components/observe-typing/index.js index c3ca2db40d478..dcec1ba2eec90 100644 --- a/packages/block-editor/src/components/observe-typing/index.js +++ b/packages/block-editor/src/components/observe-typing/index.js @@ -47,7 +47,6 @@ function ObserveTyping( { children, setTimeout: setSafeTimeout, } ) { - const node = useRef(); const lastMouseMove = useRef(); const isTyping = useSelect( ( select ) => select( 'core/block-editor' ).isTyping() ); const { startTyping, stopTyping } = useDispatch( 'core/block-editor' ); @@ -72,7 +71,7 @@ function ObserveTyping( { * @return {(event:WPSyntheticEvent)=>void} Enhanced event handler. */ const ifDOMDescendentEventTarget = ( handler ) => ( event ) => { - if ( node.current.contains( event.target ) ) { + if ( event.currentTarget.contains( event.target ) ) { handler( event ); } }; @@ -186,7 +185,6 @@ function ObserveTyping( { /* eslint-disable jsx-a11y/no-static-element-interactions */ return (