diff --git a/components/dropdown/index.js b/components/dropdown/index.js index 449e7afc3afe8a..2ad102b6cdcfd6 100644 --- a/components/dropdown/index.js +++ b/components/dropdown/index.js @@ -6,11 +6,8 @@ import { Component } from '@wordpress/element'; /** * Internal Dependencies */ -import withFocusReturn from '../higher-order/with-focus-return'; import Popover from '../popover'; -const FocusManaged = withFocusReturn( ( { children } ) => children ); - class Dropdown extends Component { constructor() { super( ...arguments ); @@ -88,9 +85,7 @@ class Dropdown extends Component { onClickOutside={ this.clickOutside } expandOnMobile={ expandOnMobile } > - - { renderContent( args ) } - + { renderContent( args ) } diff --git a/components/higher-order/with-focus-return/index.js b/components/higher-order/with-focus-return/index.js index 9fc822117213a9..6813ef55f53710 100644 --- a/components/higher-order/with-focus-return/index.js +++ b/components/higher-order/with-focus-return/index.js @@ -2,6 +2,7 @@ * WordPress dependencies */ import { Component } from '@wordpress/element'; +import { focus } from '@wordpress/utils'; /** * Higher Order Component used to be used to wrap disposable elements like Sidebars, modals, dropdowns. @@ -19,12 +20,24 @@ function withFocusReturn( WrappedComponent ) { this.setIsFocusedTrue = () => this.isFocused = true; this.setIsFocusedFalse = () => this.isFocused = false; + this.bindContainer = this.bindContainer.bind( this ); } componentWillMount() { this.activeElementOnMount = document.activeElement; } + componentDidMount() { + // Find first tabbable node within content and shift focus, falling + // back to the popover panel itself. + const firstTabbable = focus.tabbable.find( this.container )[ 0 ]; + if ( firstTabbable ) { + firstTabbable.focus(); + } else { + this.container.focus(); + } + } + componentWillUnmount() { const { activeElementOnMount, isFocused } = this; if ( ! activeElementOnMount ) { @@ -37,11 +50,17 @@ function withFocusReturn( WrappedComponent ) { } } + bindContainer( ref ) { + this.container = ref; + } + render() { return (
diff --git a/components/higher-order/with-focus-return/test/__snapshots__/index.js.snap b/components/higher-order/with-focus-return/test/__snapshots__/index.js.snap new file mode 100644 index 00000000000000..d0d7d777619780 --- /dev/null +++ b/components/higher-order/with-focus-return/test/__snapshots__/index.js.snap @@ -0,0 +1,43 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`withFocusReturn() testing rendering and focus handling should focus the container on mount 1`] = `"
Testing
"`; + +exports[`withFocusReturn() testing rendering and focus handling should pass additional props through to the wrapped element 1`] = ` +<_class + data-test="test" +> +
+ +
+ Testing +
+
+
+ +`; + +exports[`withFocusReturn() testing rendering and focus handling should render a basic Test component inside the HOC 1`] = ` +<_class> +
+ +
+ Testing +
+
+
+ +`; diff --git a/components/higher-order/with-focus-return/test/index.js b/components/higher-order/with-focus-return/test/index.js index 3226dc65e676c6..84df61dc34fe0b 100644 --- a/components/higher-order/with-focus-return/test/index.js +++ b/components/higher-order/with-focus-return/test/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { shallow, mount } from 'enzyme'; +import { mount } from 'enzyme'; import { Component } from '../../../../element'; /** @@ -10,9 +10,9 @@ import { Component } from '../../../../element'; import withFocusReturn from '../'; class Test extends Component { - render() { + render( props ) { return ( -
Testing
+
Testing
); } } @@ -32,19 +32,18 @@ describe( 'withFocusReturn()', () => { } ); it( 'should render a basic Test component inside the HOC', () => { - const renderedComposite = shallow( ); - const wrappedElement = renderedComposite.find( 'Test' ); - const wrappedElementShallow = wrappedElement.shallow(); - expect( wrappedElementShallow.hasClass( 'test' ) ).toBe( true ); - expect( wrappedElementShallow.type() ).toBe( 'div' ); - expect( wrappedElementShallow.text() ).toBe( 'Testing' ); + const renderedComposite = mount( ); + expect( renderedComposite ).toMatchSnapshot(); } ); it( 'should pass additional props through to the wrapped element', () => { - const renderedComposite = shallow( ); - const wrappedElement = renderedComposite.find( 'Test' ); - // Ensure that the wrapped Test element has the appropriate props. - expect( wrappedElement.props().test ).toBe( 'test' ); + const renderedComposite = mount( ); + expect( renderedComposite ).toMatchSnapshot(); + } ); + + it( 'should focus the container on mount', () => { + mount( ); + expect( document.activeElement.outerHTML ).toMatchSnapshot(); } ); it( 'should not switch focus back to the bound focus element', () => { diff --git a/components/popover/index.js b/components/popover/index.js index 93f2359c8bc98f..1e86f30a386fed 100644 --- a/components/popover/index.js +++ b/components/popover/index.js @@ -8,7 +8,7 @@ import { isEqual, noop } from 'lodash'; * WordPress dependencies */ import { Component } from '@wordpress/element'; -import { focus, keycodes } from '@wordpress/utils'; +import { keycodes } from '@wordpress/utils'; /** * Internal dependencies @@ -35,7 +35,6 @@ class Popover extends Component { constructor() { super( ...arguments ); - this.focus = this.focus.bind( this ); this.bindNode = this.bindNode.bind( this ); this.getAnchorRect = this.getAnchorRect.bind( this ); this.setOffset = this.setOffset.bind( this ); @@ -73,10 +72,6 @@ class Popover extends Component { const { isOpen: prevIsOpen, position: prevPosition } = prevProps; if ( isOpen !== prevIsOpen ) { this.toggleWindowEvents( isOpen ); - - if ( isOpen ) { - this.focus(); - } } if ( ! isOpen ) { @@ -104,27 +99,6 @@ class Popover extends Component { window[ handler ]( 'scroll', this.throttledSetOffset, true ); } - focus() { - const { focusOnOpen = true } = this.props; - if ( ! focusOnOpen ) { - return; - } - - const { content } = this.nodes; - if ( ! content ) { - return; - } - - // Find first tabbable node within content and shift focus, falling - // back to the popover panel itself. - const firstTabbable = focus.tabbable.find( content )[ 0 ]; - if ( firstTabbable ) { - firstTabbable.focus(); - } else { - content.focus(); - } - } - throttledSetOffset() { this.rafHandle = window.requestAnimationFrame( this.setOffset ); } @@ -303,7 +277,6 @@ class Popover extends Component {
{ children }
diff --git a/components/popover/test/index.js b/components/popover/test/index.js index fdf4f1e2e22389..7d588b62c65971 100644 --- a/components/popover/test/index.js +++ b/components/popover/test/index.js @@ -70,11 +70,12 @@ describe( 'Popover', () => { it( 'should focus when opening', () => { // An ideal test here would mount with an input child and focus the // child, but in context of JSDOM the inputs are not visible and - // are therefore skipped as tabbable, defaulting to popover. + // are therefore skipped as tabbable, defaulting to the popover wrapper. wrapper = mount( ); wrapper.setProps( { isOpen: true } ); - const content = wrapper.find( '.components-popover__content' ).getDOMNode(); + // Should focus the withFocusReturn wrapper + const content = wrapper.find( '.components-popover' ).getDOMNode().parentNode; expect( document.activeElement ).toBe( content ); } ); diff --git a/editor/components/post-publish-panel/index.js b/editor/components/post-publish-panel/index.js index 49497486cc890a..286e74db2fdd1a 100644 --- a/editor/components/post-publish-panel/index.js +++ b/editor/components/post-publish-panel/index.js @@ -9,7 +9,7 @@ import { get } from 'lodash'; */ import { __ } from '@wordpress/i18n'; import { compose, Component } from '@wordpress/element'; -import { withAPIData, IconButton, Spinner } from '@wordpress/components'; +import { withAPIData, IconButton, Spinner, withFocusReturn } from '@wordpress/components'; /** * Internal Dependencies @@ -100,4 +100,5 @@ const applyWithAPIData = withAPIData( ( props ) => { export default compose( [ applyConnect, applyWithAPIData, + withFocusReturn, ] )( PostPublishPanel );