Modal: use code instead of keyCode for keyboard events (#43429)

* Modal: use `code` instead of `keyCode` for keyboard events * CHANGELOG * Improve Modal tests * Rewite (almost all) ConfirmDialog tests to use `user-event` instead of `fireEvent` * Update CHANGELOG
WordPress · Aug 19, 2022 · e1b033c · e1b033c
1 parent ccf1f44
commit e1b033c
Show file tree

Hide file tree

Showing 4 changed files with 71 additions and 45 deletions.
diff --git a/packages/components/CHANGELOG.md b/packages/components/CHANGELOG.md
@@ -23,6 +23,7 @@
 -   `ToggleGroupControl`: Improve TypeScript documentation ([#43265](https://github.com/WordPress/gutenberg/pull/43265)).
 -   `ComboboxControl`: Normalize hyphen-like characters to an ASCII hyphen ([#42942](https://github.com/WordPress/gutenberg/pull/42942)).
 -   `FormTokenField`: Refactor away from `_.difference()` ([#43224](https://github.com/WordPress/gutenberg/pull/43224/)).
+-   `ConfirmDialog`: replace (almost) every usage of `fireEvent` with `@testing-library/user-event`  ([#43429](https://github.com/WordPress/gutenberg/pull/43429/)).
 
 ### Internal
 
@@ -36,6 +37,7 @@
 -   `contextConnect`: Refactor away from `_.uniq()` ([#43330](https://github.com/WordPress/gutenberg/pull/43330/)).
 -   `ColorPalette`: Refactor away from `_.uniq()` ([#43330](https://github.com/WordPress/gutenberg/pull/43330/)).
 -   `Guide`: Refactor away from `_.times()` ([#43374](https://github.com/WordPress/gutenberg/pull/43374/)).
+-   `Modal`: use `KeyboardEvent.code` instead of deprecated `KeyboardEvent.keyCode`. improve unit tests ([#43429](https://github.com/WordPress/gutenberg/pull/43429/)).
 
 ### Experimental
 -   `FormTokenField`: add `__experimentalAutoSelectFirstMatch` prop to auto select the first matching suggestion on typing ([#42527](https://github.com/WordPress/gutenberg/pull/42527/)).

diff --git a/packages/components/src/confirm-dialog/test/index.js b/packages/components/src/confirm-dialog/test/index.js
@@ -6,7 +6,9 @@ import {
 	screen,
 	fireEvent,
 	waitForElementToBeRemoved,
+	waitFor,
 } from '@testing-library/react';
+import userEvent from '@testing-library/user-event';
 
 /**
  * Internal dependencies
@@ -78,6 +80,10 @@ describe( 'Confirm', () => {
 			} );
 
 			it( 'should not render if closed by clicking `OK`, and the `onConfirm` callback should be called', async () => {
+				const user = userEvent.setup( {
+					advanceTimers: jest.advanceTimersByTime,
+				} );
+
 				const onConfirm = jest.fn().mockName( 'onConfirm()' );
 
 				render(
@@ -89,13 +95,17 @@ describe( 'Confirm', () => {
 				const confirmDialog = screen.getByRole( 'dialog' );
 				const button = screen.getByText( 'OK' );
 
-				fireEvent.click( button );
+				await user.click( button );
 
 				expect( confirmDialog ).not.toBeInTheDocument();
 				expect( onConfirm ).toHaveBeenCalled();
 			} );
 
 			it( 'should not render if closed by clicking `Cancel`, and the `onCancel` callback should be called', async () => {
+				const user = userEvent.setup( {
+					advanceTimers: jest.advanceTimersByTime,
+				} );
+
 				const onCancel = jest.fn().mockName( 'onCancel()' );
 
 				render(
@@ -107,13 +117,17 @@ describe( 'Confirm', () => {
 				const confirmDialog = screen.getByRole( 'dialog' );
 				const button = screen.getByText( 'Cancel' );
 
-				fireEvent.click( button );
+				await user.click( button );
 
 				expect( confirmDialog ).not.toBeInTheDocument();
 				expect( onCancel ).toHaveBeenCalled();
 			} );
 
 			it( 'should be dismissable even if an `onCancel` callback is not provided', async () => {
+				const user = userEvent.setup( {
+					advanceTimers: jest.advanceTimersByTime,
+				} );
+
 				render(
 					<ConfirmDialog onConfirm={ noop }>
 						Are you sure?
@@ -123,7 +137,7 @@ describe( 'Confirm', () => {
 				const confirmDialog = screen.getByRole( 'dialog' );
 				const button = screen.getByText( 'Cancel' );
 
-				fireEvent.click( button );
+				await user.click( button );
 
 				expect( confirmDialog ).not.toBeInTheDocument();
 			} );
@@ -140,6 +154,7 @@ describe( 'Confirm', () => {
 				const confirmDialog = screen.getByRole( 'dialog' );
 
 				//The overlay click is handled by detecting an onBlur from the modal frame.
+				// TODO: replace with `@testing-library/user-event`
 				fireEvent.blur( confirmDialog );
 
 				await waitForElementToBeRemoved( confirmDialog );
@@ -149,6 +164,10 @@ describe( 'Confirm', () => {
 			} );
 
 			it( 'should not render if dialog is closed by pressing `Escape`, and the `onCancel` callback should be called', async () => {
+				const user = userEvent.setup( {
+					advanceTimers: jest.advanceTimersByTime,
+				} );
+
 				const onCancel = jest.fn().mockName( 'onCancel()' );
 
 				render(
@@ -159,13 +178,17 @@ describe( 'Confirm', () => {
 
 				const confirmDialog = screen.getByRole( 'dialog' );
 
-				fireEvent.keyDown( confirmDialog, { keyCode: 27 } );
+				await user.keyboard( '[Escape]' );
 
 				expect( confirmDialog ).not.toBeInTheDocument();
 				expect( onCancel ).toHaveBeenCalled();
 			} );
 
 			it( 'should not render if dialog is closed by pressing `Enter`, and the `onConfirm` callback should be called', async () => {
+				const user = userEvent.setup( {
+					advanceTimers: jest.advanceTimersByTime,
+				} );
+
 				const onConfirm = jest.fn().mockName( 'onConfirm()' );
 
 				render(
@@ -176,7 +199,7 @@ describe( 'Confirm', () => {
 
 				const confirmDialog = screen.getByRole( 'dialog' );
 
-				fireEvent.keyDown( confirmDialog, { keyCode: 13 } );
+				await user.keyboard( '[Enter]' );
 
 				expect( confirmDialog ).not.toBeInTheDocument();
 				expect( onConfirm ).toHaveBeenCalled();
@@ -220,6 +243,10 @@ describe( 'Confirm', () => {
 		} );
 
 		it( 'should call the `onConfirm` callback if `OK`', async () => {
+			const user = userEvent.setup( {
+				advanceTimers: jest.advanceTimersByTime,
+			} );
+
 			const onConfirm = jest.fn().mockName( 'onConfirm()' );
 
 			render(
@@ -230,12 +257,16 @@ describe( 'Confirm', () => {
 
 			const button = screen.getByText( 'OK' );
 
-			fireEvent.click( button );
+			await user.click( button );
 
 			expect( onConfirm ).toHaveBeenCalled();
 		} );
 
 		it( 'should call the `onCancel` callback if `Cancel` is clicked', async () => {
+			const user = userEvent.setup( {
+				advanceTimers: jest.advanceTimersByTime,
+			} );
+
 			const onCancel = jest.fn().mockName( 'onCancel()' );
 
 			render(
@@ -250,17 +281,15 @@ describe( 'Confirm', () => {
 
 			const button = screen.getByText( 'Cancel' );
 
-			fireEvent.click( button );
+			await user.click( button );
 
 			expect( onCancel ).toHaveBeenCalled();
 		} );
 
 		it( 'should call the `onCancel` callback if the overlay is clicked', async () => {
-			jest.useFakeTimers();
-
 			const onCancel = jest.fn().mockName( 'onCancel()' );
 
-			const wrapper = render(
+			render(
 				<ConfirmDialog
 					isOpen={ true }
 					onConfirm={ noop }
@@ -270,45 +299,43 @@ describe( 'Confirm', () => {
 				</ConfirmDialog>
 			);
 
-			const frame = wrapper.baseElement.querySelector(
-				'.components-modal__frame'
-			);
+			const confirmDialog = screen.getByRole( 'dialog' );
 
 			//The overlay click is handled by detecting an onBlur from the modal frame.
-			fireEvent.blur( frame );
-
-			// We don't wait for a DOM side effect here, so we need to fake the timers
-			// and "advance" it so that the `queueBlurCheck` in the `useFocusOutside` hook
-			// properly executes its timeout task.
-			jest.advanceTimersByTime( 0 );
-
-			expect( onCancel ).toHaveBeenCalled();
+			// TODO: replace with `@testing-library/user-event`
+			fireEvent.blur( confirmDialog );
 
-			jest.useRealTimers();
+			// Wait for a DOM side effect here, so that the `queueBlurCheck` in the
+			// `useFocusOutside` hook properly executes its timeout task.
+			await waitFor( () => expect( onCancel ).toHaveBeenCalled() );
 		} );
 
 		it( 'should call the `onCancel` callback if the `Escape` key is pressed', async () => {
+			const user = userEvent.setup( {
+				advanceTimers: jest.advanceTimersByTime,
+			} );
+
 			const onCancel = jest.fn().mockName( 'onCancel()' );
 
-			const wrapper = render(
+			render(
 				<ConfirmDialog onConfirm={ noop } onCancel={ onCancel }>
 					Are you sure?
 				</ConfirmDialog>
 			);
 
-			const frame = wrapper.baseElement.querySelector(
-				'.components-modal__frame'
-			);
-
-			fireEvent.keyDown( frame, { keyCode: 27 } );
+			await user.keyboard( '[Escape]' );
 
 			expect( onCancel ).toHaveBeenCalled();
 		} );
 
 		it( 'should call the `onConfirm` callback if the `Enter` key is pressed', async () => {
+			const user = userEvent.setup( {
+				advanceTimers: jest.advanceTimersByTime,
+			} );
+
 			const onConfirm = jest.fn().mockName( 'onConfirm()' );
 
-			const wrapper = render(
+			render(
 				<ConfirmDialog
 					isOpen={ true }
 					onConfirm={ onConfirm }
@@ -318,11 +345,7 @@ describe( 'Confirm', () => {
 				</ConfirmDialog>
 			);
 
-			const frame = wrapper.baseElement.querySelector(
-				'.components-modal__frame'
-			);
-
-			fireEvent.keyDown( frame, { keyCode: 13 } );
+			await user.keyboard( '[Enter]' );
 
 			expect( onConfirm ).toHaveBeenCalled();
 		} );

diff --git a/packages/components/src/modal/index.js b/packages/components/src/modal/index.js
@@ -24,7 +24,6 @@ import {
 	useConstrainedTabbing,
 	useMergeRefs,
 } from '@wordpress/compose';
-import { ESCAPE } from '@wordpress/keycodes';
 import { __ } from '@wordpress/i18n';
 import { close } from '@wordpress/icons';
 
@@ -98,7 +97,7 @@ function Modal( props, forwardedRef ) {
 	function handleEscapeKeyDown( event ) {
 		if (
 			shouldCloseOnEsc &&
-			event.keyCode === ESCAPE &&
+			event.code === 'Escape' &&
 			! event.defaultPrevented
 		) {
 			event.preventDefault();

diff --git a/packages/components/src/modal/test/index.js b/packages/components/src/modal/test/index.js
@@ -26,25 +26,27 @@ describe( 'Modal', () => {
 		render(
 			<Modal aria={ { labelledby: 'title-id' } }>
 				{ /* eslint-disable-next-line no-restricted-syntax */ }
-				<h1 id="title-id">Test Title</h1>
+				<h1 id="title-id">Modal Title Text</h1>
 			</Modal>
 		);
-		expect( screen.getByRole( 'dialog' ) ).toHaveAttribute(
-			'aria-labelledby',
-			'title-id'
+		expect( screen.getByRole( 'dialog' ) ).toHaveAccessibleName(
+			'Modal Title Text'
 		);
 	} );
 
 	it( 'prefers the aria label of the title prop over the aria.labelledby prop', () => {
 		render(
-			<Modal title="Test Title" aria={ { labelledby: 'title-id' } }>
+			<Modal
+				title="Modal Title Attribute"
+				aria={ { labelledby: 'title-id' } }
+			>
 				{ /* eslint-disable-next-line no-restricted-syntax */ }
-				<h1 id="title-id">Wrong Title</h1>
+				<h1 id="title-id">Modal Title Text</h1>
 			</Modal>
 		);
-		const dialog = screen.getByRole( 'dialog' );
-		const titleId = within( dialog ).getByText( 'Test Title' ).id;
-		expect( dialog ).toHaveAttribute( 'aria-labelledby', titleId );
+		expect( screen.getByRole( 'dialog' ) ).toHaveAccessibleName(
+			'Modal Title Attribute'
+		);
 	} );
 
 	it( 'hides the header when the `__experimentalHideHeader` prop is used', () => {