From 7fd6bc40b0baa6fb0c20c9e2b0a5b0710cd2e491 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 11 Oct 2018 13:21:57 -0400 Subject: [PATCH] Editor: Move block focus restore to Block Toolbar --- .../src/components/block-toolbar/index.js | 94 +++++++++++++--- .../src/components/navigable-toolbar/index.js | 101 +++--------------- test/e2e/specs/block-toolbar.test.js | 58 ++++++++++ 3 files changed, 148 insertions(+), 105 deletions(-) create mode 100644 test/e2e/specs/block-toolbar.test.js diff --git a/packages/editor/src/components/block-toolbar/index.js b/packages/editor/src/components/block-toolbar/index.js index de34bf2747ec25..561aa3fe5f7978 100644 --- a/packages/editor/src/components/block-toolbar/index.js +++ b/packages/editor/src/components/block-toolbar/index.js @@ -1,9 +1,16 @@ +/** + * External dependencies + */ +import { cond, matchesProperty } from 'lodash'; + /** * WordPress Dependencies */ import { withSelect } from '@wordpress/data'; import { Component, createRef, Fragment } from '@wordpress/element'; import { focus } from '@wordpress/dom'; +import { KeyboardShortcuts } from '@wordpress/components'; +import { ESCAPE } from '@wordpress/keycodes'; /** * Internal Dependencies @@ -17,7 +24,15 @@ import BlockSettingsMenu from '../block-settings-menu'; class BlockToolbar extends Component { constructor() { super( ...arguments ); + this.container = createRef(); + + this.focusContainer = this.focusContainer.bind( this ); + this.restoreFocus = this.restoreFocus.bind( this ); + this.resetActiveElementBeforeFocus = this.resetActiveElementBeforeFocus.bind( this ); + this.switchOnKeyDown = cond( [ + [ matchesProperty( [ 'keyCode' ], ESCAPE ), this.restoreFocus ], + ] ); } componentDidMount() { @@ -35,41 +50,88 @@ class BlockToolbar extends Component { } } + /** + * Shifts focus to the first tabbable element within the toolbar container, + * if one exists. + */ focusContainer() { const tabbables = focus.tabbable.find( this.container.current ); - if ( tabbables.length ) { - tabbables[ 0 ].focus(); + if ( ! tabbables.length ) { + return; } + + // Track the original active element prior to shifting focus, so that + // focus can be returned if the user presses Escape while in toolbar. + this.activeElementBeforeFocus = document.activeElement; + + tabbables[ 0 ].focus(); + } + + /** + * Restores focus to the active element at the time focus was + * programattically shifted to the toolbar, if one exists. + */ + restoreFocus() { + if ( this.activeElementBeforeFocus ) { + this.activeElementBeforeFocus.focus(); + this.resetActiveElementBeforeFocus(); + } + } + + /** + * Clears the assigned active element which would be otherwise used in + * restoreFocus. + */ + resetActiveElementBeforeFocus() { + delete this.activeElementBeforeFocus; } render() { const { blockClientIds, isValid, mode } = this.props; - if ( blockClientIds.length === 0 ) { + if ( ! blockClientIds.length ) { return null; } + let controls; if ( blockClientIds.length > 1 ) { - return ( -
- - -
+ controls = ; + } else if ( mode === 'visual' && isValid ) { + controls = ( + + + + + ); } + // Disable reason: The div is not intended to be interactable, but it + // observes bubbled keypresses from its interactable children to infer + // blur intent. + + /* eslint-disable jsx-a11y/no-static-element-interactions */ return ( -
- { mode === 'visual' && isValid && ( - - - - - - ) } +
+ + { controls }
); + /* eslint-disable jsx-a11y/no-static-element-interactions */ } } diff --git a/packages/editor/src/components/navigable-toolbar/index.js b/packages/editor/src/components/navigable-toolbar/index.js index f631f17b3c8301..2a8bccc21e1b03 100644 --- a/packages/editor/src/components/navigable-toolbar/index.js +++ b/packages/editor/src/components/navigable-toolbar/index.js @@ -1,95 +1,18 @@ -/** - * External dependencies - */ -import { cond, matchesProperty } from 'lodash'; - /** * WordPress dependencies */ -import { NavigableMenu, KeyboardShortcuts } from '@wordpress/components'; -import { Component, findDOMNode } from '@wordpress/element'; -import { focus } from '@wordpress/dom'; -import { ESCAPE } from '@wordpress/keycodes'; - -/** - * Browser dependencies - */ - -const { Node, getSelection } = window; - -class NavigableToolbar extends Component { - constructor() { - super( ...arguments ); - - this.bindNode = this.bindNode.bind( this ); - this.focusToolbar = this.focusToolbar.bind( this ); - this.focusSelection = this.focusSelection.bind( this ); - - this.switchOnKeyDown = cond( [ - [ matchesProperty( [ 'keyCode' ], ESCAPE ), this.focusSelection ], - ] ); - } - - bindNode( ref ) { - // Disable reason: Need DOM node for finding first focusable element - // on keyboard interaction to shift to toolbar. - // eslint-disable-next-line react/no-find-dom-node - this.toolbar = findDOMNode( ref ); - } - - focusToolbar() { - const tabbables = focus.tabbable.find( this.toolbar ); - if ( tabbables.length ) { - tabbables[ 0 ].focus(); - } - } - - /** - * Programmatically shifts focus to the element where the current selection - * exists, if there is a selection. - */ - focusSelection() { - // Ensure that a selection exists. - const selection = getSelection(); - if ( ! selection ) { - return; - } - - // Focus node may be a text node, which cannot be focused directly. - // Find its parent element instead. - const { focusNode } = selection; - let focusElement = focusNode; - if ( focusElement.nodeType !== Node.ELEMENT_NODE ) { - focusElement = focusElement.parentElement; - } - - if ( focusElement ) { - focusElement.focus(); - } - } - - render() { - const { children, ...props } = this.props; - return ( - - - { children } - - ); - } +import { NavigableMenu } from '@wordpress/components'; + +function NavigableToolbar( { children, ...props } ) { + return ( + + { children } + + ); } export default NavigableToolbar; diff --git a/test/e2e/specs/block-toolbar.test.js b/test/e2e/specs/block-toolbar.test.js new file mode 100644 index 00000000000000..1d73c703401508 --- /dev/null +++ b/test/e2e/specs/block-toolbar.test.js @@ -0,0 +1,58 @@ +/** + * External dependencies + */ +import { forEach } from 'lodash'; + +/** + * Internal dependencies + */ +import { + newPost, + clickBlockAppender, + pressWithModifier, +} from '../support/utils'; + +describe( 'block toolbar', () => { + forEach( { + unified: true, + 'not unified': false, + }, ( isUnifiedToolbar, label ) => { + beforeEach( async () => { + await newPost(); + + await page.evaluate( ( _isUnifiedToolbar ) => { + const { select, dispatch } = wp.data; + const isCurrentlyUnified = select( 'core/edit-post' ).isFeatureActive( 'fixedToolbar' ); + if ( isCurrentlyUnified !== _isUnifiedToolbar ) { + dispatch( 'core/edit-post' ).toggleFeature( 'fixedToolbar' ); + } + }, isUnifiedToolbar ); + } ); + + describe( label, () => { + it( 'navigates in and out of toolbar by keyboard (Alt+F10, Escape)', async () => { + await clickBlockAppender(); + + // [TEMPORARY/BUG]: Unless in unified toolbar mode, the toolbar + // does not appear until the user types some text and changes + // the selection. + await page.keyboard.type( 'a' ); + await pressWithModifier( 'Shift', 'ArrowLeft' ); + + await pressWithModifier( 'Alt', 'F10' ); + const isInToolbar = await page.evaluate( () => ( + !! document.activeElement.closest( '.editor-block-toolbar' ) + ) ); + + expect( isInToolbar ).toBe( true ); + + await page.keyboard.press( 'Escape' ); + const isInBlockEdit = await page.evaluate( () => ( + !! document.activeElement.closest( '.editor-block-list__block-edit' ) + ) ); + + expect( isInBlockEdit ).toBe( true ); + } ); + } ); + } ); +} );