From 2dcf6bffb1014ada191c88793a2ed1fa5a1cfc6e Mon Sep 17 00:00:00 2001 From: Harrison Healey Date: Thu, 3 Apr 2025 14:22:42 -0400 Subject: [PATCH] MM-63451 Rely on MUI's MenuList to handle focus when opening menus (#30585) * MM-63451 Rely on MUI's MenuList to handle focus when opening menus * MM-63451 Add E2E tests for keyboard accessibility in the account menu * Run prettier on E2E tests * And check in the rest of those changes * Fix lint --- .../lib/src/ui/components/global_header.ts | 2 + .../channels/account_menu_keyboard.spec.ts | 108 ++++++++++++++++++ webapp/channels/src/components/menu/menu.tsx | 11 +- 3 files changed, 111 insertions(+), 10 deletions(-) create mode 100644 e2e-tests/playwright/test/specs/accessibility/channels/account_menu_keyboard.spec.ts diff --git a/e2e-tests/playwright/lib/src/ui/components/global_header.ts b/e2e-tests/playwright/lib/src/ui/components/global_header.ts index b0b21bea7a..5c001e2fe6 100644 --- a/e2e-tests/playwright/lib/src/ui/components/global_header.ts +++ b/e2e-tests/playwright/lib/src/ui/components/global_header.ts @@ -6,6 +6,7 @@ import {Locator, expect} from '@playwright/test'; export default class GlobalHeader { readonly container: Locator; + readonly accountMenuButton; readonly productSwitchMenu; readonly recentMentionsButton; readonly settingsButton; @@ -14,6 +15,7 @@ export default class GlobalHeader { constructor(container: Locator) { this.container = container; + this.accountMenuButton = container.getByRole('button', {name: "'s account menu"}); this.productSwitchMenu = container.getByRole('button', {name: 'Product switch menu'}); this.recentMentionsButton = container.getByRole('button', {name: 'Recent mentions'}); this.settingsButton = container.getByRole('button', {name: 'Settings'}); diff --git a/e2e-tests/playwright/test/specs/accessibility/channels/account_menu_keyboard.spec.ts b/e2e-tests/playwright/test/specs/accessibility/channels/account_menu_keyboard.spec.ts new file mode 100644 index 0000000000..b13dcb3645 --- /dev/null +++ b/e2e-tests/playwright/test/specs/accessibility/channels/account_menu_keyboard.spec.ts @@ -0,0 +1,108 @@ +// Copyright (c) 2015-present Mattermost, Inc. All Rights Reserved. +// See LICENSE.txt for license information. + +import {expect, test, ChannelsPage} from '@mattermost/playwright-lib'; +import {UserProfile} from '@mattermost/types/users'; +import {Page} from '@playwright/test'; + +test('MM-63451 should be able to navigate the account settings menu with the keyboard after opening it with the mouse', async ({ + pw, +}) => { + // # Create and sign in a new user + const {user} = await pw.initSetup(); + + // # Log in a user in new browser context + const {page, channelsPage} = await pw.testBrowser.login(user); + + // # Visit a default channel page + await channelsPage.goto(); + await channelsPage.toBeVisible(); + + // # Click on the account menu button + await channelsPage.globalHeader.accountMenuButton.click(); + + await testMenuWithKeyboard(user, page, channelsPage); +}); + +test('MM-63451 should be able to navigate the account settings menu with the keyboard after opening it with the keyboard', async ({ + pw, +}) => { + // # Create and sign in a new user + const {user} = await pw.initSetup(); + + // # Log in a user in new browser context + const {page, channelsPage} = await pw.testBrowser.login(user); + + // # Visit a default channel page + await channelsPage.goto(); + await channelsPage.toBeVisible(); + + // # Focus the account menu button + await channelsPage.globalHeader.accountMenuButton.focus(); + await expect(channelsPage.globalHeader.accountMenuButton).toBeFocused(); + await page.keyboard.press('Space'); + + await testMenuWithKeyboard(user, page, channelsPage); +}); + +async function testMenuWithKeyboard(user: UserProfile, page: Page, channelsPage: ChannelsPage) { + // * Should start focused on the first menu item + await expect(page.getByRole('menuitem', {name: '@' + user.username})).toBeFocused(); + + // * Should be able to scroll down through the menu with the keyboard + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Set custom status'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Online'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Away'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Do not disturb Disables all notifications'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Offline'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Profile'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Log Out'})).toBeFocused(); + + // * Should be able to scroll back up through the menu with the keyboard + await page.keyboard.press('ArrowUp'); + await expect(page.getByRole('menuitem', {name: 'Profile'})).toBeFocused(); + await page.keyboard.press('ArrowUp'); + await expect(page.getByRole('menuitem', {name: 'Offline'})).toBeFocused(); + await page.keyboard.press('ArrowUp'); + await expect(page.getByRole('menuitem', {name: 'Do not disturb Disables all notifications'})).toBeFocused(); + + // * Should be able to move into the submenu by pressing the right arrow + await page.keyboard.press('ArrowRight'); + await expect(page.getByRole('menuitem', {name: "Don't clear"})).toBeFocused(); + + // * Should be able to scroll through the submenu with the keyboard + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: '30 mins'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: '1 hour'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: '2 hours'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Tomorrow'})).toBeFocused(); + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: 'Choose date and time'})).toBeFocused(); + + // * Should wrap around when you reach the end + await page.keyboard.press('ArrowDown'); + await expect(page.getByRole('menuitem', {name: "Don't clear"})).toBeFocused(); + await page.keyboard.press('ArrowUp'); + await expect(page.getByRole('menuitem', {name: 'Choose date and time'})).toBeFocused(); + + // * Should be able to close the submenu by pressing the left arrow + await page.keyboard.press('ArrowLeft'); + await expect(page.getByRole('menuitem', {name: 'Do not disturb Disables all notifications'})).toBeFocused(); + + // * Should be able to close the menu by pressing escape + await page.keyboard.press('Escape'); + await expect(page.getByRole('menuitem')).toHaveCount(0); + + // * Should be focused back on the menu button + await expect(channelsPage.globalHeader.accountMenuButton).toBeFocused(); +} diff --git a/webapp/channels/src/components/menu/menu.tsx b/webapp/channels/src/components/menu/menu.tsx index ceee60916a..aeea0fa1fb 100644 --- a/webapp/channels/src/components/menu/menu.tsx +++ b/webapp/channels/src/components/menu/menu.tsx @@ -113,20 +113,17 @@ export function Menu(props: Props) { const dispatch = useDispatch(); const [anchorElement, setAnchorElement] = useState(null); - const [disableAutoFocusItem, setDisableAutoFocusItem] = useState(false); const isMenuOpen = Boolean(anchorElement); // Callback function handler called when menu is closed by escapeKeyDown, backdropClick or tabKeyDown function handleMenuClose(event: MouseEvent) { event.preventDefault(); setAnchorElement(null); - setDisableAutoFocusItem(false); } // Handle function injected into menu items to close the menu const closeMenu = useCallback(() => { setAnchorElement(null); - setDisableAutoFocusItem(false); }, []); function handleMenuModalClose(modalId: MenuProps['id']) { @@ -195,11 +192,6 @@ export function Menu(props: Props) { } } - // Function to prevent focus-visible from being set on clicking menu items with the mouse - function handleMenuButtonMouseDown() { - setDisableAutoFocusItem(true); - } - // We construct the menu button so we can set onClick correctly here to support both web and mobile view function renderMenuButton() { const MenuButtonComponent = props.menuButton?.as ?? 'button'; @@ -216,7 +208,6 @@ export function Menu(props: Props) { aria-describedby={props.menuButton?.['aria-describedby']} className={props.menuButton?.class ?? ''} onClick={handleMenuButtonClick} - onMouseDown={handleMenuButtonMouseDown} > {props.menuButton.children} @@ -293,11 +284,11 @@ export function Menu(props: Props) { id={props.menu.id} aria-label={props.menu?.['aria-label']} aria-labelledby={props.menu['aria-labelledby']} - autoFocusItem={!disableAutoFocusItem} className={props.menu.className} style={{ width: props.menu.width, }} + autoFocusItem={isMenuOpen} > {props.children}