Skip to content

Commit

Permalink
Guide: use code instead of keyCode for keyboard events (#43604)
Browse files Browse the repository at this point in the history
* Guide: use `code` instead of `keyCode` for keyboard events

* Prevent scrolling the page contents when pressing left/right arrows

* Add unit tes for arrow navigation, rewrite page control unit tests to modern RTL

* CHANGELOG
  • Loading branch information
ciampo authored Aug 26, 2022
1 parent b40b540 commit f68ec14
Show file tree
Hide file tree
Showing 4 changed files with 145 additions and 44 deletions.
1 change: 1 addition & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
9 changes: 6 additions & 3 deletions packages/components/src/guide/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

/**
Expand Down Expand Up @@ -76,10 +75,14 @@ export default function Guide( {
contentLabel={ contentLabel }
onRequestClose={ onFinish }
onKeyDown={ ( event ) => {
if ( event.keyCode === LEFT ) {
if ( event.code === 'ArrowLeft' ) {
goBack();
} else if ( event.keyCode === RIGHT ) {
// 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 }
Expand Down
139 changes: 138 additions & 1 deletion packages/components/src/guide/test/index.js
Original file line number Diff line number Diff line change
@@ -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';

/**
Expand Down Expand Up @@ -118,4 +118,141 @@ describe( 'Guide', () => {

expect( onFinish ).toHaveBeenCalled();
} );

describe( 'page navigation', () => {
it( 'renders an empty list when there are no pages', () => {
render( <Guide pages={ [] } /> );
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(
<Guide
pages={ [
{ content: <p>Page 1</p> },
{ content: <p>Page 2</p> },
{ content: <p>Page 3</p> },
{ content: <p>Page 4</p> },
] }
/>
);
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(
<Guide
pages={ [
{ content: <p>Page 1</p> },
{ content: <p>Page 2</p> },
{ content: <p>Page 3</p> },
] }
/>
);

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(
<Guide
pages={ [
{ content: <p>Page 1</p> },
{ content: <p>Page 2</p> },
{ content: <p>Page 3</p> },
] }
/>
);

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();
} );
} );
} );
40 changes: 0 additions & 40 deletions packages/components/src/guide/test/page-control.js

This file was deleted.

0 comments on commit f68ec14

Please sign in to comment.