NavigableContainer: use code instead of keyCode for keyboard events

packages/components/CHANGELOG.md

@@ -18,6 +18,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/)).
 -   `ComboboxControl`: updated to satisfy `react/exhuastive-deps` eslint rule ([#41417](https://github.com/WordPress/gutenberg/pull/41417))
 
 ## 20.0.0 (2022-08-24)

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`.

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();
 			}
 		}

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.

packages/components/src/navigable-container/stories/navigable-menu.js

@@ -0,0 +1,49 @@
+/**
+ * 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 (
+		<>
+			<button>Before navigable menu</button>
+			<NavigableMenu
+				{ ...args }
+				style={ {
+					margin: '32px 0',
+					padding: '16px',
+					border: '1px solid black',
+				} }
+			>
+				<div role="menuitem">Item 1 (non-tabbable, non-focusable)</div>
+				<button role="menuitem">Item 2 (tabbable, focusable)</button>
+				<button role="menuitem" disabled>
+					Item 3 (disabled, therefore non-tabbable and not-focusable)
+				</button>
+				<span role="menuitem" tabIndex={ -1 }>
+					Item 4 (non-tabbable, non-focusable)
+				</span>
+				<div role="menuitem" tabIndex={ 0 }>
+					Item 5 (tabbable, focusable)
+				</div>
+			</NavigableMenu>
+			<button>After navigable menu</button>
+		</>
+	);
+};

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 (
+		<>
+			<button>Before tabbable container</button>
+			<TabbableContainer
+				{ ...args }
+				style={ {
+					margin: '32px 0',
+					padding: '16px',
+					border: '1px solid black',
+				} }
+			>
+				<button>Item 1</button>
+				<button>Item 2</button>
+				<button disabled>Item 3 (disabled)</button>
+				<button tabIndex={ -1 }>Item 4 (non-tabbable)</button>
+				<button tabIndex={ 0 }>Item 5</button>
+				<button>Item 6</button>
+			</TabbableContainer>
+			<button>After tabbable container</button>
+		</>
+	);
+};

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;
 		}