diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 7ad6becdb8aef6..07e8ff4ea5dc50 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -12,6 +12,7 @@ - `Tooltip` and `Button`: tidy up unit tests ([#57975](https://github.com/WordPress/gutenberg/pull/57975)). - `BorderControl`, `BorderBoxControl`: Replace style picker with ToggleGroupControl ([#57562](https://github.com/WordPress/gutenberg/pull/57562)). - `SlotFill`: fix typo in use-slot-fills return docs ([#57654](https://github.com/WordPress/gutenberg/pull/57654)) +- `Popover`: Adding `constrainTabbing` prop to `useDialog` hook ([#57962](https://github.com/WordPress/gutenberg/pull/57962)) ### Bug Fix diff --git a/packages/components/src/popover/index.tsx b/packages/components/src/popover/index.tsx index 709d4b9884b5ec..1634079c8cae76 100644 --- a/packages/components/src/popover/index.tsx +++ b/packages/components/src/popover/index.tsx @@ -126,6 +126,7 @@ const UnconnectedPopover = ( const { animate = true, headerTitle, + constrainTabbing, onClose, children, className, @@ -264,6 +265,7 @@ const UnconnectedPopover = ( } const [ dialogRef, dialogProps ] = useDialog( { + constrainTabbing, focusOnMount, __unstableOnClose: onDialogClose, // @ts-expect-error The __unstableOnClose property needs to be deprecated first (see https://github.com/WordPress/gutenberg/pull/27675) diff --git a/packages/components/src/popover/test/index.tsx b/packages/components/src/popover/test/index.tsx index 33a2d8758c09db..22c576683a0108 100644 --- a/packages/components/src/popover/test/index.tsx +++ b/packages/components/src/popover/test/index.tsx @@ -2,6 +2,7 @@ * External dependencies */ import { render, screen, waitFor, getByText } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; import type { CSSProperties } from 'react'; /** @@ -36,6 +37,28 @@ type PlacementToInitialTranslationTuple = [ CSSProperties[ 'translate' ], ]; +beforeAll( () => { + // This mock is necessary because deep in the weeds, `useConstrained` relies + // on `focusable` to return a list of DOM elements that can be focused. Part + // of this process involves checking that an element has an intrinsic size, + // which will always fail in JSDom. + // + // https://github.com/WordPress/gutenberg/blob/trunk/packages/dom/src/focusable.js#L55-L61 + jest.spyOn( + HTMLElement.prototype, + 'offsetHeight', + 'get' + ).mockImplementation( function getOffsetHeight( this: HTMLElement ) { + // The `1` returned here is somewhat arbitrary – it just needs to be a + // non-zero integer. + return 1; + } ); +} ); + +afterAll( () => { + jest.restoreAllMocks(); +} ); + // There's no matching `placement` for 'middle center' positions, // fallback to 'bottom' (same as `floating-ui`'s default.) const FALLBACK_FOR_MIDDLE_CENTER_POSITIONS = 'bottom'; @@ -188,6 +211,233 @@ describe( 'Popover', () => { expect( document.body ).toHaveFocus(); } ); } ); + + describe( 'tab constraint behavior', () => { + // `constrainTabbing` is implicitly controlled by `focusOnMount`. + // By default, when `focusOnMount` is false, `constrainTabbing` will + // also be false; otherwise, `constrainTabbing` will be true. + + const setup = async ( + props?: Partial< React.ComponentProps< typeof Popover > > + ) => { + const user = await userEvent.setup(); + const view = render( + + + + + + ); + + const popover = screen.getByTestId( 'popover-element' ); + await waitFor( () => expect( popover ).toBeVisible() ); + + const [ firstButton, secondButton, thirdButton ] = + screen.getAllByRole( 'button' ); + + return { + ...view, + popover, + firstButton, + secondButton, + thirdButton, + user, + }; + }; + + // Note: due to an issue in testing-library/user-event [1], the + // tests for constrained tabbing fail. + // [1]: https://github.com/testing-library/user-event/issues/1188 + // + // eslint-disable-next-line jest/no-disabled-tests + describe.skip( 'constrains tabbing', () => { + test( 'by default', async () => { + // The default value for `focusOnMount` is 'firstElement', + // which means the default value for `constrainTabbing` is + // 'true'. + + const { user, firstButton, secondButton, thirdButton } = + await setup(); + + await waitFor( () => expect( firstButton ).toHaveFocus() ); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( thirdButton ).toHaveFocus(); + } ); + + test( 'when `focusOnMount` is true', async () => { + const { + user, + popover, + firstButton, + secondButton, + thirdButton, + } = await setup( { focusOnMount: true } ); + + expect( popover ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( thirdButton ).toHaveFocus(); + } ); + + test( 'when `focusOnMount` is "firstElement"', async () => { + const { user, firstButton, secondButton, thirdButton } = + await setup( { focusOnMount: 'firstElement' } ); + + await waitFor( () => expect( firstButton ).toHaveFocus() ); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( thirdButton ).toHaveFocus(); + } ); + + test( 'when `focusOnMount` is false if `constrainTabbing` is true', async () => { + const { + user, + baseElement, + firstButton, + secondButton, + thirdButton, + } = await setup( { + focusOnMount: false, + constrainTabbing: true, + } ); + + expect( baseElement ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( thirdButton ).toHaveFocus(); + } ); + } ); + + describe( 'does not constrain tabbing', () => { + test( 'when `constrainTabbing` is false', async () => { + // The default value for `focusOnMount` is 'firstElement', + // which means the default value for `constrainTabbing` is + // 'true', but the provided value should override this. + + const { + user, + baseElement, + firstButton, + secondButton, + thirdButton, + } = await setup( { constrainTabbing: false } ); + + await waitFor( () => expect( firstButton ).toHaveFocus() ); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( baseElement ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( baseElement ).toHaveFocus(); + } ); + + test( 'when `focusOnMount` is false', async () => { + const { + user, + baseElement, + firstButton, + secondButton, + thirdButton, + } = await setup( { focusOnMount: false } ); + + expect( baseElement ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( baseElement ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( baseElement ).toHaveFocus(); + } ); + + test( 'when `focusOnMount` is true if `constrainTabbing` is false', async () => { + const { + user, + baseElement, + popover, + firstButton, + secondButton, + thirdButton, + } = await setup( { + focusOnMount: true, + constrainTabbing: false, + } ); + + expect( popover ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( baseElement ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( baseElement ).toHaveFocus(); + } ); + + test( 'when `focusOnMount` is "firstElement" if `constrainTabbing` is false', async () => { + const { + user, + baseElement, + firstButton, + secondButton, + thirdButton, + } = await setup( { + focusOnMount: 'firstElement', + constrainTabbing: false, + } ); + + await waitFor( () => expect( firstButton ).toHaveFocus() ); + await user.tab(); + expect( secondButton ).toHaveFocus(); + await user.tab(); + expect( thirdButton ).toHaveFocus(); + await user.tab(); + expect( baseElement ).toHaveFocus(); + await user.tab(); + expect( firstButton ).toHaveFocus(); + await user.tab( { shift: true } ); + expect( baseElement ).toHaveFocus(); + } ); + } ); + } ); } ); describe( 'Slot outside iframe', () => { diff --git a/packages/components/src/popover/types.ts b/packages/components/src/popover/types.ts index c4250b22ba8349..427f4afb81bfb9 100644 --- a/packages/components/src/popover/types.ts +++ b/packages/components/src/popover/types.ts @@ -65,6 +65,15 @@ export type PopoverProps = { * @default true */ flip?: boolean; + /** + * Determines whether tabbing is constrained to within the popover, + * preventing keyboard focus from leaving the popover content without + * explicit focus elswhere, or whether the popover remains part of the wider + * tab order. If no value is passed, it will be derived from `focusOnMount`. + * + * @default `focusOnMount` !== false + */ + constrainTabbing?: boolean; /** * By default, the _first tabbable element_ in the popover will receive focus * when it mounts. This is the same as setting this prop to `"firstElement"`. diff --git a/packages/compose/src/hooks/use-constrained-tabbing/index.js b/packages/compose/src/hooks/use-constrained-tabbing/index.js index 97b8a2a0a5eb52..94e0080b211cd9 100644 --- a/packages/compose/src/hooks/use-constrained-tabbing/index.js +++ b/packages/compose/src/hooks/use-constrained-tabbing/index.js @@ -1,7 +1,6 @@ /** * WordPress dependencies */ -import { TAB } from '@wordpress/keycodes'; import { focus } from '@wordpress/dom'; /** @@ -33,9 +32,9 @@ import useRefEffect from '../use-ref-effect'; function useConstrainedTabbing() { return useRefEffect( ( /** @type {HTMLElement} */ node ) => { function onKeyDown( /** @type {KeyboardEvent} */ event ) { - const { keyCode, shiftKey, target } = event; + const { key, shiftKey, target } = event; - if ( keyCode !== TAB ) { + if ( key !== 'Tab' ) { return; } diff --git a/packages/compose/src/hooks/use-dialog/index.ts b/packages/compose/src/hooks/use-dialog/index.ts index 66974b60e07035..1b517478fa0bd2 100644 --- a/packages/compose/src/hooks/use-dialog/index.ts +++ b/packages/compose/src/hooks/use-dialog/index.ts @@ -19,7 +19,26 @@ import useFocusOutside from '../use-focus-outside'; import useMergeRefs from '../use-merge-refs'; type DialogOptions = { + /** + * Determines whether focus should be automatically moved to the popover + * when it mounts. `false` causes no focus shift, `true` causes the popover + * itself to gain focus, and `firstElement` focuses the first focusable + * element within the popover. + * + * @default 'firstElement' + */ focusOnMount?: Parameters< typeof useFocusOnMount >[ 0 ]; + /** + * Determines whether tabbing is constrained to within the popover, + * preventing keyboard focus from leaving the popover content without + * explicit focus elsewhere, or whether the popover remains part of the + * wider tab order. + * If no value is passed, it will be derived from `focusOnMount`. + * + * @see focusOnMount + * @default `focusOnMount` !== false + */ + constrainTabbing?: boolean; onClose?: () => void; /** * Use the `onClose` prop instead. @@ -48,6 +67,7 @@ type useDialogReturn = [ */ function useDialog( options: DialogOptions ): useDialogReturn { const currentOptions = useRef< DialogOptions | undefined >(); + const { constrainTabbing = options.focusOnMount !== false } = options; useEffect( () => { currentOptions.current = options; }, Object.values( options ) ); @@ -83,7 +103,7 @@ function useDialog( options: DialogOptions ): useDialogReturn { return [ useMergeRefs( [ - options.focusOnMount !== false ? constrainedTabbingRef : null, + constrainTabbing ? constrainedTabbingRef : null, options.focusOnMount !== false ? focusReturnRef : null, options.focusOnMount !== false ? focusOnMountRef : null, closeOnEscapeRef,