diff --git a/apps/web/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts b/apps/web/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts index 82e6692ea8..70d11b5f24 100644 --- a/apps/web/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts +++ b/apps/web/playwright/e2e/left-panel/room-list-panel/room-list.spec.ts @@ -258,6 +258,40 @@ test.describe("Room list", () => { await expect(notificationButton).toBeFocused(); }); + test("should reveal the options menu when a room is focused with the keyboard", async ({ + page, + app, + user, + }) => { + // Regression test: navigating the room list with the keyboard must reveal a room's hover + // menu so the "More options" button is reachable by Tab, rather than focus escaping to + // . The reveal must depend on keyboard focus alone, so we move focus with the + // keyboard to an adjacent room the pointer is NOT over — otherwise :hover would reveal + // the menu and mask the behaviour (which is why the other keyboard tests don't catch it). + const roomListView = getRoomList(page); + const room29 = roomListView.getByRole("option", { name: "Open room room29" }); + const room28 = roomListView.getByRole("option", { name: "Open room room28" }); + const moreButton = room28.getByRole("button", { name: "More options" }); + + // Open the room, then put focus back on the room list item. + await room29.click(); + await room29.click(); + await expect(room29).toBeFocused(); + + // Keyboard-focus the adjacent room (the pointer is still over room29, not room28), so the + // menu's visibility depends purely on keyboard focus and not on :hover. + await page.keyboard.press("ArrowDown"); + await expect(room28).toBeFocused(); + + // The "More options" button must be revealed and reachable by Tab. + await page.keyboard.press("Tab"); + await expect(moreButton).toBeFocused(); + + // TODO: once menu-close focus restoration is fixed, extend this to open the menu + // (Enter) and assert that Escape returns focus to a room list item rather than . + // Today that focus restoration is broken, so it isn't asserted here. + }); + test("should navigate to the top and then bottom of the room list", async ({ page, app, user }) => { const roomListView = getRoomList(page); diff --git a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.module.css b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.module.css index 626d60a953..4359634380 100644 --- a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.module.css +++ b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.module.css @@ -36,7 +36,10 @@ /* Show hover menu and background on hover/focus/menu-open states */ .roomListItem:hover, -.roomListItem:focus-visible, +/* keyboardActive is toggled in JS for keyboard focus only (see RoomListItemView), and kept while + focus is on the hover-menu buttons so they stay reachable by Tab — without revealing the menu on + mouse-focused or selected rows the way :focus-within would */ +.roomListItem.keyboardActive, /* When the context menu is opened */ .roomListItem[data-state="open"], /* When the options and notifications menu are opened */ diff --git a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.test.tsx b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.test.tsx index 646e114de3..db239a9c4c 100644 --- a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.test.tsx +++ b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.test.tsx @@ -6,7 +6,7 @@ */ import React from "react"; -import { render, screen } from "@test-utils"; +import { render, screen, waitFor } from "@test-utils"; import userEvent from "@testing-library/user-event"; import { composeStories } from "@storybook/react-vite"; import { describe, it, expect } from "vitest"; @@ -132,4 +132,19 @@ describe("", () => { const { container } = render(); expect(container.querySelector('[aria-label="More Options"]')).toBeNull(); }); + + it("reveals the hover menu on keyboard focus and clears it when focus leaves", async () => { + // isFocused focuses the row via the keyboard on mount, so the hover menu is revealed. + const { container } = render(); + const option = screen.getByRole("option"); + const moreButton = container.querySelector('[aria-label="More Options"]'); + + expect(option.className).toMatch(/keyboardActive/); + expect(moreButton).toBeVisible(); + + // Focus leaving the row hides the menu again. + option.blur(); + await waitFor(() => expect(option.className).not.toMatch(/keyboardActive/)); + expect(moreButton).not.toBeVisible(); + }); }); diff --git a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.tsx b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.tsx index b28662070e..fbfa2f9e28 100644 --- a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.tsx +++ b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListItemWrapper/RoomListItemView/RoomListItemView.tsx @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -import React, { type JSX, memo, useEffect, useRef, type ReactNode, type Ref } from "react"; +import React, { type JSX, memo, useEffect, useRef, useState, type ReactNode, type Ref } from "react"; import classNames from "classnames"; import { useMergeRefs } from "react-merge-refs"; @@ -175,12 +175,41 @@ export const RoomListItemView = memo(function RoomListItemView({ const mergedRef = useMergeRefs([ref, internalRef]); const item = useViewModel(vm); + // Reveal the hover menu when the row is focused via the keyboard (not the mouse), and keep it + // revealed while focus moves onto the menu buttons so they stay reachable by Tab. A pure CSS + // :focus-visible rule can't do the latter: it drops the instant focus leaves the row for a child, + // hiding the menu mid-Tab and dropping focus to . A :focus-within rule reveals it for mouse + // focus too, which clutters selected/clicked rows. So we mark keyboard focus in JS (using the + // browser's own :focus-visible determination) and keep it set until focus leaves the row entirely. + const [keyboardActive, setKeyboardActive] = useState(false); + useEffect(() => { if (isFocused) { internalRef.current?.focus({ preventScroll: true } as FocusOptions); } }, [isFocused]); + const onItemFocus = (e: React.FocusEvent): void => { + onFocus(item.id, e); + // Only when focus enters the row from outside via the keyboard. + if (!e.currentTarget.contains(e.relatedTarget as Node | null) && e.currentTarget.matches(":focus-visible")) { + setKeyboardActive(true); + } + }; + + const onItemBlur = (e: React.FocusEvent): void => { + // Keep it revealed while focus is on a child menu button, and while one of the menus is open + // (focus is then in the portaled popover, outside the row). The latter means that when the + // menu closes with Escape, the trigger is still revealed, so the popover's own focus + // restoration lands on it instead of dropping to . Clear once focus leaves for good. + if ( + !e.currentTarget.contains(e.relatedTarget as Node | null) && + !e.currentTarget.querySelector('[data-state="open"]') + ) { + setKeyboardActive(false); + } + }; + // Generate a11y label from notification state and room name const a11yLabel = getA11yLabel(item.name, item.notification); @@ -190,6 +219,7 @@ export const RoomListItemView = memo(function RoomListItemView({ as="button" ref={mergedRef} className={classNames(styles.roomListItem, "mx_RoomListItemView", { + [styles.keyboardActive]: keyboardActive, [styles.selected]: isSelected, [styles.bold]: item.isBold, [styles.firstItem]: isFirstItem, @@ -202,7 +232,8 @@ export const RoomListItemView = memo(function RoomListItemView({ type="button" aria-label={a11yLabel} onClick={vm.onOpenRoom} - onFocus={(e: React.FocusEvent) => onFocus(item.id, e)} + onFocus={onItemFocus} + onBlur={onItemBlur} tabIndex={isFocused ? 0 : -1} aria-selected={props.role === "option" ? isSelected : undefined} {...props} diff --git a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.module.css b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.module.css index 348e986ae5..147d64f5c0 100644 --- a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.module.css +++ b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.module.css @@ -19,7 +19,10 @@ padding: var(--cpd-space-1x) 0; &:hover, - &:focus-visible, + /* keyboardActive is toggled in JS for keyboard focus only (see RoomListSectionHeaderView), and + kept while focus is on the menu button so it stays reachable by Tab — without revealing the + menu on a mouse-focused header the way :focus-within would */ + &.keyboardActive, &:has(button[data-state="open"]) { color: var(--cpd-color-text-primary); diff --git a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.test.tsx b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.test.tsx index 7438a0648d..7552c72e0b 100644 --- a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.test.tsx +++ b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.test.tsx @@ -6,7 +6,7 @@ */ import React from "react"; -import { render, screen } from "@test-utils"; +import { render, screen, waitFor } from "@test-utils"; import { composeStories } from "@storybook/react-vite"; import { describe, it, expect, type Mock, afterEach, vi } from "vitest"; import userEvent from "@testing-library/user-event"; @@ -44,6 +44,17 @@ describe(" stories", () => { expect(document.activeElement).toBe(button); }); + it("reveals the section menu on keyboard focus and clears it when focus leaves", async () => { + // isFocused focuses the header via the keyboard on mount, so the menu is revealed. + render(); + const button = screen.getByRole("button", { name: HEADER_NAME }); + expect(button.className).toMatch(/keyboardActive/); + + // Focus leaving the header hides the menu again. + button.blur(); + await waitFor(() => expect(button.className).not.toMatch(/keyboardActive/)); + }); + it("expands a collapsed section on ArrowRight", async () => { const user = userEvent.setup(); render(); diff --git a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.tsx b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.tsx index 9010aa889c..cb4765da3a 100644 --- a/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.tsx +++ b/packages/shared-components/src/room-list/VirtualizedRoomListView/RoomListSectionHeaderView/RoomListSectionHeaderView.tsx @@ -5,7 +5,7 @@ * Please see LICENSE files in the repository root for full details. */ -import React, { memo, type JSX, type FocusEvent, useEffect, useRef } from "react"; +import React, { memo, type JSX, type FocusEvent, useEffect, useRef, useState } from "react"; import classNames from "classnames"; import { useDraggable, useDragOperation, useDroppable } from "@dnd-kit/react"; import { useMergeRefs } from "react-merge-refs"; @@ -168,6 +168,29 @@ export const RoomListSectionHeaderView = memo(function RoomListSectionHeaderView } }, [isFocused]); + // Reveal the menu only on keyboard focus (not mouse), kept while focus is on the menu button so + // it stays reachable by Tab. See RoomListItemView for the full rationale. + const [keyboardActive, setKeyboardActive] = useState(false); + + const onHeaderFocus = (e: React.FocusEvent): void => { + onFocus(id, e); + if (!e.currentTarget.contains(e.relatedTarget as Node | null) && e.currentTarget.matches(":focus-visible")) { + setKeyboardActive(true); + } + }; + + const onHeaderBlur = (e: React.FocusEvent): void => { + // Keep it revealed while focus is on the menu button, and while the menu is open (focus is + // then in the portaled popover, outside the header). That way closing with Escape restores + // focus to the still-revealed trigger instead of dropping to . Clear once focus leaves. + if ( + !e.currentTarget.contains(e.relatedTarget as Node | null) && + !e.currentTarget.querySelector('[data-state="open"]') + ) { + setKeyboardActive(false); + } + }; + return (
onFocus(id, e)} + onFocus={onHeaderFocus} + onBlur={onHeaderBlur} tabIndex={isFocused ? 0 : -1} aria-label={ isUnread