Accessibility: Consolidate the focusOnOpen behavior in the withFocusReturn Higher Order Component

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 }
 					>
-						<FocusManaged>
-							{ renderContent( args ) }
-						</FocusManaged>
+						{ renderContent( args ) }
 					</Popover>
 				</div>
 			</div>

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 (
 				<div
 					onFocus={ this.setIsFocusedTrue }
 					onBlur={ this.setIsFocusedFalse }
+					ref={ this.bindContainer }
+					tabIndex="-1"
 				>
 					<WrappedComponent { ...this.props } />
 				</div>

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`] = `"<div tabindex=\\"-1\\"><div class=\\"test\\">Testing</div></div>"`;
+
+exports[`withFocusReturn() testing rendering and focus handling should pass additional props through to the wrapped element 1`] = `
+<_class
+  data-test="test"
+>
+  <div
+    onBlur={[Function]}
+    onFocus={[Function]}
+    tabIndex="-1"
+  >
+    <Test
+      data-test="test"
+    >
+      <div
+        className="test"
+      >
+        Testing
+      </div>
+    </Test>
+  </div>
+</_class>
+`;
+
+exports[`withFocusReturn() testing rendering and focus handling should render a basic Test component inside the HOC 1`] = `
+<_class>
+  <div
+    onBlur={[Function]}
+    onFocus={[Function]}
+    tabIndex="-1"
+  >
+    <Test>
+      <div
+        className="test"
+      >
+        Testing
+      </div>
+    </Test>
+  </div>
+</_class>
+`;

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 (
-			<div className="test">Testing</div>
+			<div className="test" { ...props }>Testing</div>
 		);
 	}
 }
@@ -32,19 +32,18 @@ describe( 'withFocusReturn()', () => {
 		} );
 
 		it( 'should render a basic Test component inside the HOC', () => {
-			const renderedComposite = shallow( <Composite /> );
-			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( <Composite /> );
+			expect( renderedComposite ).toMatchSnapshot();
 		} );
 
 		it( 'should pass additional props through to the wrapped element', () => {
-			const renderedComposite = shallow( <Composite test="test" /> );
-			const wrappedElement = renderedComposite.find( 'Test' );
-			// Ensure that the wrapped Test element has the appropriate props.
-			expect( wrappedElement.props().test ).toBe( 'test' );
+			const renderedComposite = mount( <Composite data-test="test" /> );
+			expect( renderedComposite ).toMatchSnapshot();
+		} );
+
+		it( 'should focus the container on mount', () => {
+			mount( <Composite data-test="test" /> );
+			expect( document.activeElement.outerHTML ).toMatchSnapshot();
 		} );
 
 		it( 'should not switch focus back to the bound focus element', () => {

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 {
 					<div
 						ref={ this.bindNode( 'content' ) }
 						className="components-popover__content"
-						tabIndex="-1"
 					>
 						{ children }
 					</div>

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( <Popover /> );
 			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 );
 		} );

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 );