Fix dropdown menu focus loss when using arrow keys with Safari and Voiceover

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

@@ -9,6 +9,8 @@ import { omit, noop, isFunction } from 'lodash';
 import { Component, forwardRef } from '@wordpress/element';
 import { focus } from '@wordpress/dom';
 
+const MENU_ITEM_ROLES = [ 'menuitem', 'menuitemradio', 'menuitemcheckbox' ];
+
 function cycleValue( value, total, offset ) {
 	const nextValue = value + offset;
 	if ( nextValue < 0 ) {
@@ -96,8 +98,11 @@ class NavigableContainer extends Component {
 			event.stopImmediatePropagation();
 
 			// When navigating a collection of items, prevent scroll containers
-			// from scrolling.
-			if ( event.target.getAttribute( 'role' ) === 'menuitem' ) {
+			// from scrolling. The preventDefault also prevents Voiceover from
+			// '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 ) ) {
 				event.preventDefault();
 			}
 		}

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

@@ -38,6 +38,11 @@ export function NavigableMenu(
 			return 1;
 		} else if ( includes( previous, keyCode ) ) {
 			return -1;
+		} else if ( includes( [ DOWN, UP, LEFT, RIGHT ], keyCode ) ) {
+			// Key press should be handled, e.g. have event propagation and
+			// default behavior handled by NavigableContainer but not result
+			// in an offset.
+			return 0;
 		}
 	};
 

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

@@ -94,8 +94,8 @@ describe( 'NavigableMenu', () => {
 		assertKeyDown( UP, 2, true );
 		assertKeyDown( UP, 1, true );
 		assertKeyDown( UP, 0, true );
-		assertKeyDown( LEFT, 0, false );
-		assertKeyDown( RIGHT, 0, false );
+		assertKeyDown( LEFT, 0, true );
+		assertKeyDown( RIGHT, 0, true );
 		assertKeyDown( SPACE, 0, false );
 	} );
 
@@ -146,8 +146,8 @@ describe( 'NavigableMenu', () => {
 		assertKeyDown( UP, 1, true );
 		assertKeyDown( UP, 0, true );
 		assertKeyDown( UP, 0, true );
-		assertKeyDown( LEFT, 0, false );
-		assertKeyDown( RIGHT, 0, false );
+		assertKeyDown( LEFT, 0, true );
+		assertKeyDown( RIGHT, 0, true );
 		assertKeyDown( SPACE, 0, false );
 	} );
 
@@ -204,8 +204,8 @@ describe( 'NavigableMenu', () => {
 		assertKeyDown( LEFT, 2, true );
 		assertKeyDown( LEFT, 1, true );
 		assertKeyDown( LEFT, 0, true );
-		assertKeyDown( UP, 0, false );
-		assertKeyDown( DOWN, 0, false );
+		assertKeyDown( UP, 0, true );
+		assertKeyDown( DOWN, 0, true );
 		assertKeyDown( SPACE, 0, false );
 	} );
 
@@ -256,8 +256,8 @@ describe( 'NavigableMenu', () => {
 		assertKeyDown( LEFT, 1, true );
 		assertKeyDown( LEFT, 0, true );
 		assertKeyDown( LEFT, 0, true );
-		assertKeyDown( DOWN, 0, false );
-		assertKeyDown( UP, 0, false );
+		assertKeyDown( DOWN, 0, true );
+		assertKeyDown( UP, 0, true );
 		assertKeyDown( SPACE, 0, false );
 	} );
 

packages/e2e-tests/specs/editor/various/dropdown-menu.test.js

@@ -0,0 +1,144 @@
+/**
+ * WordPress dependencies
+ */
+import { createNewPost, pressKeyTimes } from '@wordpress/e2e-test-utils';
+
+const moreMenuButtonSelector =
+	'.components-button[aria-label="More tools & options"]';
+const moreMenuDropdownSelector =
+	'.components-dropdown-menu__menu[aria-label="More tools & options"]';
+const menuItemsSelector = [ 'menuitem', 'menuitemcheckbox', 'menuitemradio' ]
+	.map( ( role ) => `${ moreMenuDropdownSelector } [role="${ role }"]` )
+	.join( ',' );
+
+describe( 'Dropdown Menu', () => {
+	beforeEach( async () => {
+		await createNewPost();
+	} );
+
+	it( 'allows navigation through each item using arrow keys', async () => {
+		await page.click( moreMenuButtonSelector );
+		const menuItems = await page.$$( menuItemsSelector );
+
+		// Catch any issues with the selector, which could cause a false positive test result.
+		expect( menuItems.length ).toBeGreaterThan( 0 );
+
+		let activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+		const [ firstMenuItem ] = menuItems;
+		const firstMenuItemText = await firstMenuItem.evaluate(
+			( element ) => element.textContent
+		);
+
+		// Expect the first menu item to be focused.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( firstMenuItemText );
+
+		// Arrow down to the last item.
+		await pressKeyTimes( 'ArrowDown', menuItems.length - 1 );
+
+		activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+
+		const [ lastMenuItem ] = menuItems.slice( -1 );
+		const lastMenuItemText = await lastMenuItem.evaluate(
+			( element ) => element.textContent
+		);
+
+		// Expect the last menu item to be focused.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( lastMenuItemText );
+
+		// Arrow back up to the first item.
+		await pressKeyTimes( 'ArrowUp', menuItems.length - 1 );
+
+		activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+
+		// Expect the first menu item to be focused again.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( firstMenuItemText );
+	} );
+
+	it( 'loops to the beginning and end when navigating past the boundaries of the menu', async () => {
+		await page.click( moreMenuButtonSelector );
+		const menuItems = await page.$$( menuItemsSelector );
+
+		// Catch any issues with the selector, which could cause a false positive test result.
+		expect( menuItems.length ).toBeGreaterThan( 0 );
+
+		let activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+		const [ firstMenuItem ] = menuItems;
+		const firstMenuItemText = await firstMenuItem.evaluate(
+			( element ) => element.textContent
+		);
+
+		// Expect the first menu item to be focused.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( firstMenuItemText );
+
+		// Arrow up to the last item.
+		await page.keyboard.press( 'ArrowUp' );
+
+		activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+
+		const [ lastMenuItem ] = menuItems.slice( -1 );
+		const lastMenuItemText = await lastMenuItem.evaluate(
+			( element ) => element.textContent
+		);
+
+		// Expect the last menu item to be focused.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( lastMenuItemText );
+
+		// Arrow back down to the first item.
+		await page.keyboard.press( 'ArrowDown' );
+
+		activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+
+		// Expect the first menu item to be focused again.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( firstMenuItemText );
+	} );
+
+	it( 'ignores arrow key navigation that is orthogonal to the orientation of the menu, but stays open', async () => {
+		await page.click( moreMenuButtonSelector );
+		const menuItems = await page.$$( menuItemsSelector );
+
+		// Catch any issues with the selector, which could cause a false positive test result.
+		expect( menuItems.length ).toBeGreaterThan( 0 );
+
+		let activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+		const [ firstMenuItem ] = menuItems;
+		const firstMenuItemText = await firstMenuItem.evaluate(
+			( element ) => element.textContent
+		);
+
+		// Expect the first menu item to be focused.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( firstMenuItemText );
+
+		// Press left and right keys an arbitrary (but > 1) number of times.
+		await pressKeyTimes( 'ArrowLeft', 5 );
+		await pressKeyTimes( 'ArrowRight', 5 );
+
+		activeElementText = await page.evaluate(
+			() => document.activeElement.textContent
+		);
+
+		// Expect the first menu item to still be focused.
+		expect( activeElementText ).toBeDefined();
+		expect( activeElementText ).toBe( firstMenuItemText );
+	} );
+} );