From e90926f679149cd836ca9c6de09534722eabb5e6 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 25 Aug 2022 13:57:54 +0200 Subject: [PATCH 1/9] NavigableContainer: use code instead of keyCode for keyboard events --- .../src/navigable-container/menu.js | 25 +++++++++++-------- .../src/navigable-container/tabbable.js | 5 ++-- 2 files changed, 16 insertions(+), 14 deletions(-) diff --git a/packages/components/src/navigable-container/menu.js b/packages/components/src/navigable-container/menu.js index dbc6c695b2a97..cf19c23adcb9d 100644 --- a/packages/components/src/navigable-container/menu.js +++ b/packages/components/src/navigable-container/menu.js @@ -4,7 +4,6 @@ * WordPress dependencies */ import { forwardRef } from '@wordpress/element'; -import { UP, DOWN, LEFT, RIGHT } from '@wordpress/keycodes'; /** * Internal dependencies @@ -16,26 +15,30 @@ export function NavigableMenu( ref ) { const eventToOffset = ( evt ) => { - const { keyCode } = evt; + const { code } = evt; - let next = [ DOWN ]; - let previous = [ UP ]; + let next = [ 'ArrowDown' ]; + let previous = [ 'ArrowUp' ]; if ( orientation === 'horizontal' ) { - next = [ RIGHT ]; - previous = [ LEFT ]; + next = [ 'ArrowRight' ]; + previous = [ 'ArrowLeft' ]; } if ( orientation === 'both' ) { - next = [ RIGHT, DOWN ]; - previous = [ LEFT, UP ]; + next = [ 'ArrowRight', 'ArrowDown' ]; + previous = [ 'ArrowLeft', 'ArrowUp' ]; } - if ( next.includes( keyCode ) ) { + if ( next.includes( code ) ) { return 1; - } else if ( previous.includes( keyCode ) ) { + } else if ( previous.includes( code ) ) { return -1; - } else if ( [ DOWN, UP, LEFT, RIGHT ].includes( keyCode ) ) { + } else if ( + [ 'ArrowDown', 'ArrowUp', 'ArrowLeft', 'ArrowRight' ].includes( + code + ) + ) { // Key press should be handled, e.g. have event propagation and // default behavior handled by NavigableContainer but not result // in an offset. diff --git a/packages/components/src/navigable-container/tabbable.js b/packages/components/src/navigable-container/tabbable.js index b6b1a89fceb4d..8f38970bf0670 100644 --- a/packages/components/src/navigable-container/tabbable.js +++ b/packages/components/src/navigable-container/tabbable.js @@ -3,7 +3,6 @@ * WordPress dependencies */ import { forwardRef } from '@wordpress/element'; -import { TAB } from '@wordpress/keycodes'; /** * Internal dependencies @@ -12,8 +11,8 @@ import NavigableContainer from './container'; export function TabbableContainer( { eventToOffset, ...props }, ref ) { const innerEventToOffset = ( evt ) => { - const { keyCode, shiftKey } = evt; - if ( TAB === keyCode ) { + const { code, shiftKey } = evt; + if ( 'Tab' === code ) { return shiftKey ? -1 : 1; } From 63dc6bea60b204c8500df8249e9e69244b4585dc Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 26 Aug 2022 12:02:12 +0200 Subject: [PATCH 2/9] Add Storybook examples --- .../stories/navigable-menu.js | 50 +++++++++++++++++++ .../stories/tabbable-container.js | 40 +++++++++++++++ 2 files changed, 90 insertions(+) create mode 100644 packages/components/src/navigable-container/stories/navigable-menu.js create mode 100644 packages/components/src/navigable-container/stories/tabbable-container.js diff --git a/packages/components/src/navigable-container/stories/navigable-menu.js b/packages/components/src/navigable-container/stories/navigable-menu.js new file mode 100644 index 0000000000000..fa016ff9f04c3 --- /dev/null +++ b/packages/components/src/navigable-container/stories/navigable-menu.js @@ -0,0 +1,50 @@ +/** + * Internal dependencies + */ +import { NavigableMenu } from '..'; + +export default { + title: 'Components/NavigableMenu', + component: NavigableMenu, + argTypes: { + children: { type: null }, + cycle: { + type: 'boolean', + }, + onNavigate: { action: 'onNavigate' }, + orientation: { + options: [ 'horizontal', 'vertical' ], + control: { type: 'radio' }, + }, + }, +}; + +export const Default = ( args ) => { + return ( + <> + + + + + + + + + + + + ); +}; diff --git a/packages/components/src/navigable-container/stories/tabbable-container.js b/packages/components/src/navigable-container/stories/tabbable-container.js new file mode 100644 index 0000000000000..3e12825189f1a --- /dev/null +++ b/packages/components/src/navigable-container/stories/tabbable-container.js @@ -0,0 +1,40 @@ +/** + * Internal dependencies + */ +import { TabbableContainer } from '..'; + +export default { + title: 'Components/TabbableContainer', + component: TabbableContainer, + argTypes: { + children: { type: null }, + cycle: { + type: 'boolean', + }, + onNavigate: { action: 'onNavigate' }, + }, +}; + +export const Default = ( args ) => { + return ( + <> + + + + + + + + + + + + ); +}; From 8b728314d7f32ed896386e9ae9443881e3dda3a7 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 26 Aug 2022 16:57:51 +0200 Subject: [PATCH 3/9] Rewrite unit tests using modern RTL and user-event --- .../src/navigable-container/test/index.js | 464 ++++++++++++++++++ .../src/navigable-container/test/menu.js | 310 ------------ .../src/navigable-container/test/tabbable.js | 158 ------ 3 files changed, 464 insertions(+), 468 deletions(-) create mode 100644 packages/components/src/navigable-container/test/index.js delete mode 100644 packages/components/src/navigable-container/test/menu.js delete mode 100644 packages/components/src/navigable-container/test/tabbable.js diff --git a/packages/components/src/navigable-container/test/index.js b/packages/components/src/navigable-container/test/index.js new file mode 100644 index 0000000000000..bc9e5e94bed3f --- /dev/null +++ b/packages/components/src/navigable-container/test/index.js @@ -0,0 +1,464 @@ +/** + * External dependencies + */ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +/** + * Internal dependencies + */ +import { NavigableMenu } from '../menu'; +import { TabbableContainer } from '../tabbable'; + +const NavigableMenuTestCase = ( props ) => ( + + + + Item 2 (not tabbable) + +

I can not be focused

+ + Item 4 + +); + +const TabbableContainerTestCase = ( props ) => ( + + + + Item 2 (not tabbable) + + + Item 3 + +

I can not be tabbed

+ + Item 4 + +); + +const getNavigableMenuFocusables = () => [ + screen.getByRole( 'button', { name: 'Item 1' } ), + screen.getByText( 'Item 2 (not tabbable)' ), + screen.getByRole( 'link', { name: 'Item 4' } ), +]; + +const getTabbableContainerTabbables = () => [ + screen.getByRole( 'button', { name: 'Item 1' } ), + screen.getByText( 'Item 3' ), + screen.getByRole( 'link', { name: 'Item 4' } ), +]; + +const originalGetClientRects = window.HTMLElement.prototype.getClientRects; + +describe( 'NavigableMenu and TabbableContainer', () => { + beforeAll( () => { + window.HTMLElement.prototype.getClientRects = function () { + return [ 'trick-jsdom-into-having-size-for-element-rect' ]; + }; + } ); + + afterAll( () => { + window.HTMLElement.prototype.getClientRects = originalGetClientRects; + } ); + + describe( 'NavigableMenu', () => { + it( 'moves focus on its focusable children by using the up/down arrow keys', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + render( ); + + const focusables = getNavigableMenuFocusables(); + + // Move focus to first item. + await user.tab(); + expect( focusables[ 0 ] ).toHaveFocus(); + + // By default, up/down arrows are used to navigate. + await user.keyboard( '[ArrowDown]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 1, + focusables[ 1 ] + ); + + await user.keyboard( '[ArrowDown]' ); + expect( focusables[ 2 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 2, + focusables[ 2 ] + ); + + await user.keyboard( '[ArrowUp]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 1, + focusables[ 1 ] + ); + + // Left/right arrows don't navigate. + await user.keyboard( '[ArrowLeft]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + + await user.keyboard( '[ArrowRight]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + } ); + + it( 'moves focus on its focusable children by using the left/right arrow keys when the `orientation`prop is set to `horizontal', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + render( + + ); + + const focusables = getNavigableMenuFocusables(); + + // Move focus to first item. + await user.tab(); + expect( focusables[ 0 ] ).toHaveFocus(); + + // When `orientation="horizontal"`, left/right arrows are used to navigate. + await user.keyboard( '[ArrowRight]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 1, + focusables[ 1 ] + ); + + await user.keyboard( '[ArrowRight]' ); + expect( focusables[ 2 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 2, + focusables[ 2 ] + ); + + await user.keyboard( '[ArrowLeft]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 1, + focusables[ 1 ] + ); + + // When `orientation="horizontal"`, up/down arrows don't navigate. + await user.keyboard( '[ArrowUp]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + + await user.keyboard( '[ArrowDown]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + } ); + + it( 'should stop at the edges when the `cycle` prop is set to `false`', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + const { rerender } = render( + + ); + + const focusables = getNavigableMenuFocusables(); + const firstFocusable = focusables[ 0 ]; + const lastFocusableIndex = focusables.length - 1; + const lastFocusable = focusables[ lastFocusableIndex ]; + + // Move focus to first item. + await user.tab(); + expect( firstFocusable ).toHaveFocus(); + + // By default, cycling from first to last and from last to first is allowed. + await user.keyboard( '[ArrowUp]' ); + expect( lastFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastFocusableIndex, + lastFocusable + ); + + await user.keyboard( '[ArrowDown]' ); + expect( firstFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 0, + firstFocusable + ); + + rerender( + + ); + + // With the `cycle` prop set to `false`, cycling is not allowed. + // By default, cycling from first to last and from last to first is allowed. + await user.keyboard( '[ArrowUp]' ); + expect( firstFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + + await user.keyboard( '[ArrowDown][ArrowDown]' ); + expect( lastFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastFocusableIndex, + lastFocusable + ); + + await user.keyboard( '[ArrowDown]' ); + expect( lastFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + } ); + + it( 'stops keydown event propagation when arrow keys are pressed, regardless of the `orientation` prop', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const externalWrapperOnKeyDownSpy = jest.fn(); + + render( + // Disable reason: this is only for test purposes. + // eslint-disable-next-line jsx-a11y/no-static-element-interactions +
+ +
+ ); + + const focusables = getNavigableMenuFocusables(); + + // Move focus to first item + await user.tab(); + expect( focusables[ 0 ] ).toHaveFocus(); + + await user.keyboard( '[Space]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + + await user.keyboard( + '[ArrowDown][ArrowUp][ArrowLeft][ArrowRight]' + ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + + await user.keyboard( '[Escape]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 2 ); + } ); + + it( 'skips its internal logic when the tab key is pressed', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + render( + <> + + + + + ); + + const beforeFocusable = screen.getByRole( 'button', { + name: 'Before menu', + } ); + const internalFocusables = getNavigableMenuFocusables(); + const firstFocusable = internalFocusables[ 0 ]; + const lastFocusableIndex = internalFocusables.length - 1; + const lastFocusable = internalFocusables[ lastFocusableIndex ]; + const afterFocusable = screen.getByRole( 'button', { + name: 'After menu', + } ); + + // The 'tab' key is not handled by the component, which means that elements + // are focused following the standard browser behavior. + await user.tab(); + expect( beforeFocusable ).toHaveFocus(); + + await user.tab(); + expect( firstFocusable ).toHaveFocus(); + + // Note: the second element "internalFocusables" is not focused by default + // by the browser because it has `tabindex="-1"` + + await user.tab(); + expect( lastFocusable ).toHaveFocus(); + + await user.tab(); + expect( afterFocusable ).toHaveFocus(); + + await user.tab( { shift: true } ); + expect( lastFocusable ).toHaveFocus(); + + // Note: the second element "internalFocusables" is not focused by default + // by the browser because it has `tabindex="-1"` + + await user.tab( { shift: true } ); + expect( firstFocusable ).toHaveFocus(); + + await user.tab( { shift: true } ); + expect( beforeFocusable ).toHaveFocus(); + } ); + } ); + + describe( 'TabbableContainer', () => { + it( 'moves focus on its tabbable children by using the tab key', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + render( + + ); + + const tabbables = getTabbableContainerTabbables(); + + // Move focus to first item. + await user.tab(); + expect( tabbables[ 0 ] ).toHaveFocus(); + + await user.tab(); + expect( tabbables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 1, + tabbables[ 1 ] + ); + + await user.tab(); + expect( tabbables[ 2 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 2, + tabbables[ 2 ] + ); + + await user.tab( { shift: true } ); + expect( tabbables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 1, + tabbables[ 1 ] + ); + } ); + + it( 'should stop at the edges when the `cycle` prop is set to `false`', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + const { rerender } = render( + + ); + + const tabbables = getTabbableContainerTabbables(); + const firstTabbable = tabbables[ 0 ]; + const lastTabbableIndex = tabbables.length - 1; + const lastTabbable = tabbables[ lastTabbableIndex ]; + + // Move focus to first item. + await user.tab(); + expect( firstTabbable ).toHaveFocus(); + + // By default, cycling from first to last and from last to first is allowed. + await user.tab( { shift: true } ); + expect( lastTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastTabbableIndex, + lastTabbable + ); + + await user.tab(); + expect( firstTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + 0, + firstTabbable + ); + + rerender( + + ); + + // With the `cycle` prop set to `false`, cycling is not allowed. + // By default, cycling from first to last and from last to first is allowed. + await user.tab( { shift: true } ); + expect( firstTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + + await user.tab(); + await user.tab(); + expect( lastTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastTabbableIndex, + lastTabbable + ); + + await user.tab(); + expect( lastTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + } ); + + it( 'stops keydown event propagation when the tab key is pressed', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const externalWrapperOnKeyDownSpy = jest.fn(); + + render( + // Disable reason: this is only for test purposes. + // eslint-disable-next-line jsx-a11y/no-static-element-interactions +
+ +
+ ); + + const tabbables = getTabbableContainerTabbables(); + + // Move focus to first item + await user.tab(); + expect( tabbables[ 0 ] ).toHaveFocus(); + + await user.keyboard( '[Space]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + + await user.tab(); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + await user.tab( { shift: true } ); + // This extra call is caused by the "shift" key being pressed + // on its own before "tab" + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 2 ); + + await user.keyboard( '[Escape]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 3 ); + } ); + } ); +} ); diff --git a/packages/components/src/navigable-container/test/menu.js b/packages/components/src/navigable-container/test/menu.js deleted file mode 100644 index 64170432cb373..0000000000000 --- a/packages/components/src/navigable-container/test/menu.js +++ /dev/null @@ -1,310 +0,0 @@ -/** - * External dependencies - */ -import { fireEvent, render, screen } from '@testing-library/react'; - -/** - * WordPress dependencies - */ -import { UP, DOWN, LEFT, RIGHT, SPACE } from '@wordpress/keycodes'; - -/** - * Internal dependencies - */ -import { NavigableMenu } from '../menu'; - -function simulateVisible( container, selector ) { - const elements = container.querySelectorAll( selector ); - elements.forEach( ( elem ) => { - elem.getClientRects = () => [ - 'trick-jsdom-into-having-size-for-element-rect', - ]; - } ); -} - -function fireKeyDown( node, keyCode, shiftKey ) { - const interaction = { - stopped: false, - }; - - const event = new window.KeyboardEvent( 'keydown', { - keyCode, - shiftKey, - } ); - event.stopImmediatePropagation = () => { - interaction.stopped = true; - }; - fireEvent( node, event ); - - return interaction; -} - -describe( 'NavigableMenu', () => { - it( 'vertical: should navigate by up and down', () => { - let currentIndex = 0; - const { container } = render( - /* - Disabled because of our rule restricting literal IDs, preferring - `withInstanceId`. In this case, it's fine to use literal IDs. - */ - /* eslint-disable no-restricted-syntax */ - ( currentIndex = index ) } - > - - One - - - Two - - - - Deep - - - - Three - - - /* eslint-enable no-restricted-syntax */ - ); - - simulateVisible( container, '*' ); - - container.querySelector( '#btn1' ).focus(); - - // Navigate options. - function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) { - const interaction = fireKeyDown( - screen.getByRole( 'menu' ), - keyCode, - false - ); - expect( currentIndex ).toBe( expectedActiveIndex ); - expect( interaction.stopped ).toBe( expectedStop ); - } - - assertKeyDown( DOWN, 1, true ); - assertKeyDown( DOWN, 2, true ); - assertKeyDown( DOWN, 3, true ); - assertKeyDown( DOWN, 0, true ); - assertKeyDown( UP, 3, true ); - assertKeyDown( UP, 2, true ); - assertKeyDown( UP, 1, true ); - assertKeyDown( UP, 0, true ); - assertKeyDown( LEFT, 0, true ); - assertKeyDown( RIGHT, 0, true ); - assertKeyDown( SPACE, 0, false ); - } ); - - it( 'vertical: should navigate by up and down, and stop at edges', () => { - let currentIndex = 0; - const { container } = render( - /* - Disabled because of our rule restricting literal IDs, preferring - `withInstanceId`. In this case, it's fine to use literal IDs. - */ - /* eslint-disable no-restricted-syntax */ - ( currentIndex = index ) } - > - - One - - - Two - - - Three - - - /* eslint-enable no-restricted-syntax */ - ); - - simulateVisible( container, '*' ); - - container.querySelector( '#btn1' ).focus(); - - // Navigate options. - function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) { - const interaction = fireKeyDown( - screen.getByRole( 'menu' ), - keyCode, - false - ); - expect( currentIndex ).toBe( expectedActiveIndex ); - expect( interaction.stopped ).toBe( expectedStop ); - } - - assertKeyDown( DOWN, 1, true ); - assertKeyDown( DOWN, 2, true ); - assertKeyDown( DOWN, 2, true ); - assertKeyDown( UP, 1, true ); - assertKeyDown( UP, 0, true ); - assertKeyDown( UP, 0, true ); - assertKeyDown( LEFT, 0, true ); - assertKeyDown( RIGHT, 0, true ); - assertKeyDown( SPACE, 0, false ); - } ); - - it( 'horizontal: should navigate by left and right', () => { - let currentIndex = 0; - const { container } = render( - /* - Disabled because of our rule restricting literal IDs, preferring - `withInstanceId`. In this case, it's fine to use literal IDs. - */ - /* eslint-disable no-restricted-syntax */ - ( currentIndex = index ) } - > - - One - - - Two - - - - Deep - - - - Three - - - /* eslint-enable no-restricted-syntax */ - ); - - simulateVisible( container, '*' ); - - container.querySelector( '#btn1' ).focus(); - - // Navigate options. - function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) { - const interaction = fireKeyDown( - screen.getByRole( 'menu' ), - keyCode, - false - ); - expect( currentIndex ).toBe( expectedActiveIndex ); - expect( interaction.stopped ).toBe( expectedStop ); - } - - assertKeyDown( RIGHT, 1, true ); - assertKeyDown( RIGHT, 2, true ); - assertKeyDown( RIGHT, 3, true ); - assertKeyDown( RIGHT, 0, true ); - assertKeyDown( LEFT, 3, true ); - assertKeyDown( LEFT, 2, true ); - assertKeyDown( LEFT, 1, true ); - assertKeyDown( LEFT, 0, true ); - assertKeyDown( UP, 0, true ); - assertKeyDown( DOWN, 0, true ); - assertKeyDown( SPACE, 0, false ); - } ); - - it( 'horizontal: should navigate by left and right, and stop at edges', () => { - let currentIndex = 0; - const { container } = render( - /* - Disabled because of our rule restricting literal IDs, preferring - `withInstanceId`. In this case, it's fine to use literal IDs. - */ - /* eslint-disable no-restricted-syntax */ - ( currentIndex = index ) } - > - - One - - - Two - - - Three - - - /* eslint-enable no-restricted-syntax */ - ); - - simulateVisible( container, '*' ); - - container.querySelector( '#btn1' ).focus(); - - // Navigate options. - function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) { - const interaction = fireKeyDown( - screen.getByRole( 'menu' ), - keyCode, - false - ); - expect( currentIndex ).toBe( expectedActiveIndex ); - expect( interaction.stopped ).toBe( expectedStop ); - } - - assertKeyDown( RIGHT, 1, true ); - assertKeyDown( RIGHT, 2, true ); - assertKeyDown( RIGHT, 2, true ); - assertKeyDown( LEFT, 1, true ); - assertKeyDown( LEFT, 0, true ); - assertKeyDown( LEFT, 0, true ); - assertKeyDown( DOWN, 0, true ); - assertKeyDown( UP, 0, true ); - assertKeyDown( SPACE, 0, false ); - } ); - - it( 'both: should navigate by up/down and left/right', () => { - let currentIndex = 0; - const { container } = render( - /* - Disabled because of our rule restricting literal IDs, preferring - `withInstanceId`. In this case, it's fine to use literal IDs. - */ - /* eslint-disable no-restricted-syntax */ - ( currentIndex = index ) } - > - - - - - /* eslint-enable no-restricted-syntax */ - ); - - simulateVisible( container, '*' ); - - container.querySelector( '#btn1' ).focus(); - - // Navigate options. - function assertKeyDown( keyCode, expectedActiveIndex, expectedStop ) { - const interaction = fireKeyDown( - screen.getByRole( 'menu' ), - keyCode - ); - expect( currentIndex ).toBe( expectedActiveIndex ); - expect( interaction.stopped ).toBe( expectedStop ); - } - - assertKeyDown( DOWN, 1, true ); - assertKeyDown( DOWN, 2, true ); - assertKeyDown( DOWN, 0, true ); - assertKeyDown( RIGHT, 1, true ); - assertKeyDown( RIGHT, 2, true ); - assertKeyDown( RIGHT, 0, true ); - assertKeyDown( UP, 2, true ); - assertKeyDown( UP, 1, true ); - assertKeyDown( UP, 0, true ); - assertKeyDown( LEFT, 2, true ); - assertKeyDown( LEFT, 1, true ); - assertKeyDown( LEFT, 0, true ); - assertKeyDown( SPACE, 0, false ); - } ); -} ); diff --git a/packages/components/src/navigable-container/test/tabbable.js b/packages/components/src/navigable-container/test/tabbable.js deleted file mode 100644 index e8b9a5b568cc6..0000000000000 --- a/packages/components/src/navigable-container/test/tabbable.js +++ /dev/null @@ -1,158 +0,0 @@ -/** - * External dependencies - */ -import { fireEvent, render } from '@testing-library/react'; - -/** - * WordPress dependencies - */ -import { TAB, SPACE } from '@wordpress/keycodes'; - -/** - * Internal dependencies - */ -import { TabbableContainer } from '../tabbable'; - -function simulateVisible( container, selector ) { - const elements = container.querySelectorAll( selector ); - elements.forEach( ( elem ) => { - elem.getClientRects = () => [ - 'trick-jsdom-into-having-size-for-element-rect', - ]; - } ); -} - -function fireKeyDown( node, keyCode, shiftKey ) { - const interaction = { - stopped: false, - }; - - const event = new window.KeyboardEvent( 'keydown', { - keyCode, - shiftKey, - } ); - event.stopImmediatePropagation = () => { - interaction.stopped = true; - }; - fireEvent( node, event ); - - return interaction; -} - -describe( 'TabbableContainer', () => { - it( 'should navigate by keypresses', () => { - let currentIndex = 0; - const { container } = render( - /* - Disabled because of our rule restricting literal IDs, preferring - `withInstanceId`. In this case, it's fine to use literal IDs. - */ - /* eslint-disable no-restricted-syntax */ - ( currentIndex = index ) } - > -
- Section One -
-
- Section Two -
-
-
- Section to not skip -
-
-
- Section Three -
- - /* eslint-enable no-restricted-syntax */ - ); - - simulateVisible( container, '*' ); - - container.querySelector( '#section1' ).focus(); - - // Navigate options. - function assertKeyDown( - keyCode, - shiftKey, - expectedActiveIndex, - expectedStop - ) { - const interaction = fireKeyDown( - container.querySelector( '.wrapper' ), - keyCode, - shiftKey - ); - expect( currentIndex ).toBe( expectedActiveIndex ); - expect( interaction.stopped ).toBe( expectedStop ); - } - - assertKeyDown( TAB, false, 1, true ); - assertKeyDown( TAB, false, 2, true ); - assertKeyDown( TAB, false, 3, true ); - assertKeyDown( TAB, false, 0, true ); - assertKeyDown( TAB, true, 3, true ); - assertKeyDown( TAB, true, 2, true ); - assertKeyDown( TAB, true, 1, true ); - assertKeyDown( TAB, true, 0, true ); - assertKeyDown( SPACE, false, 0, false ); - } ); - - it( 'should navigate by keypresses and stop at edges', () => { - let currentIndex = 0; - const { container } = render( - /* - Disabled because of our rule restricting literal IDs, preferring - `withInstanceId`. In this case, it's fine to use literal IDs. - */ - /* eslint-disable no-restricted-syntax */ - ( currentIndex = index ) } - > -
- Section One -
-
- Section Two -
-
- Section Three -
- - /* eslint-enable no-restricted-syntax */ - ); - - simulateVisible( container, '*' ); - - container.querySelector( '#section1' ).focus(); - - // Navigate options. - function assertKeyDown( - keyCode, - shiftKey, - expectedActiveIndex, - expectedStop - ) { - const interaction = fireKeyDown( - container.querySelector( '.wrapper' ), - keyCode, - shiftKey - ); - expect( currentIndex ).toBe( expectedActiveIndex ); - expect( interaction.stopped ).toBe( expectedStop ); - } - - assertKeyDown( TAB, false, 1, true ); - assertKeyDown( TAB, false, 2, true ); - assertKeyDown( TAB, false, 2, true ); - assertKeyDown( TAB, true, 1, true ); - assertKeyDown( TAB, true, 0, true ); - assertKeyDown( TAB, true, 0, true ); - assertKeyDown( SPACE, false, 0, false ); - } ); -} ); From 5cbbc568ddaa21dc27a74061038c266deddc2069 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 26 Aug 2022 17:27:34 +0200 Subject: [PATCH 4/9] TabbableContainer: fix bug where pressing tab in browser environments would move the focus by two tabbable elements (instead of one) --- packages/components/src/navigable-container/container.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/packages/components/src/navigable-container/container.js b/packages/components/src/navigable-container/container.js index 13e7455abddb1..6c9de468f0c59 100644 --- a/packages/components/src/navigable-container/container.js +++ b/packages/components/src/navigable-container/container.js @@ -99,7 +99,14 @@ class NavigableContainer extends Component { // 'handling' the event, as voiceover will try to use arrow keys // for highlighting text. const targetRole = event.target.getAttribute( 'role' ); - if ( MENU_ITEM_ROLES.includes( targetRole ) ) { + const targetHasMenuItemRole = + MENU_ITEM_ROLES.includes( targetRole ); + + // `preventDefault()` on tab to avoid having the browser move the focus + // after this component has already moved it. + const isTab = event.code === 'Tab'; + + if ( targetHasMenuItemRole || isTab ) { event.preventDefault(); } } From b7f33e9bd4ec003ac4b56198b1e24d0393b06b12 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 26 Aug 2022 17:29:44 +0200 Subject: [PATCH 5/9] Add comment explaining the reason for a mock --- packages/components/src/navigable-container/test/index.js | 3 +++ 1 file changed, 3 insertions(+) diff --git a/packages/components/src/navigable-container/test/index.js b/packages/components/src/navigable-container/test/index.js index bc9e5e94bed3f..401d2ed5e19b2 100644 --- a/packages/components/src/navigable-container/test/index.js +++ b/packages/components/src/navigable-container/test/index.js @@ -53,6 +53,9 @@ const originalGetClientRects = window.HTMLElement.prototype.getClientRects; describe( 'NavigableMenu and TabbableContainer', () => { beforeAll( () => { + // Mocking `getClientRects()` is necessary to pass a check performed by + // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions + // from the `@wordpress/dom` package. window.HTMLElement.prototype.getClientRects = function () { return [ 'trick-jsdom-into-having-size-for-element-rect' ]; }; From 24001b571863c398a5878d1cd9212069822d439a Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Fri, 26 Aug 2022 17:31:30 +0200 Subject: [PATCH 6/9] CHANGELOG --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index 0e60b4a23f27b..3c953d51fe830 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -14,6 +14,7 @@ - `Guide`: use `code` instead of `keyCode` for keyboard events ([#43604](https://github.com/WordPress/gutenberg/pull/43604/)). - `Navigation`: use `code` instead of `keyCode` for keyboard events ([#43644](https://github.com/WordPress/gutenberg/pull/43644/)). - `ComboboxControl`: Add unit tests ([#42403](https://github.com/WordPress/gutenberg/pull/42403)). +- `NavigableContainer`: use `code` instead of `keyCode` for keyboard events, rewrite tests using RTL and `user-event` ([#43606](https://github.com/WordPress/gutenberg/pull/43606/)). ## 20.0.0 (2022-08-24) From 37be0d4080f76b7046218c9caa5e83f177305417 Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 31 Aug 2022 12:19:25 +0200 Subject: [PATCH 7/9] Improve Storybook example with less tabbable elements --- .../stories/navigable-menu.js | 19 +++++++++---------- 1 file changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/components/src/navigable-container/stories/navigable-menu.js b/packages/components/src/navigable-container/stories/navigable-menu.js index fa016ff9f04c3..af9b08fd9b032 100644 --- a/packages/components/src/navigable-container/stories/navigable-menu.js +++ b/packages/components/src/navigable-container/stories/navigable-menu.js @@ -31,18 +31,17 @@ export const Default = ( args ) => { border: '1px solid black', } } > - - +
Item 1 (non-tabbable, non-focusable)
+ - - - + + Item 4 (non-tabbable, non-focusable) + +
+ Item 5 (tabbable, focusable) +
From d1455f7908f70b826b4ca01dd6eed654f9bdbc5b Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 31 Aug 2022 16:17:59 +0200 Subject: [PATCH 8/9] Split unit test --- .../src/navigable-container/test/index.js | 467 ------------------ .../test/navigable-menu.js | 277 +++++++++++ .../test/tababble-container.js | 175 +++++++ 3 files changed, 452 insertions(+), 467 deletions(-) delete mode 100644 packages/components/src/navigable-container/test/index.js create mode 100644 packages/components/src/navigable-container/test/navigable-menu.js create mode 100644 packages/components/src/navigable-container/test/tababble-container.js diff --git a/packages/components/src/navigable-container/test/index.js b/packages/components/src/navigable-container/test/index.js deleted file mode 100644 index 401d2ed5e19b2..0000000000000 --- a/packages/components/src/navigable-container/test/index.js +++ /dev/null @@ -1,467 +0,0 @@ -/** - * External dependencies - */ -import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; - -/** - * Internal dependencies - */ -import { NavigableMenu } from '../menu'; -import { TabbableContainer } from '../tabbable'; - -const NavigableMenuTestCase = ( props ) => ( - - - - Item 2 (not tabbable) - -

I can not be focused

- - Item 4 - -); - -const TabbableContainerTestCase = ( props ) => ( - - - - Item 2 (not tabbable) - - - Item 3 - -

I can not be tabbed

- - Item 4 - -); - -const getNavigableMenuFocusables = () => [ - screen.getByRole( 'button', { name: 'Item 1' } ), - screen.getByText( 'Item 2 (not tabbable)' ), - screen.getByRole( 'link', { name: 'Item 4' } ), -]; - -const getTabbableContainerTabbables = () => [ - screen.getByRole( 'button', { name: 'Item 1' } ), - screen.getByText( 'Item 3' ), - screen.getByRole( 'link', { name: 'Item 4' } ), -]; - -const originalGetClientRects = window.HTMLElement.prototype.getClientRects; - -describe( 'NavigableMenu and TabbableContainer', () => { - beforeAll( () => { - // Mocking `getClientRects()` is necessary to pass a check performed by - // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions - // from the `@wordpress/dom` package. - window.HTMLElement.prototype.getClientRects = function () { - return [ 'trick-jsdom-into-having-size-for-element-rect' ]; - }; - } ); - - afterAll( () => { - window.HTMLElement.prototype.getClientRects = originalGetClientRects; - } ); - - describe( 'NavigableMenu', () => { - it( 'moves focus on its focusable children by using the up/down arrow keys', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - const onNavigateSpy = jest.fn(); - - render( ); - - const focusables = getNavigableMenuFocusables(); - - // Move focus to first item. - await user.tab(); - expect( focusables[ 0 ] ).toHaveFocus(); - - // By default, up/down arrows are used to navigate. - await user.keyboard( '[ArrowDown]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 1, - focusables[ 1 ] - ); - - await user.keyboard( '[ArrowDown]' ); - expect( focusables[ 2 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 2, - focusables[ 2 ] - ); - - await user.keyboard( '[ArrowUp]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 1, - focusables[ 1 ] - ); - - // Left/right arrows don't navigate. - await user.keyboard( '[ArrowLeft]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); - - await user.keyboard( '[ArrowRight]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); - } ); - - it( 'moves focus on its focusable children by using the left/right arrow keys when the `orientation`prop is set to `horizontal', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - const onNavigateSpy = jest.fn(); - - render( - - ); - - const focusables = getNavigableMenuFocusables(); - - // Move focus to first item. - await user.tab(); - expect( focusables[ 0 ] ).toHaveFocus(); - - // When `orientation="horizontal"`, left/right arrows are used to navigate. - await user.keyboard( '[ArrowRight]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 1, - focusables[ 1 ] - ); - - await user.keyboard( '[ArrowRight]' ); - expect( focusables[ 2 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 2, - focusables[ 2 ] - ); - - await user.keyboard( '[ArrowLeft]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 1, - focusables[ 1 ] - ); - - // When `orientation="horizontal"`, up/down arrows don't navigate. - await user.keyboard( '[ArrowUp]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); - - await user.keyboard( '[ArrowDown]' ); - expect( focusables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); - } ); - - it( 'should stop at the edges when the `cycle` prop is set to `false`', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - const onNavigateSpy = jest.fn(); - - const { rerender } = render( - - ); - - const focusables = getNavigableMenuFocusables(); - const firstFocusable = focusables[ 0 ]; - const lastFocusableIndex = focusables.length - 1; - const lastFocusable = focusables[ lastFocusableIndex ]; - - // Move focus to first item. - await user.tab(); - expect( firstFocusable ).toHaveFocus(); - - // By default, cycling from first to last and from last to first is allowed. - await user.keyboard( '[ArrowUp]' ); - expect( lastFocusable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - lastFocusableIndex, - lastFocusable - ); - - await user.keyboard( '[ArrowDown]' ); - expect( firstFocusable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 0, - firstFocusable - ); - - rerender( - - ); - - // With the `cycle` prop set to `false`, cycling is not allowed. - // By default, cycling from first to last and from last to first is allowed. - await user.keyboard( '[ArrowUp]' ); - expect( firstFocusable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); - - await user.keyboard( '[ArrowDown][ArrowDown]' ); - expect( lastFocusable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - lastFocusableIndex, - lastFocusable - ); - - await user.keyboard( '[ArrowDown]' ); - expect( lastFocusable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); - } ); - - it( 'stops keydown event propagation when arrow keys are pressed, regardless of the `orientation` prop', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - const externalWrapperOnKeyDownSpy = jest.fn(); - - render( - // Disable reason: this is only for test purposes. - // eslint-disable-next-line jsx-a11y/no-static-element-interactions -
- -
- ); - - const focusables = getNavigableMenuFocusables(); - - // Move focus to first item - await user.tab(); - expect( focusables[ 0 ] ).toHaveFocus(); - - await user.keyboard( '[Space]' ); - expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); - - await user.keyboard( - '[ArrowDown][ArrowUp][ArrowLeft][ArrowRight]' - ); - expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); - - await user.keyboard( '[Escape]' ); - expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 2 ); - } ); - - it( 'skips its internal logic when the tab key is pressed', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - render( - <> - - - - - ); - - const beforeFocusable = screen.getByRole( 'button', { - name: 'Before menu', - } ); - const internalFocusables = getNavigableMenuFocusables(); - const firstFocusable = internalFocusables[ 0 ]; - const lastFocusableIndex = internalFocusables.length - 1; - const lastFocusable = internalFocusables[ lastFocusableIndex ]; - const afterFocusable = screen.getByRole( 'button', { - name: 'After menu', - } ); - - // The 'tab' key is not handled by the component, which means that elements - // are focused following the standard browser behavior. - await user.tab(); - expect( beforeFocusable ).toHaveFocus(); - - await user.tab(); - expect( firstFocusable ).toHaveFocus(); - - // Note: the second element "internalFocusables" is not focused by default - // by the browser because it has `tabindex="-1"` - - await user.tab(); - expect( lastFocusable ).toHaveFocus(); - - await user.tab(); - expect( afterFocusable ).toHaveFocus(); - - await user.tab( { shift: true } ); - expect( lastFocusable ).toHaveFocus(); - - // Note: the second element "internalFocusables" is not focused by default - // by the browser because it has `tabindex="-1"` - - await user.tab( { shift: true } ); - expect( firstFocusable ).toHaveFocus(); - - await user.tab( { shift: true } ); - expect( beforeFocusable ).toHaveFocus(); - } ); - } ); - - describe( 'TabbableContainer', () => { - it( 'moves focus on its tabbable children by using the tab key', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - const onNavigateSpy = jest.fn(); - - render( - - ); - - const tabbables = getTabbableContainerTabbables(); - - // Move focus to first item. - await user.tab(); - expect( tabbables[ 0 ] ).toHaveFocus(); - - await user.tab(); - expect( tabbables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 1, - tabbables[ 1 ] - ); - - await user.tab(); - expect( tabbables[ 2 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 2, - tabbables[ 2 ] - ); - - await user.tab( { shift: true } ); - expect( tabbables[ 1 ] ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 1, - tabbables[ 1 ] - ); - } ); - - it( 'should stop at the edges when the `cycle` prop is set to `false`', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - const onNavigateSpy = jest.fn(); - - const { rerender } = render( - - ); - - const tabbables = getTabbableContainerTabbables(); - const firstTabbable = tabbables[ 0 ]; - const lastTabbableIndex = tabbables.length - 1; - const lastTabbable = tabbables[ lastTabbableIndex ]; - - // Move focus to first item. - await user.tab(); - expect( firstTabbable ).toHaveFocus(); - - // By default, cycling from first to last and from last to first is allowed. - await user.tab( { shift: true } ); - expect( lastTabbable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - lastTabbableIndex, - lastTabbable - ); - - await user.tab(); - expect( firstTabbable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - 0, - firstTabbable - ); - - rerender( - - ); - - // With the `cycle` prop set to `false`, cycling is not allowed. - // By default, cycling from first to last and from last to first is allowed. - await user.tab( { shift: true } ); - expect( firstTabbable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); - - await user.tab(); - await user.tab(); - expect( lastTabbable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); - expect( onNavigateSpy ).toHaveBeenLastCalledWith( - lastTabbableIndex, - lastTabbable - ); - - await user.tab(); - expect( lastTabbable ).toHaveFocus(); - expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); - } ); - - it( 'stops keydown event propagation when the tab key is pressed', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - - const externalWrapperOnKeyDownSpy = jest.fn(); - - render( - // Disable reason: this is only for test purposes. - // eslint-disable-next-line jsx-a11y/no-static-element-interactions -
- -
- ); - - const tabbables = getTabbableContainerTabbables(); - - // Move focus to first item - await user.tab(); - expect( tabbables[ 0 ] ).toHaveFocus(); - - await user.keyboard( '[Space]' ); - expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); - - await user.tab(); - expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); - await user.tab( { shift: true } ); - // This extra call is caused by the "shift" key being pressed - // on its own before "tab" - expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 2 ); - - await user.keyboard( '[Escape]' ); - expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 3 ); - } ); - } ); -} ); diff --git a/packages/components/src/navigable-container/test/navigable-menu.js b/packages/components/src/navigable-container/test/navigable-menu.js new file mode 100644 index 0000000000000..e50925a73c1d3 --- /dev/null +++ b/packages/components/src/navigable-container/test/navigable-menu.js @@ -0,0 +1,277 @@ +/** + * External dependencies + */ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +/** + * Internal dependencies + */ +import { NavigableMenu } from '../menu'; + +const NavigableMenuTestCase = ( props ) => ( + + + + Item 2 (not tabbable) + +

I can not be focused

+ + Item 4 + +); + +const getNavigableMenuFocusables = () => [ + screen.getByRole( 'button', { name: 'Item 1' } ), + screen.getByText( 'Item 2 (not tabbable)' ), + screen.getByRole( 'link', { name: 'Item 4' } ), +]; + +const originalGetClientRects = window.HTMLElement.prototype.getClientRects; + +describe( 'NavigableMenu', () => { + beforeAll( () => { + // Mocking `getClientRects()` is necessary to pass a check performed by + // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions + // from the `@wordpress/dom` package. + window.HTMLElement.prototype.getClientRects = function () { + return [ 'trick-jsdom-into-having-size-for-element-rect' ]; + }; + } ); + + afterAll( () => { + window.HTMLElement.prototype.getClientRects = originalGetClientRects; + } ); + + it( 'moves focus on its focusable children by using the up/down arrow keys', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + render( ); + + const focusables = getNavigableMenuFocusables(); + + // Move focus to first item. + await user.tab(); + expect( focusables[ 0 ] ).toHaveFocus(); + + // By default, up/down arrows are used to navigate. + await user.keyboard( '[ArrowDown]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 1, focusables[ 1 ] ); + + await user.keyboard( '[ArrowDown]' ); + expect( focusables[ 2 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 2, focusables[ 2 ] ); + + await user.keyboard( '[ArrowUp]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 1, focusables[ 1 ] ); + + // Left/right arrows don't navigate. + await user.keyboard( '[ArrowLeft]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + + await user.keyboard( '[ArrowRight]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + } ); + + it( 'moves focus on its focusable children by using the left/right arrow keys when the `orientation`prop is set to `horizontal', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + render( + + ); + + const focusables = getNavigableMenuFocusables(); + + // Move focus to first item. + await user.tab(); + expect( focusables[ 0 ] ).toHaveFocus(); + + // When `orientation="horizontal"`, left/right arrows are used to navigate. + await user.keyboard( '[ArrowRight]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 1, focusables[ 1 ] ); + + await user.keyboard( '[ArrowRight]' ); + expect( focusables[ 2 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 2, focusables[ 2 ] ); + + await user.keyboard( '[ArrowLeft]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 1, focusables[ 1 ] ); + + // When `orientation="horizontal"`, up/down arrows don't navigate. + await user.keyboard( '[ArrowUp]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + + await user.keyboard( '[ArrowDown]' ); + expect( focusables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + } ); + + it( 'should stop at the edges when the `cycle` prop is set to `false`', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + const { rerender } = render( + + ); + + const focusables = getNavigableMenuFocusables(); + const firstFocusable = focusables[ 0 ]; + const lastFocusableIndex = focusables.length - 1; + const lastFocusable = focusables[ lastFocusableIndex ]; + + // Move focus to first item. + await user.tab(); + expect( firstFocusable ).toHaveFocus(); + + // By default, cycling from first to last and from last to first is allowed. + await user.keyboard( '[ArrowUp]' ); + expect( lastFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastFocusableIndex, + lastFocusable + ); + + await user.keyboard( '[ArrowDown]' ); + expect( firstFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 0, firstFocusable ); + + rerender( + + ); + + // With the `cycle` prop set to `false`, cycling is not allowed. + // By default, cycling from first to last and from last to first is allowed. + await user.keyboard( '[ArrowUp]' ); + expect( firstFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + + await user.keyboard( '[ArrowDown][ArrowDown]' ); + expect( lastFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastFocusableIndex, + lastFocusable + ); + + await user.keyboard( '[ArrowDown]' ); + expect( lastFocusable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + } ); + + it( 'stops keydown event propagation when arrow keys are pressed, regardless of the `orientation` prop', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const externalWrapperOnKeyDownSpy = jest.fn(); + + render( + // Disable reason: this is only for test purposes. + // eslint-disable-next-line jsx-a11y/no-static-element-interactions +
+ +
+ ); + + const focusables = getNavigableMenuFocusables(); + + // Move focus to first item + await user.tab(); + expect( focusables[ 0 ] ).toHaveFocus(); + + await user.keyboard( '[Space]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + + await user.keyboard( '[ArrowDown][ArrowUp][ArrowLeft][ArrowRight]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + + await user.keyboard( '[Escape]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 2 ); + } ); + + it( 'skips its internal logic when the tab key is pressed', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + render( + <> + + + + + ); + + const beforeFocusable = screen.getByRole( 'button', { + name: 'Before menu', + } ); + const internalFocusables = getNavigableMenuFocusables(); + const firstFocusable = internalFocusables[ 0 ]; + const lastFocusableIndex = internalFocusables.length - 1; + const lastFocusable = internalFocusables[ lastFocusableIndex ]; + const afterFocusable = screen.getByRole( 'button', { + name: 'After menu', + } ); + + // The 'tab' key is not handled by the component, which means that elements + // are focused following the standard browser behavior. + await user.tab(); + expect( beforeFocusable ).toHaveFocus(); + + await user.tab(); + expect( firstFocusable ).toHaveFocus(); + + // Note: the second element "internalFocusables" is not focused by default + // by the browser because it has `tabindex="-1"` + + await user.tab(); + expect( lastFocusable ).toHaveFocus(); + + await user.tab(); + expect( afterFocusable ).toHaveFocus(); + + await user.tab( { shift: true } ); + expect( lastFocusable ).toHaveFocus(); + + // Note: the second element "internalFocusables" is not focused by default + // by the browser because it has `tabindex="-1"` + + await user.tab( { shift: true } ); + expect( firstFocusable ).toHaveFocus(); + + await user.tab( { shift: true } ); + expect( beforeFocusable ).toHaveFocus(); + } ); +} ); diff --git a/packages/components/src/navigable-container/test/tababble-container.js b/packages/components/src/navigable-container/test/tababble-container.js new file mode 100644 index 0000000000000..593238578792f --- /dev/null +++ b/packages/components/src/navigable-container/test/tababble-container.js @@ -0,0 +1,175 @@ +/** + * External dependencies + */ +import { render, screen } from '@testing-library/react'; +import userEvent from '@testing-library/user-event'; + +/** + * Internal dependencies + */ +import { TabbableContainer } from '../tabbable'; + +const TabbableContainerTestCase = ( props ) => ( + + + + Item 2 (not tabbable) + + + Item 3 + +

I can not be tabbed

+ + Item 4 + +); + +const getTabbableContainerTabbables = () => [ + screen.getByRole( 'button', { name: 'Item 1' } ), + screen.getByText( 'Item 3' ), + screen.getByRole( 'link', { name: 'Item 4' } ), +]; + +const originalGetClientRects = window.HTMLElement.prototype.getClientRects; + +describe( 'TabbableContainer', () => { + beforeAll( () => { + // Mocking `getClientRects()` is necessary to pass a check performed by + // the `focus.tabbable.find()` and by the `focus.focusable.find()` functions + // from the `@wordpress/dom` package. + window.HTMLElement.prototype.getClientRects = function () { + return [ 'trick-jsdom-into-having-size-for-element-rect' ]; + }; + } ); + + afterAll( () => { + window.HTMLElement.prototype.getClientRects = originalGetClientRects; + } ); + + it( 'moves focus on its tabbable children by using the tab key', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + render( ); + + const tabbables = getTabbableContainerTabbables(); + + // Move focus to first item. + await user.tab(); + expect( tabbables[ 0 ] ).toHaveFocus(); + + await user.tab(); + expect( tabbables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 1, tabbables[ 1 ] ); + + await user.tab(); + expect( tabbables[ 2 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 2, tabbables[ 2 ] ); + + await user.tab( { shift: true } ); + expect( tabbables[ 1 ] ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 3 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 1, tabbables[ 1 ] ); + } ); + + it( 'should stop at the edges when the `cycle` prop is set to `false`', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const onNavigateSpy = jest.fn(); + + const { rerender } = render( + + ); + + const tabbables = getTabbableContainerTabbables(); + const firstTabbable = tabbables[ 0 ]; + const lastTabbableIndex = tabbables.length - 1; + const lastTabbable = tabbables[ lastTabbableIndex ]; + + // Move focus to first item. + await user.tab(); + expect( firstTabbable ).toHaveFocus(); + + // By default, cycling from first to last and from last to first is allowed. + await user.tab( { shift: true } ); + expect( lastTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 1 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastTabbableIndex, + lastTabbable + ); + + await user.tab(); + expect( firstTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( 0, firstTabbable ); + + rerender( + + ); + + // With the `cycle` prop set to `false`, cycling is not allowed. + // By default, cycling from first to last and from last to first is allowed. + await user.tab( { shift: true } ); + expect( firstTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 2 ); + + await user.tab(); + await user.tab(); + expect( lastTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + expect( onNavigateSpy ).toHaveBeenLastCalledWith( + lastTabbableIndex, + lastTabbable + ); + + await user.tab(); + expect( lastTabbable ).toHaveFocus(); + expect( onNavigateSpy ).toHaveBeenCalledTimes( 4 ); + } ); + + it( 'stops keydown event propagation when the tab key is pressed', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + const externalWrapperOnKeyDownSpy = jest.fn(); + + render( + // Disable reason: this is only for test purposes. + // eslint-disable-next-line jsx-a11y/no-static-element-interactions +
+ +
+ ); + + const tabbables = getTabbableContainerTabbables(); + + // Move focus to first item + await user.tab(); + expect( tabbables[ 0 ] ).toHaveFocus(); + + await user.keyboard( '[Space]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + + await user.tab(); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 1 ); + await user.tab( { shift: true } ); + // This extra call is caused by the "shift" key being pressed + // on its own before "tab" + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 2 ); + + await user.keyboard( '[Escape]' ); + expect( externalWrapperOnKeyDownSpy ).toHaveBeenCalledTimes( 3 ); + } ); +} ); From 9c38277ac2bf2c695e6003abac4d2860327364ea Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Wed, 31 Aug 2022 16:35:06 +0200 Subject: [PATCH 9/9] README --- packages/components/src/navigable-container/README.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/components/src/navigable-container/README.md b/packages/components/src/navigable-container/README.md index 25a795c1ec0c7..de6c1103ae6de 100644 --- a/packages/components/src/navigable-container/README.md +++ b/packages/components/src/navigable-container/README.md @@ -39,6 +39,8 @@ The orientation of the menu. It could be "vertical", "horizontal" or "both" A NavigableMenu allows movement up and down (or left and right) the component via the arrow keys. The `tab` key is not handled. The `orientation` prop is used to determine whether the arrow keys used are vertical, horizontal or both. +The `NavigableMenu` by default has a `menu` role and therefore, in order to function as expected, the component expects its children elements to have one of the following roles: `'menuitem' | 'menuitemradio' | 'menuitemcheckbox'`. + ### TabbableContainer A `TabbableContainer` will only be navigated using the `tab` key. Every intended tabstop must have a tabIndex `0`.