From b3b764d87fb31492f62b38d809d921478185b843 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 14 Mar 2019 16:33:15 -0400 Subject: [PATCH 01/11] Components: Add onFocusLoss option to withFocusReturn --- .../higher-order/with-focus-return/index.js | 81 ++++++++++++++++--- packages/components/src/index.js | 2 +- 2 files changed, 69 insertions(+), 14 deletions(-) diff --git a/packages/components/src/higher-order/with-focus-return/index.js b/packages/components/src/higher-order/with-focus-return/index.js index ae77932d135485..b9ba6010732ba9 100644 --- a/packages/components/src/higher-order/with-focus-return/index.js +++ b/packages/components/src/higher-order/with-focus-return/index.js @@ -1,22 +1,65 @@ /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; +import { Component, createContext } from '@wordpress/element'; import { createHigherOrderComponent } from '@wordpress/compose'; +const { Provider, Consumer } = createContext( { + onFocusLoss: () => {}, +} ); + +Provider.displayName = 'FocusReturnProvider'; +Consumer.displayName = 'FocusReturnConsumer'; + +/** + * Returns true if the given object is component-like. An object is component- + * like if it is an instance of wp.element.Component, or is a function. + * + * @param {*} object Object to test. + * + * @return {boolean} Whether object is component-like. + */ +function isComponentLike( object ) { + return ( + object instanceof Component || + typeof object === 'function' + ); +} + +/** + * Returns true if there is a focused element, or false otherwise. + * + * @return {boolean} Whether focused element exists. + */ +function hasFocusedElement() { + return ( + null !== document.activeElement && + document.body !== document.activeElement + ); +} + /** * Higher Order Component used to be used to wrap disposable elements like * sidebars, modals, dropdowns. When mounting the wrapped component, we track a * reference to the current active element so we know where to restore focus * when the component is unmounted. * - * @param {WPElement} WrappedComponent The disposable component. + * @param {(WPComponent|Object)} options The component to be enhanced with + * focus return behavior, or an object + * describing the component and the + * focus return characteristics. * * @return {Component} Component with the focus restauration behaviour. */ -export default createHigherOrderComponent( - ( WrappedComponent ) => { - return class extends Component { +function withFocusReturn( options ) { + // Normalize as overloaded form `withFocusReturn( options )( Component )` + // or as `withFocusReturn( Component )`. + if ( isComponentLike( options ) ) { + return withFocusReturn( {} )( options ); + } + + return function( WrappedComponent ) { + class FocusReturn extends Component { constructor() { super( ...arguments ); @@ -26,15 +69,18 @@ export default createHigherOrderComponent( } componentWillUnmount() { + const { onFocusLoss = this.props.onFocusLoss } = options; const { activeElementOnMount, isFocused } = this; - if ( ! activeElementOnMount ) { - return; - } - const { body, activeElement } = document; - if ( isFocused || null === activeElement || body === activeElement ) { + if ( activeElementOnMount && ( isFocused || ! hasFocusedElement() ) ) { activeElementOnMount.focus(); } + + setTimeout( () => { + if ( ! hasFocusedElement() ) { + onFocusLoss(); + } + }, 0 ); } render() { @@ -47,6 +93,15 @@ export default createHigherOrderComponent( ); } - }; - }, 'withFocusReturn' -); + } + + return ( props ) => ( + + { ( context ) => } + + ); + }; +} + +export default createHigherOrderComponent( withFocusReturn, 'withFocusReturn' ); +export { Provider, Consumer }; diff --git a/packages/components/src/index.js b/packages/components/src/index.js index 6e45caa6e1e055..c5c34089cdb7ce 100644 --- a/packages/components/src/index.js +++ b/packages/components/src/index.js @@ -70,6 +70,6 @@ export { default as withConstrainedTabbing } from './higher-order/with-constrain export { default as withFallbackStyles } from './higher-order/with-fallback-styles'; export { default as withFilters } from './higher-order/with-filters'; export { default as withFocusOutside } from './higher-order/with-focus-outside'; -export { default as withFocusReturn } from './higher-order/with-focus-return'; +export { default as withFocusReturn, Provider as FocusReturnProvider } from './higher-order/with-focus-return'; export { default as withNotices } from './higher-order/with-notices'; export { default as withSpokenMessages } from './higher-order/with-spoken-messages'; From 1ad3f95371b56874ed84d16620be71df159e3928 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 14 Mar 2019 16:33:28 -0400 Subject: [PATCH 02/11] Edit Post: Add application-wide focus loss handler --- .../edit-post/src/components/layout/index.js | 137 ++++++++++-------- 1 file changed, 77 insertions(+), 60 deletions(-) diff --git a/packages/edit-post/src/components/layout/index.js b/packages/edit-post/src/components/layout/index.js index d033b993a00b28..f17068ca1a3dfa 100644 --- a/packages/edit-post/src/components/layout/index.js +++ b/packages/edit-post/src/components/layout/index.js @@ -6,7 +6,13 @@ import classnames from 'classnames'; /** * WordPress dependencies */ -import { Button, Popover, ScrollLock, navigateRegions } from '@wordpress/components'; +import { + Button, + Popover, + ScrollLock, + FocusReturnProvider, + navigateRegions, +} from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { PreserveScrollInReorder } from '@wordpress/block-editor'; import { @@ -66,67 +72,78 @@ function Layout( { tabIndex: -1, }; return ( -
- - - - -
-
- - - - - - - - { ( mode === 'text' || ! isRichEditingEnabled ) && } - { isRichEditingEnabled && mode === 'visual' && } -
- -
-
- + +
+ + + + +
+
+ + + + + + + + { ( mode === 'text' || ! isRichEditingEnabled ) && } + { isRichEditingEnabled && mode === 'visual' && } +
+ +
+
+ +
+ { publishSidebarOpened ? ( + + ) : ( + +
+ +
+ + + { + isMobileViewport && sidebarIsOpened && + } + + ) } + +
- { publishSidebarOpened ? ( - - ) : ( - -
- -
- - - { - isMobileViewport && sidebarIsOpened && - } - - ) } - - -
+ ); } From 79420a2723b0e95b3d66e3029809767d25f216b8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 14 Mar 2019 16:33:44 -0400 Subject: [PATCH 03/11] Edit Post: Add sidebar focus loss handler --- packages/edit-post/src/components/sidebar/index.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/edit-post/src/components/sidebar/index.js b/packages/edit-post/src/components/sidebar/index.js index a9da35865e8755..484562e7bbc7ae 100644 --- a/packages/edit-post/src/components/sidebar/index.js +++ b/packages/edit-post/src/components/sidebar/index.js @@ -41,7 +41,14 @@ const WrappedSidebar = compose( isActive: select( 'core/edit-post' ).getActiveGeneralSidebarName() === name, } ) ), ifCondition( ( { isActive } ) => isActive ), - withFocusReturn, + withFocusReturn( { + onFocusLoss() { + const button = document.querySelector( '.edit-post-header__settings [aria-label="Settings"]' ); + if ( button ) { + button.focus(); + } + }, + } ), )( Sidebar ); WrappedSidebar.Slot = Slot; From ee486fefbd56937a04e4ee432e0df9891101a5c4 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 14 Mar 2019 16:49:46 -0400 Subject: [PATCH 04/11] Components: Add onFocusLoss prop to Modal component --- packages/components/src/modal/index.js | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/components/src/modal/index.js b/packages/components/src/modal/index.js index d82c6928a1073c..269cb8e3e9324e 100644 --- a/packages/components/src/modal/index.js +++ b/packages/components/src/modal/index.js @@ -17,6 +17,7 @@ import ModalFrame from './frame'; import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; import IsolatedEventContainer from '../isolated-event-container'; +import { Provider as FocusReturnProvider } from '../higher-order/with-focus-return'; // Used to count the number of open modals. let parentElement, @@ -119,6 +120,7 @@ class Modal extends Component { aria, instanceId, isDismissable, + onFocusLoss, ...otherProps } = this.props; @@ -127,7 +129,7 @@ class Modal extends Component { // Disable reason: this stops mouse events from triggering tooltips and // other elements underneath the modal overlay. /* eslint-disable jsx-a11y/no-static-element-interactions */ - return createPortal( + let element = ( @@ -155,10 +157,19 @@ class Modal extends Component { { children }
- , - this.node + ); /* eslint-enable jsx-a11y/no-static-element-interactions */ + + if ( onFocusLoss ) { + element = ( + + { element } + + ); + } + + return createPortal( element, this.node ); } } From a69f29dcf6785b14e44989c30df8726ce883ca5a Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Thu, 14 Mar 2019 16:50:01 -0400 Subject: [PATCH 05/11] Edit Post: Direct More Menu modals focus loss to menu button --- .../src/components/header/more-menu/modal.js | 20 +++++++++++++++++++ .../keyboard-shortcut-help-modal/index.js | 7 ++++--- .../src/components/options-modal/index.js | 6 +++--- 3 files changed, 27 insertions(+), 6 deletions(-) create mode 100644 packages/edit-post/src/components/header/more-menu/modal.js diff --git a/packages/edit-post/src/components/header/more-menu/modal.js b/packages/edit-post/src/components/header/more-menu/modal.js new file mode 100644 index 00000000000000..a67c5cad96ff62 --- /dev/null +++ b/packages/edit-post/src/components/header/more-menu/modal.js @@ -0,0 +1,20 @@ +/** + * WordPress dependencies + */ +import { Modal } from '@wordpress/components'; + +function MoreMenuModal( props ) { + return ( + { + const button = document.querySelector( '.edit-post-more-menu button' ); + if ( button ) { + button.focus(); + } + } } + /> + ); +} + +export default MoreMenuModal; diff --git a/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js b/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js index 810519ccd3a493..37759a20c0709f 100644 --- a/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js +++ b/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js @@ -7,7 +7,7 @@ import { castArray } from 'lodash'; * WordPress dependencies */ import { Fragment } from '@wordpress/element'; -import { Modal, KeyboardShortcuts } from '@wordpress/components'; +import { KeyboardShortcuts } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { rawShortcut } from '@wordpress/keycodes'; import { withSelect, withDispatch } from '@wordpress/data'; @@ -17,6 +17,7 @@ import { compose } from '@wordpress/compose'; * Internal dependencies */ import shortcutConfig from './config'; +import MoreMenuModal from '../header/more-menu/modal'; const MODAL_NAME = 'edit-post/keyboard-shortcut-help'; @@ -78,7 +79,7 @@ export function KeyboardShortcutHelpModal( { isModalActive, toggleModal } ) { } } /> { isModalActive && ( - ( ) ) } - + ) } ); diff --git a/packages/edit-post/src/components/options-modal/index.js b/packages/edit-post/src/components/options-modal/index.js index a7e2cc2f395cc0..bb792908035891 100644 --- a/packages/edit-post/src/components/options-modal/index.js +++ b/packages/edit-post/src/components/options-modal/index.js @@ -6,7 +6,6 @@ import { get } from 'lodash'; /** * WordPress dependencies */ -import { Modal } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { withSelect, withDispatch } from '@wordpress/data'; import { compose } from '@wordpress/compose'; @@ -28,6 +27,7 @@ import { EnablePanelOption, } from './options'; import MetaBoxesSection from './meta-boxes-section'; +import MoreMenuModal from '../header/more-menu/modal'; const MODAL_NAME = 'edit-post/options'; @@ -37,7 +37,7 @@ export function OptionsModal( { isModalActive, isViewable, closeModal } ) { } return ( - - + ); } From a203b08fd616d0f36a7bd35c88b5d333b224067f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 15 Mar 2019 12:20:25 -0400 Subject: [PATCH 06/11] Edit Post: Apply withFocusReturn to rendered DOM element --- .../edit-post/src/components/sidebar/index.js | 47 ++++++++++--------- 1 file changed, 26 insertions(+), 21 deletions(-) diff --git a/packages/edit-post/src/components/sidebar/index.js b/packages/edit-post/src/components/sidebar/index.js index 484562e7bbc7ae..cdaf3e6c5dfaa5 100644 --- a/packages/edit-post/src/components/sidebar/index.js +++ b/packages/edit-post/src/components/sidebar/index.js @@ -17,39 +17,44 @@ const { Fill, Slot } = createSlotFill( 'Sidebar' ); * * @return {Object} The rendered sidebar. */ -const Sidebar = ( { children, label } ) => { +function Sidebar( { children, label, className } ) { + return ( +
+ { children } +
+ ); +} + +Sidebar = withFocusReturn( { + onFocusLoss() { + const button = document.querySelector( '.edit-post-header__settings [aria-label="Settings"]' ); + if ( button ) { + button.focus(); + } + }, +} )( Sidebar ); + +function AnimatedSidebarFill( props ) { return ( - { ( { className } ) => ( -
- { children } -
- ) } + { () => } ); -}; +} const WrappedSidebar = compose( withSelect( ( select, { name } ) => ( { isActive: select( 'core/edit-post' ).getActiveGeneralSidebarName() === name, } ) ), ifCondition( ( { isActive } ) => isActive ), - withFocusReturn( { - onFocusLoss() { - const button = document.querySelector( '.edit-post-header__settings [aria-label="Settings"]' ); - if ( button ) { - button.focus(); - } - }, - } ), -)( Sidebar ); +)( AnimatedSidebarFill ); WrappedSidebar.Slot = Slot; From 5586d29bda0a4928648315ff95c2e1b6a242b7f7 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 15 Mar 2019 12:30:08 -0400 Subject: [PATCH 07/11] Components: Return focus by stack memory --- .../higher-order/with-focus-return/context.js | 54 ++++++++++++++++ .../higher-order/with-focus-return/index.js | 62 +++++++++++-------- .../with-focus-return/test/index.js | 13 ---- packages/components/src/modal/index.js | 17 +---- .../src/components/header/more-menu/modal.js | 20 ------ .../keyboard-shortcut-help-modal/index.js | 7 +-- .../edit-post/src/components/layout/index.js | 11 +--- .../src/components/options-modal/index.js | 6 +- .../edit-post/src/components/sidebar/index.js | 3 +- 9 files changed, 101 insertions(+), 92 deletions(-) create mode 100644 packages/components/src/higher-order/with-focus-return/context.js delete mode 100644 packages/edit-post/src/components/header/more-menu/modal.js diff --git a/packages/components/src/higher-order/with-focus-return/context.js b/packages/components/src/higher-order/with-focus-return/context.js new file mode 100644 index 00000000000000..8f270ebfe9b235 --- /dev/null +++ b/packages/components/src/higher-order/with-focus-return/context.js @@ -0,0 +1,54 @@ +/** + * WordPress dependencies + */ +import { Component, createContext } from '@wordpress/element'; + +const { Provider, Consumer } = createContext( { + focusHistory: [], +} ); + +Provider.displayName = 'FocusReturnProvider'; +Consumer.displayName = 'FocusReturnConsumer'; + +/** + * The maximum history length to capture for the focus stack. When exceeded, + * items should be shifted from the stack for each consecutive push. + * + * @type {number} + */ +const MAX_STACK_LENGTH = 100; + +class FocusReturnProvider extends Component { + constructor() { + super( ...arguments ); + + this.onFocus = this.onFocus.bind( this ); + + this.state = { + focusHistory: [], + }; + } + + onFocus( event ) { + const { focusHistory } = this.state; + const nextFocusHistory = [ + ...focusHistory, + event.target, + ].slice( -1 * MAX_STACK_LENGTH ); + + this.setState( { focusHistory: nextFocusHistory } ); + } + + render() { + return ( + +
+ { this.props.children } +
+ + ); + } +} + +export default FocusReturnProvider; +export { Consumer }; diff --git a/packages/components/src/higher-order/with-focus-return/index.js b/packages/components/src/higher-order/with-focus-return/index.js index b9ba6010732ba9..861e925cd06a8e 100644 --- a/packages/components/src/higher-order/with-focus-return/index.js +++ b/packages/components/src/higher-order/with-focus-return/index.js @@ -1,15 +1,18 @@ +/** + * External dependencies + */ +import { stubTrue } from 'lodash'; + /** * WordPress dependencies */ -import { Component, createContext } from '@wordpress/element'; +import { Component } from '@wordpress/element'; import { createHigherOrderComponent } from '@wordpress/compose'; -const { Provider, Consumer } = createContext( { - onFocusLoss: () => {}, -} ); - -Provider.displayName = 'FocusReturnProvider'; -Consumer.displayName = 'FocusReturnConsumer'; +/** + * Internal dependencies + */ +import Provider, { Consumer } from './context'; /** * Returns true if the given object is component-like. An object is component- @@ -26,18 +29,6 @@ function isComponentLike( object ) { ); } -/** - * Returns true if there is a focused element, or false otherwise. - * - * @return {boolean} Whether focused element exists. - */ -function hasFocusedElement() { - return ( - null !== document.activeElement && - document.body !== document.activeElement - ); -} - /** * Higher Order Component used to be used to wrap disposable elements like * sidebars, modals, dropdowns. When mounting the wrapped component, we track a @@ -58,6 +49,8 @@ function withFocusReturn( options ) { return withFocusReturn( {} )( options ); } + const { onFocusReturn = stubTrue } = options; + return function( WrappedComponent ) { class FocusReturn extends Component { constructor() { @@ -69,18 +62,33 @@ function withFocusReturn( options ) { } componentWillUnmount() { - const { onFocusLoss = this.props.onFocusLoss } = options; const { activeElementOnMount, isFocused } = this; - if ( activeElementOnMount && ( isFocused || ! hasFocusedElement() ) ) { - activeElementOnMount.focus(); + if ( ! isFocused ) { + return; } - setTimeout( () => { - if ( ! hasFocusedElement() ) { - onFocusLoss(); + // Defer to the component's own explicit focus return behavior, + // if specified. The function should return `false` to prevent + // the default behavior otherwise occurring here. This allows + // for support that the `onFocusReturn` decides to allow the + // default behavior to occur under some conditions. + if ( onFocusReturn() === false ) { + return; + } + + const stack = [ + ...this.props.focusHistory, + activeElementOnMount, + ]; + + let candidate; + while ( ( candidate = stack.pop() ) ) { + if ( document.body.contains( candidate ) ) { + candidate.focus(); + return; } - }, 0 ); + } } render() { @@ -104,4 +112,4 @@ function withFocusReturn( options ) { } export default createHigherOrderComponent( withFocusReturn, 'withFocusReturn' ); -export { Provider, Consumer }; +export { Provider }; diff --git a/packages/components/src/higher-order/with-focus-return/test/index.js b/packages/components/src/higher-order/with-focus-return/test/index.js index 3f88457e52732e..1a1163cf698069 100644 --- a/packages/components/src/higher-order/with-focus-return/test/index.js +++ b/packages/components/src/higher-order/with-focus-return/test/index.js @@ -70,18 +70,5 @@ describe( 'withFocusReturn()', () => { mountedComposite.unmount(); expect( document.activeElement ).toBe( switchFocusTo ); } ); - - it( 'should return focus to element associated with HOC', () => { - const mountedComposite = renderer.create( ); - expect( getInstance( mountedComposite ).activeElementOnMount ).toBe( activeElement ); - - // Change activeElement. - document.activeElement.blur(); - expect( document.activeElement ).toBe( document.body ); - - // Should return to the activeElement saved with this component. - mountedComposite.unmount(); - expect( document.activeElement ).toBe( activeElement ); - } ); } ); } ); diff --git a/packages/components/src/modal/index.js b/packages/components/src/modal/index.js index 269cb8e3e9324e..d82c6928a1073c 100644 --- a/packages/components/src/modal/index.js +++ b/packages/components/src/modal/index.js @@ -17,7 +17,6 @@ import ModalFrame from './frame'; import ModalHeader from './header'; import * as ariaHelper from './aria-helper'; import IsolatedEventContainer from '../isolated-event-container'; -import { Provider as FocusReturnProvider } from '../higher-order/with-focus-return'; // Used to count the number of open modals. let parentElement, @@ -120,7 +119,6 @@ class Modal extends Component { aria, instanceId, isDismissable, - onFocusLoss, ...otherProps } = this.props; @@ -129,7 +127,7 @@ class Modal extends Component { // Disable reason: this stops mouse events from triggering tooltips and // other elements underneath the modal overlay. /* eslint-disable jsx-a11y/no-static-element-interactions */ - let element = ( + return createPortal( @@ -157,19 +155,10 @@ class Modal extends Component { { children }
- + , + this.node ); /* eslint-enable jsx-a11y/no-static-element-interactions */ - - if ( onFocusLoss ) { - element = ( - - { element } - - ); - } - - return createPortal( element, this.node ); } } diff --git a/packages/edit-post/src/components/header/more-menu/modal.js b/packages/edit-post/src/components/header/more-menu/modal.js deleted file mode 100644 index a67c5cad96ff62..00000000000000 --- a/packages/edit-post/src/components/header/more-menu/modal.js +++ /dev/null @@ -1,20 +0,0 @@ -/** - * WordPress dependencies - */ -import { Modal } from '@wordpress/components'; - -function MoreMenuModal( props ) { - return ( - { - const button = document.querySelector( '.edit-post-more-menu button' ); - if ( button ) { - button.focus(); - } - } } - /> - ); -} - -export default MoreMenuModal; diff --git a/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js b/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js index 37759a20c0709f..810519ccd3a493 100644 --- a/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js +++ b/packages/edit-post/src/components/keyboard-shortcut-help-modal/index.js @@ -7,7 +7,7 @@ import { castArray } from 'lodash'; * WordPress dependencies */ import { Fragment } from '@wordpress/element'; -import { KeyboardShortcuts } from '@wordpress/components'; +import { Modal, KeyboardShortcuts } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { rawShortcut } from '@wordpress/keycodes'; import { withSelect, withDispatch } from '@wordpress/data'; @@ -17,7 +17,6 @@ import { compose } from '@wordpress/compose'; * Internal dependencies */ import shortcutConfig from './config'; -import MoreMenuModal from '../header/more-menu/modal'; const MODAL_NAME = 'edit-post/keyboard-shortcut-help'; @@ -79,7 +78,7 @@ export function KeyboardShortcutHelpModal( { isModalActive, toggleModal } ) { } } /> { isModalActive && ( - ( ) ) } - + ) } ); diff --git a/packages/edit-post/src/components/layout/index.js b/packages/edit-post/src/components/layout/index.js index f17068ca1a3dfa..1ee4b46c24ad39 100644 --- a/packages/edit-post/src/components/layout/index.js +++ b/packages/edit-post/src/components/layout/index.js @@ -72,16 +72,7 @@ function Layout( { tabIndex: -1, }; return ( - +
diff --git a/packages/edit-post/src/components/options-modal/index.js b/packages/edit-post/src/components/options-modal/index.js index bb792908035891..a7e2cc2f395cc0 100644 --- a/packages/edit-post/src/components/options-modal/index.js +++ b/packages/edit-post/src/components/options-modal/index.js @@ -6,6 +6,7 @@ import { get } from 'lodash'; /** * WordPress dependencies */ +import { Modal } from '@wordpress/components'; import { __ } from '@wordpress/i18n'; import { withSelect, withDispatch } from '@wordpress/data'; import { compose } from '@wordpress/compose'; @@ -27,7 +28,6 @@ import { EnablePanelOption, } from './options'; import MetaBoxesSection from './meta-boxes-section'; -import MoreMenuModal from '../header/more-menu/modal'; const MODAL_NAME = 'edit-post/options'; @@ -37,7 +37,7 @@ export function OptionsModal( { isModalActive, isViewable, closeModal } ) { } return ( - - + ); } diff --git a/packages/edit-post/src/components/sidebar/index.js b/packages/edit-post/src/components/sidebar/index.js index cdaf3e6c5dfaa5..21559c2eea2553 100644 --- a/packages/edit-post/src/components/sidebar/index.js +++ b/packages/edit-post/src/components/sidebar/index.js @@ -31,10 +31,11 @@ function Sidebar( { children, label, className } ) { } Sidebar = withFocusReturn( { - onFocusLoss() { + onFocusReturn() { const button = document.querySelector( '.edit-post-header__settings [aria-label="Settings"]' ); if ( button ) { button.focus(); + return false; } }, } )( Sidebar ); From 13c7ae3d351d8f8c8437fa59ac1bcc7178d7e151 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 15 Mar 2019 12:36:49 -0400 Subject: [PATCH 08/11] Edit Post: Collapse FocusReturnProvider to handle className application to div --- .../higher-order/with-focus-return/context.js | 6 +- .../edit-post/src/components/layout/index.js | 118 +++++++++--------- 2 files changed, 62 insertions(+), 62 deletions(-) diff --git a/packages/components/src/higher-order/with-focus-return/context.js b/packages/components/src/higher-order/with-focus-return/context.js index 8f270ebfe9b235..5db8823422f660 100644 --- a/packages/components/src/higher-order/with-focus-return/context.js +++ b/packages/components/src/higher-order/with-focus-return/context.js @@ -40,10 +40,12 @@ class FocusReturnProvider extends Component { } render() { + const { children, className } = this.props; + return ( -
- { this.props.children } +
+ { children }
); diff --git a/packages/edit-post/src/components/layout/index.js b/packages/edit-post/src/components/layout/index.js index 1ee4b46c24ad39..643184f4b971be 100644 --- a/packages/edit-post/src/components/layout/index.js +++ b/packages/edit-post/src/components/layout/index.js @@ -72,68 +72,66 @@ function Layout( { tabIndex: -1, }; return ( - -
- - - - -
-
- - - - - - - - { ( mode === 'text' || ! isRichEditingEnabled ) && } - { isRichEditingEnabled && mode === 'visual' && } -
- -
-
- -
+ + + + + +
+
+ + + + + + + + { ( mode === 'text' || ! isRichEditingEnabled ) && } + { isRichEditingEnabled && mode === 'visual' && } +
+ +
+
+
- { publishSidebarOpened ? ( - - ) : ( - -
- -
- - - { - isMobileViewport && sidebarIsOpened && - } - - ) } - -
+ { publishSidebarOpened ? ( + + ) : ( + +
+ +
+ + + { + isMobileViewport && sidebarIsOpened && + } + + ) } + + ); } From 902a163c6c830c1a3a86b4aa50f9d8abb99987d8 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Sat, 16 Mar 2019 14:43:58 -0400 Subject: [PATCH 09/11] Components: Keep only unique entries of focus history --- .../higher-order/with-focus-return/context.js | 23 +++++++++++++++---- 1 file changed, 19 insertions(+), 4 deletions(-) diff --git a/packages/components/src/higher-order/with-focus-return/context.js b/packages/components/src/higher-order/with-focus-return/context.js index 5db8823422f660..f65834f4886927 100644 --- a/packages/components/src/higher-order/with-focus-return/context.js +++ b/packages/components/src/higher-order/with-focus-return/context.js @@ -1,3 +1,8 @@ +/** + * External dependencies + */ +import { uniq } from 'lodash'; + /** * WordPress dependencies */ @@ -31,10 +36,20 @@ class FocusReturnProvider extends Component { onFocus( event ) { const { focusHistory } = this.state; - const nextFocusHistory = [ - ...focusHistory, - event.target, - ].slice( -1 * MAX_STACK_LENGTH ); + + // Push the focused element to the history stack, keeping only unique + // members but preferring the _last_ occurrence of any duplicates. + // Lodash's `uniq` behavior favors the first occurrence, so the array + // is temporarily reversed prior to it being called upon. Uniqueness + // helps avoid situations where, such as in a constrained tabbing area, + // the user changes focus enough within a transient element that the + // stack may otherwise only consist of members pending destruction, at + // which point focus might have been lost. + const nextFocusHistory = uniq( + [ ...focusHistory, event.target ] + .slice( -1 * MAX_STACK_LENGTH ) + .reverse() + ).reverse(); this.setState( { focusHistory: nextFocusHistory } ); } From 76cdae1c8a23746a37c3350051011cb93a449b38 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Sat, 16 Mar 2019 14:44:54 -0400 Subject: [PATCH 10/11] Components: Monitor own focused elements for focus return unmount Candidate elements may still be in the DOM at the point of willUnmount, and must be excluded from consideration (as they are pending removal) --- .../higher-order/with-focus-return/index.js | 24 ++++++--- .../with-focus-return/test/index.js | 52 +++++++++++++++++-- 2 files changed, 66 insertions(+), 10 deletions(-) diff --git a/packages/components/src/higher-order/with-focus-return/index.js b/packages/components/src/higher-order/with-focus-return/index.js index 861e925cd06a8e..470d1f1fba7b51 100644 --- a/packages/components/src/higher-order/with-focus-return/index.js +++ b/packages/components/src/higher-order/with-focus-return/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { stubTrue } from 'lodash'; +import { stubTrue, without } from 'lodash'; /** * WordPress dependencies @@ -46,7 +46,8 @@ function withFocusReturn( options ) { // Normalize as overloaded form `withFocusReturn( options )( Component )` // or as `withFocusReturn( Component )`. if ( isComponentLike( options ) ) { - return withFocusReturn( {} )( options ); + const WrappedComponent = options; + return withFocusReturn( {} )( WrappedComponent ); } const { onFocusReturn = stubTrue } = options; @@ -56,13 +57,21 @@ function withFocusReturn( options ) { constructor() { super( ...arguments ); - this.setIsFocusedTrue = () => this.isFocused = true; - this.setIsFocusedFalse = () => this.isFocused = false; + this.ownFocusedElements = new Set; this.activeElementOnMount = document.activeElement; + this.setIsFocusedFalse = () => this.isFocused = false; + this.setIsFocusedTrue = ( event ) => { + this.ownFocusedElements.add( event.target ); + this.isFocused = true; + }; } componentWillUnmount() { - const { activeElementOnMount, isFocused } = this; + const { + activeElementOnMount, + isFocused, + ownFocusedElements, + } = this; if ( ! isFocused ) { return; @@ -78,7 +87,10 @@ function withFocusReturn( options ) { } const stack = [ - ...this.props.focusHistory, + ...without( + this.props.focusHistory, + ...ownFocusedElements + ), activeElementOnMount, ]; diff --git a/packages/components/src/higher-order/with-focus-return/test/index.js b/packages/components/src/higher-order/with-focus-return/test/index.js index 1a1163cf698069..175a118efc2850 100644 --- a/packages/components/src/higher-order/with-focus-return/test/index.js +++ b/packages/components/src/higher-order/with-focus-return/test/index.js @@ -2,21 +2,22 @@ * External dependencies */ import renderer from 'react-test-renderer'; +import { mount } from 'enzyme'; /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; +import { Component, createElement } from '@wordpress/element'; /** * Internal dependencies */ -import withFocusReturn from '../'; +import withFocusReturn, { Provider } from '../'; class Test extends Component { render() { return ( -
Testing
+