From 56f4b20a3e4071b6c6f5272a8689e816998ad1bf Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 25 Aug 2022 13:27:04 +0200 Subject: [PATCH 1/4] Guide: use `code` instead of `keyCode` for keyboard events --- packages/components/src/guide/index.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/components/src/guide/index.js b/packages/components/src/guide/index.js index 91e4b0d147ff18..379206f39b7d09 100644 --- a/packages/components/src/guide/index.js +++ b/packages/components/src/guide/index.js @@ -9,7 +9,6 @@ import classnames from 'classnames'; import { useState, useEffect, Children, useRef } from '@wordpress/element'; import deprecated from '@wordpress/deprecated'; import { __ } from '@wordpress/i18n'; -import { LEFT, RIGHT } from '@wordpress/keycodes'; import { focus } from '@wordpress/dom'; /** @@ -76,9 +75,9 @@ export default function Guide( { contentLabel={ contentLabel } onRequestClose={ onFinish } onKeyDown={ ( event ) => { - if ( event.keyCode === LEFT ) { + if ( event.code === 'ArrowLeft' ) { goBack(); - } else if ( event.keyCode === RIGHT ) { + } else if ( event.code === 'ArrowRight' ) { goForward(); } } } From bb317920a74bf1fe100a8be067a93f27894fe80c Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 25 Aug 2022 13:27:33 +0200 Subject: [PATCH 2/4] Prevent scrolling the page contents when pressing left/right arrows --- packages/components/src/guide/index.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/packages/components/src/guide/index.js b/packages/components/src/guide/index.js index 379206f39b7d09..4669f13471801b 100644 --- a/packages/components/src/guide/index.js +++ b/packages/components/src/guide/index.js @@ -77,8 +77,12 @@ export default function Guide( { onKeyDown={ ( event ) => { if ( event.code === 'ArrowLeft' ) { goBack(); + // Do not scroll the modal's contents. + event.preventDefault(); } else if ( event.code === 'ArrowRight' ) { goForward(); + // Do not scroll the modal's contents. + event.preventDefault(); } } } ref={ guideContainer } From 38d1f32042705c4379c328514c4d4b4029f070bb Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 25 Aug 2022 13:28:20 +0200 Subject: [PATCH 3/4] Add unit tes for arrow navigation, rewrite page control unit tests to modern RTL --- packages/components/src/guide/test/index.js | 139 +++++++++++++++++- .../components/src/guide/test/page-control.js | 40 ----- 2 files changed, 138 insertions(+), 41 deletions(-) delete mode 100644 packages/components/src/guide/test/page-control.js diff --git a/packages/components/src/guide/test/index.js b/packages/components/src/guide/test/index.js index bd36752959ff69..852c09ed280887 100644 --- a/packages/components/src/guide/test/index.js +++ b/packages/components/src/guide/test/index.js @@ -1,7 +1,7 @@ /** * External dependencies */ -import { render, screen } from '@testing-library/react'; +import { render, screen, within } from '@testing-library/react'; import userEvent from '@testing-library/user-event'; /** @@ -118,4 +118,141 @@ describe( 'Guide', () => { expect( onFinish ).toHaveBeenCalled(); } ); + + describe( 'page navigation', () => { + it( 'renders an empty list when there are no pages', () => { + render( ); + expect( + screen.queryByRole( 'list', { + name: 'Guide controls', + } ) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole( 'button', { + name: /page \d of \d/i, + } ) + ).not.toBeInTheDocument(); + } ); + + it( 'renders a button for each page', () => { + render( + Page 1

}, + { content:

Page 2

}, + { content:

Page 3

}, + { content:

Page 4

}, + ] } + /> + ); + const listContainer = screen.getByRole( 'list', { + name: 'Guide controls', + } ); + expect( + within( listContainer ).getAllByRole( 'button', { + name: /page \d of \d/i, + } ) + ).toHaveLength( 4 ); + } ); + + it( 'sets the current page when a button is clicked', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + render( + Page 1

}, + { content:

Page 2

}, + { content:

Page 3

}, + ] } + /> + ); + + expect( screen.getByText( 'Page 1' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + + await user.click( + screen.getByRole( 'button', { name: 'Page 2 of 3' } ) + ); + + expect( screen.getByText( 'Page 2' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 1' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + + await user.click( + screen.getByRole( 'button', { name: 'Page 3 of 3' } ) + ); + + expect( screen.getByText( 'Page 3' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 1' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + + await user.click( + screen.getByRole( 'button', { name: 'Page 1 of 3' } ) + ); + + expect( screen.getByText( 'Page 1' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + } ); + + it( 'allows navigating through the pages with the left and right arrows', async () => { + const user = userEvent.setup( { + advanceTimers: jest.advanceTimersByTime, + } ); + + render( + Page 1

}, + { content:

Page 2

}, + { content:

Page 3

}, + ] } + /> + ); + + expect( screen.getByText( 'Page 1' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + + await user.keyboard( '[ArrowLeft]' ); + + expect( screen.getByText( 'Page 1' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + + await user.keyboard( '[ArrowRight]' ); + + expect( screen.getByText( 'Page 2' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 1' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + + await user.keyboard( '[ArrowRight]' ); + + expect( screen.getByText( 'Page 3' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 1' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + + await user.keyboard( '[ArrowRight]' ); + + expect( screen.getByText( 'Page 3' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 1' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + + await user.keyboard( '[ArrowLeft]' ); + + expect( screen.getByText( 'Page 2' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 1' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + + await user.keyboard( '[ArrowLeft]' ); + + expect( screen.getByText( 'Page 1' ) ).toBeVisible(); + expect( screen.queryByText( 'Page 2' ) ).not.toBeInTheDocument(); + expect( screen.queryByText( 'Page 3' ) ).not.toBeInTheDocument(); + } ); + } ); } ); diff --git a/packages/components/src/guide/test/page-control.js b/packages/components/src/guide/test/page-control.js deleted file mode 100644 index caeb3dd005476a..00000000000000 --- a/packages/components/src/guide/test/page-control.js +++ /dev/null @@ -1,40 +0,0 @@ -/** - * External dependencies - */ -import { render, screen } from '@testing-library/react'; -import userEvent from '@testing-library/user-event'; - -/** - * Internal dependencies - */ -import PageControl from '../page-control'; - -describe( 'PageControl', () => { - it( 'renders an empty list when there are no pages', () => { - render( ); - expect( screen.queryByRole( 'button' ) ).not.toBeInTheDocument(); - } ); - - it( 'renders a button for each page', () => { - render( ); - expect( screen.getAllByRole( 'button' ) ).toHaveLength( 5 ); - } ); - - it( 'sets the current page when a button is clicked', async () => { - const user = userEvent.setup( { - advanceTimers: jest.advanceTimersByTime, - } ); - const setCurrentPage = jest.fn(); - render( - - ); - - await user.click( screen.getAllByRole( 'button' )[ 1 ] ); - - expect( setCurrentPage ).toHaveBeenCalledWith( 1 ); - } ); -} ); From 06eda7190bb46c6e8c0a20d18e63ffbfe059fc1b Mon Sep 17 00:00:00 2001 From: Marco Ciampini Date: Thu, 25 Aug 2022 13:44:59 +0200 Subject: [PATCH 4/4] CHANGELOG --- packages/components/CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md index dd236130446a3c..20a1cb6f6bc77c 100644 --- a/packages/components/CHANGELOG.md +++ b/packages/components/CHANGELOG.md @@ -5,6 +5,7 @@ ### Internal - Refactor `FocalPointPicker` to function component ([#39168](https://github.com/WordPress/gutenberg/pull/39168)). +- `Guide`: use `code` instead of `keyCode` for keyboard events ([#43604](https://github.com/WordPress/gutenberg/pull/43604/)). ## 20.0.0 (2022-08-24)