diff --git a/package-lock.json b/package-lock.json index e45471fa3960fe..194a19ce3e2826 100644 --- a/package-lock.json +++ b/package-lock.json @@ -13312,7 +13312,6 @@ "@wordpress/priority-queue": "file:packages/priority-queue", "clipboard": "^2.0.1", "lodash": "^4.17.21", - "memize": "^1.1.0", "mousetrap": "^1.6.5", "react-resize-aware": "^3.1.0", "use-memo-one": "^1.1.1" diff --git a/packages/components/src/popover/index.js b/packages/components/src/popover/index.js index 3d2d3e650c4e84..2948c879b8fa9f 100644 --- a/packages/components/src/popover/index.js +++ b/packages/components/src/popover/index.js @@ -13,17 +13,12 @@ import { forwardRef, } from '@wordpress/element'; import { getRectangleFromRange } from '@wordpress/dom'; -import { ESCAPE } from '@wordpress/keycodes'; import deprecated from '@wordpress/deprecated'; import { useViewportMatch, useResizeObserver, - useFocusOnMount, - __experimentalUseFocusOutside as useFocusOutside, - useConstrainedTabbing, - useFocusReturn, useMergeRefs, - useRefEffect, + __experimentalUseDialog as useDialog, } from '@wordpress/compose'; import { close } from '@wordpress/icons'; @@ -485,97 +480,37 @@ const Popover = ( __unstableBoundaryParent, ] ); - // Event handlers for closing the popover. - const closeEventRef = useRefEffect( - ( node ) => { - function maybeClose( event ) { - // Close on escape. - if ( event.keyCode === ESCAPE && onClose ) { - event.stopPropagation(); - onClose(); - } - } - - node.addEventListener( 'keydown', maybeClose ); - - return () => { - node.removeEventListener( 'keydown', maybeClose ); - }; - }, - [ onClose ] - ); - - const constrainedTabbingRef = useConstrainedTabbing(); - const focusReturnRef = useFocusReturn(); - const focusOnMountRef = useFocusOnMount( focusOnMount ); - const focusOutsideProps = useFocusOutside( handleOnFocusOutside ); - const mergedRefs = useMergeRefs( [ - ref, - containerRef, - // Don't register the event at all if there's no onClose callback. - onClose ? closeEventRef : null, - focusOnMount ? constrainedTabbingRef : null, - focusOnMount ? focusReturnRef : null, - focusOnMount ? focusOnMountRef : null, - ] ); - - /** - * Shims an onFocusOutside callback to be compatible with a deprecated - * onClickOutside prop function, if provided. - * - * @param {FocusEvent} event Focus event from onFocusOutside. - */ - function handleOnFocusOutside( event ) { - // Defer to given `onFocusOutside` if specified. Call `onClose` only if - // both `onFocusOutside` and `onClickOutside` are unspecified. Doing so - // assures backwards-compatibility for prior `onClickOutside` default. - if ( onFocusOutside ) { + const onDialogClose = ( type, event ) => { + // Ideally the popover should have just a single onClose prop and + // not three props that potentially do the same thing. + if ( type === 'focus-outside' && onFocusOutside ) { onFocusOutside( event ); - return; - } else if ( ! onClickOutside ) { - if ( onClose ) { - onClose(); - } - return; - } - - // Simulate MouseEvent using FocusEvent#relatedTarget as emulated click - // target. MouseEvent constructor is unsupported in Internet Explorer. - let clickEvent; - try { - clickEvent = new window.MouseEvent( 'click' ); - } catch ( error ) { - clickEvent = document.createEvent( 'MouseEvent' ); - clickEvent.initMouseEvent( - 'click', - true, - true, - window, - 0, - 0, - 0, - 0, - 0, - false, - false, - false, - false, - 0, - null - ); + } else if ( type === 'focus-outside' && onClickOutside ) { + // Simulate MouseEvent using FocusEvent#relatedTarget as emulated click target. + const clickEvent = new window.MouseEvent( 'click' ); + + Object.defineProperty( clickEvent, 'target', { + get: () => event.relatedTarget, + } ); + + deprecated( 'Popover onClickOutside prop', { + since: '5.3', + alternative: 'onFocusOutside', + } ); + + onClickOutside( clickEvent ); + } else if ( onClose ) { + onClose(); } + }; - Object.defineProperty( clickEvent, 'target', { - get: () => event.relatedTarget, - } ); + const [ dialogRef, dialogProps ] = useDialog( { + focusOnMount, + __unstableOnClose: onDialogClose, + onClose: onDialogClose, + } ); - deprecated( 'Popover onClickOutside prop', { - since: '5.3', - alternative: 'onFocusOutside', - } ); - - onClickOutside( clickEvent ); - } + const mergedRefs = useMergeRefs( [ containerRef, dialogRef, ref ] ); /** @type {false | string} */ const animateClassName = @@ -603,8 +538,8 @@ const Popover = ( } ) } { ...contentProps } - { ...focusOutsideProps } ref={ mergedRefs } + { ...dialogProps } tabIndex="-1" > { isExpanded && } diff --git a/packages/components/src/popover/test/index.js b/packages/components/src/popover/test/index.js index 20d211a5b9273f..98dc26f20699a1 100644 --- a/packages/components/src/popover/test/index.js +++ b/packages/components/src/popover/test/index.js @@ -15,29 +15,6 @@ describe( 'Popover', () => { } } ); - 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' ) ); - - expect( document.activeElement ).toBe( document.body ); - - // 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. - let result; - act( () => { - result = render( ); - - jest.advanceTimersByTime( 1 ); - } ); - - expect( document.activeElement ).toBe( - result.container.querySelector( '.components-popover' ) - ); - } ); - it( 'should allow focus-on-open behavior to be disabled', () => { expect( document.activeElement ).toBe( document.body ); diff --git a/packages/compose/package.json b/packages/compose/package.json index 24e173fc0c0cef..8cb8c870b59ccc 100644 --- a/packages/compose/package.json +++ b/packages/compose/package.json @@ -38,7 +38,6 @@ "@wordpress/priority-queue": "file:../priority-queue", "clipboard": "^2.0.1", "lodash": "^4.17.21", - "memize": "^1.1.0", "mousetrap": "^1.6.5", "react-resize-aware": "^3.1.0", "use-memo-one": "^1.1.1" diff --git a/packages/compose/src/hooks/use-dialog/index.js b/packages/compose/src/hooks/use-dialog/index.js index 777ab66b15b698..48f347f2a6b500 100644 --- a/packages/compose/src/hooks/use-dialog/index.js +++ b/packages/compose/src/hooks/use-dialog/index.js @@ -23,14 +23,22 @@ import useMergeRefs from '../use-merge-refs'; * @param {Object} options Dialog Options. */ function useDialog( options ) { - const onClose = useRef(); + const currentOptions = useRef(); useEffect( () => { - onClose.current = options.onClose; - }, [ options.onClose ] ); + currentOptions.current = options; + }, Object.values( options ) ); const constrainedTabbingRef = useConstrainedTabbing(); - const focusOnMountRef = useFocusOnMount(); + const focusOnMountRef = useFocusOnMount( options.focusOnMount ); const focusReturnRef = useFocusReturn(); - const focusOutsideProps = useFocusOutside( options.onClose ); + const focusOutsideProps = useFocusOutside( ( event ) => { + // This unstable prop is here only to manage backward compatibility + // for the Popover component otherwise, the onClose should be enough. + if ( currentOptions.current.__unstableOnClose ) { + currentOptions.current.__unstableOnClose( 'focus-outside', event ); + } else if ( currentOptions.current.onClose ) { + currentOptions.current.onClose(); + } + } ); const closeOnEscapeRef = useCallback( ( node ) => { if ( ! node ) { return; @@ -38,18 +46,18 @@ function useDialog( options ) { node.addEventListener( 'keydown', ( event ) => { // Close on escape - if ( event.keyCode === ESCAPE && onClose.current ) { + if ( event.keyCode === ESCAPE && currentOptions.current.onClose ) { event.stopPropagation(); - onClose.current(); + currentOptions.current.onClose(); } } ); }, [] ); return [ useMergeRefs( [ - constrainedTabbingRef, - focusReturnRef, - focusOnMountRef, + options.focusOnMount !== false ? constrainedTabbingRef : null, + options.focusOnMount !== false ? focusReturnRef : null, + options.focusOnMount !== false ? focusOnMountRef : null, closeOnEscapeRef, ] ), {