diff --git a/components/popover/index.js b/components/popover/index.js index f10e52f2d60ab..df975fd536975 100644 --- a/components/popover/index.js +++ b/components/popover/index.js @@ -2,7 +2,7 @@ * External dependencies */ import classnames from 'classnames'; -import { isEqual, noop } from 'lodash'; +import { defer, isEqual, noop } from 'lodash'; /** * WordPress dependencies @@ -19,6 +19,19 @@ import PopoverDetectOutside from './detect-outside'; import IconButton from '../icon-button'; import { Slot, Fill } from '../slot-fill'; +/** + * Value representing whether a key is currently pressed. Bound to the document + * for use in determining whether the Popover component has mounted in response + * to a keyboard event. Popover's focusOnMount behavior is specific to keyboard + * interaction. Must be bound at the top-level and its unsetting deferred since + * the component will have already mounted by the time keyup occurs. + * + * @type {boolean} + */ +let isKeyDown = false; +document.addEventListener( 'keydown', () => isKeyDown = true ); +document.addEventListener( 'keyup', defer.bind( null, () => isKeyDown = false ) ); + const FocusManaged = withFocusReturn( ( { children } ) => children ); const { ESCAPE } = keycodes; @@ -94,7 +107,7 @@ class Popover extends Component { focus() { const { focusOnMount = true } = this.props; - if ( ! focusOnMount ) { + if ( ! focusOnMount || ! isKeyDown ) { return; } diff --git a/components/popover/test/index.js b/components/popover/test/index.js index 8fa4b179a2ed7..f6bb7dbe7eccc 100644 --- a/components/popover/test/index.js +++ b/components/popover/test/index.js @@ -25,6 +25,14 @@ describe( 'Popover', () => { afterEach( () => { jest.restoreAllMocks(); + + // Resetting keyboard state is deferred, so ensure that timers are + // consumed to avoid leaking into other tests. + jest.runAllTimers(); + + if ( document.activeElement ) { + document.activeElement.blur(); + } } ); it( 'should add window events', () => { @@ -59,7 +67,12 @@ describe( 'Popover', () => { expect( Popover.prototype.setForcedPositions ).not.toHaveBeenCalled(); } ); - it( 'should focus when opening', () => { + it( 'should focus when opening in response to keyboard event', () => { + // As in the real world, these occur in sequence before the popover + // has been mounted. Keyup's resetting is deferred. + document.dispatchEvent( new window.KeyboardEvent( 'keydown' ) ); + document.dispatchEvent( new window.KeyboardEvent( 'keyup' ) ); + // 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. @@ -70,6 +83,12 @@ describe( 'Popover', () => { expect( document.activeElement ).toBe( content ); } ); + it( 'should not focus when opening in response to pointer event', () => { + wrapper = mount( ); + + expect( document.activeElement ).toBe( document.body ); + } ); + it( 'should allow focus-on-open behavior to be disabled', () => { const activeElement = document.activeElement;