Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Block Editor: Consider events only from DOM descendents (WritingFlow, ObserveTyping) #19879

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 2 additions & 16 deletions packages/block-editor/src/components/inserter/menu.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand All @@ -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 (
<div
className={ classnames( 'block-editor-inserter__menu', {
'has-help-panel': hasHelpPanel,
} ) }
onKeyPress={ stopKeyPropagation }
onKeyDown={ this.onKeyDown }
>
<div className="block-editor-inserter__main-area">
<label htmlFor={ `block-editor-inserter__search-${ instanceId }` } className="screen-reader-text">
Expand Down Expand Up @@ -438,7 +424,7 @@ export class InserterMenu extends Component {
) }
</div>
);
/* eslint-enable jsx-a11y/no-autofocus, jsx-a11y/no-static-element-interactions */
/* eslint-enable jsx-a11y/no-autofocus */
}
}

Expand Down
33 changes: 0 additions & 33 deletions packages/block-editor/src/components/link-control/search-input.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 ) {

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Huh

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷‍♂ #19458 (comment)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I noticed this and removed it in #19775.

Not sure how it happened. Sorry!

}
};

const LinkControlSearchInput = ( {
value,
onChange,
Expand Down Expand Up @@ -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 }
Expand Down
22 changes: 1 addition & 21 deletions packages/block-editor/src/components/media-replace-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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 );
Expand Down Expand Up @@ -99,8 +81,6 @@ const MediaReplaceFlow = (
if ( showEditURLInput ) {
urlInputUIContent = (
<LinkEditor
onKeyDown={ stopPropagationRelevantKeys }
onKeyPress={ stopPropagation }
value={ mediaURLValue }
isFullWidthInput={ true }
hasInputBorder={ true }
Expand Down
35 changes: 29 additions & 6 deletions packages/block-editor/src/components/observe-typing/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*
Expand Down Expand Up @@ -53,6 +55,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 ( event.currentTarget.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.
Expand Down Expand Up @@ -107,18 +130,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
Expand All @@ -136,14 +159,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
Expand All @@ -154,7 +177,7 @@ function ObserveTyping( {
stopTyping();
}
} );
}
} );

// Disable reason: This component is responsible for capturing bubbled
// keyboard events which are interpreted as typing intent.
Expand Down
7 changes: 0 additions & 7 deletions packages/block-editor/src/components/url-input/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,6 @@ import { BaseControl, Button, Spinner, withSpokenMessages, Popover } from '@word
import { withInstanceId, withSafeTimeout, compose } from '@wordpress/compose';
import { withSelect } from '@wordpress/data';
import { isURL } from '@wordpress/url';

// Since URLInput is rendered in the context of other inputs, but should be
// considered a separate modal node, prevent keyboard events from propagating
// as being considered from the input.
const stopEventPropagation = ( event ) => event.stopPropagation();

class URLInput extends Component {
constructor( props ) {
super( props );
Expand Down Expand Up @@ -346,7 +340,6 @@ class URLInput extends Component {
required
value={ value }
onChange={ this.onChange }
onInput={ stopEventPropagation }
placeholder={ placeholder }
onKeyDown={ this.onKeyDown }
role="combobox"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,6 @@ import {
SVG,
Path,
} from '@wordpress/components';
import {
LEFT,
RIGHT,
UP,
DOWN,
BACKSPACE,
ENTER,
} from '@wordpress/keycodes';

/**
* Internal dependencies
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -223,14 +204,10 @@ const ImageURLInputUI = ( {
label={ __( 'Link Rel' ) }
value={ removeNewTabRel( rel ) || '' }
onChange={ onSetLinkRel }
onKeyPress={ stopPropagation }
onKeyDown={ stopPropagationRelevantKeys }
/>
<TextControl
label={ __( 'Link CSS Class' ) }
value={ linkClass || '' }
onKeyPress={ stopPropagation }
onKeyDown={ stopPropagationRelevantKeys }
onChange={ onSetLinkClass }
/>
</>
Expand Down Expand Up @@ -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 }
/>
Expand All @@ -289,7 +264,6 @@ const ImageURLInputUI = ( {
<>
<URLPopover.LinkViewer
className="block-editor-format-toolbar__link-container-content"
onKeyPress={ stopPropagation }
url={ url }
onEditLinkClick={ startEditLink }
urlLabel={ urlLabel }
Expand Down
13 changes: 13 additions & 0 deletions packages/block-editor/src/components/writing-flow/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
13 changes: 0 additions & 13 deletions packages/format-library/src/image/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,15 +6,12 @@ import { __ } from '@wordpress/i18n';
import { Component } from '@wordpress/element';
import { insertObject } from '@wordpress/rich-text';
import { MediaUpload, RichTextToolbarButton, MediaUploadCheck } from '@wordpress/block-editor';
import { LEFT, RIGHT, UP, DOWN, BACKSPACE, ENTER } from '@wordpress/keycodes';

const ALLOWED_MEDIA_TYPES = [ 'image' ];

const name = 'core/image';
const title = __( 'Inline image' );

const stopKeyPropagation = ( event ) => event.stopPropagation();

function getRange() {
const selection = window.getSelection();
return selection.rangeCount ? selection.getRangeAt( 0 ) : null;
Expand All @@ -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 = {
Expand Down Expand Up @@ -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 } );
}
Expand Down Expand Up @@ -126,8 +115,6 @@ export const image = {
/* eslint-disable jsx-a11y/no-noninteractive-element-interactions */ }
<form
className="block-editor-format-toolbar__image-container-content"
onKeyPress={ stopKeyPropagation }
onKeyDown={ this.onKeyDown }
onSubmit={ ( event ) => {
const newReplacements = value.replacements.slice();

Expand Down
Loading