Room list: fix keyboard navigation on sections (#33809)

* fix: keyboard navigation on sections * test: add e2e test for keyboard navigation * fix: add more keyboard navigation * test: add e2e tests * chore: remove useless check
2026-06-11 18:14:36 +02:00
parent e441ca5d7c
commit 36a39f7fd6
6 changed files with 311 additions and 12 deletions
@@ -290,4 +290,131 @@ test.describe("Room list sections", () => {
            await expect(roomList.getByRole("row", { name: "no unread room" })).not.toBeVisible();
        });
    });
+
+    test.describe("Section keyboard navigation", () => {
+        test.beforeEach(async ({ app }) => {
+            // A favourite room forces section mode and gives us a non-trivial first section.
+            const favouriteId = await app.client.createRoom({ name: "favourite room" });
+            await app.client.evaluate(async (client, roomId) => {
+                await client.setRoomTag(roomId, "m.favourite");
+            }, favouriteId);
+
+            // A chats-section room so we have a second section to navigate to.
+            await app.client.createRoom({ name: "chat room" });
+        });
+
+        test("Arrow Down/Up move focus through sections and rooms", async ({ page }) => {
+            const roomList = getRoomList(page);
+            const favouritesHeader = getSectionHeader(page, "Favourites");
+            // In treegrid mode, a room renders as <div role="row"><div role="gridcell"><button …></button></div></div>.
+            // Only the inner <button> is focusable, so target it by role for focus assertions.
+            const favRoomButton = roomList.getByRole("button", { name: "Open room favourite room" });
+            const chatsHeader = getSectionHeader(page, "Chats");
+
+            await expect(favouritesHeader).toBeVisible();
+            await expect(favRoomButton).toBeVisible();
+            await expect(chatsHeader).toBeVisible();
+
+            await favouritesHeader.focus();
+            await expect(favouritesHeader).toBeFocused();
+
+            // Down moves into the favourite section's room.
+            await page.keyboard.press("ArrowDown");
+            await expect(favRoomButton).toBeFocused();
+
+            // Down again jumps to the next section header.
+            await page.keyboard.press("ArrowDown");
+            await expect(chatsHeader).toBeFocused();
+
+            // Up reverses the traversal.
+            await page.keyboard.press("ArrowUp");
+            await expect(favRoomButton).toBeFocused();
+
+            await page.keyboard.press("ArrowUp");
+            await expect(favouritesHeader).toBeFocused();
+        });
+
+        test("Arrow Right expands a collapsed section", async ({ page }) => {
+            const favouritesHeader = getSectionHeader(page, "Favourites");
+            const favRoom = getRoomList(page).getByRole("row", { name: "Open room favourite room" });
+
+            // Collapse the section via click so we know we start expanded=false.
+            await favouritesHeader.click();
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "false");
+            await expect(favRoom).not.toBeVisible();
+
+            await favouritesHeader.focus();
+            await page.keyboard.press("ArrowRight");
+
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "true");
+            await expect(favRoom).toBeVisible();
+        });
+
+        test("Arrow Right is a no-op on an already-expanded section", async ({ page }) => {
+            const favouritesHeader = getSectionHeader(page, "Favourites");
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "true");
+
+            await favouritesHeader.focus();
+            await page.keyboard.press("ArrowRight");
+
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "true");
+        });
+
+        test("Arrow Left collapses an expanded section", async ({ page }) => {
+            const favouritesHeader = getSectionHeader(page, "Favourites");
+            const favRoom = getRoomList(page).getByRole("row", { name: "Open room favourite room" });
+
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "true");
+            await expect(favRoom).toBeVisible();
+
+            await favouritesHeader.focus();
+            await page.keyboard.press("ArrowLeft");
+
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "false");
+            await expect(favRoom).not.toBeVisible();
+        });
+
+        test("Arrow Left is a no-op on an already-collapsed section", async ({ page }) => {
+            const favouritesHeader = getSectionHeader(page, "Favourites");
+
+            await favouritesHeader.click();
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "false");
+
+            await favouritesHeader.focus();
+            await page.keyboard.press("ArrowLeft");
+
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "false");
+        });
+
+        test("Arrow Right on an expanded section with rooms moves focus to its first room", async ({ page }) => {
+            const favouritesHeader = getSectionHeader(page, "Favourites");
+            const favRoomButton = getRoomList(page).getByRole("button", { name: "Open room favourite room" });
+
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "true");
+
+            await favouritesHeader.focus();
+            await expect(favouritesHeader).toBeFocused();
+
+            await page.keyboard.press("ArrowRight");
+
+            // Focus must move to the first room in the section, not the next section header.
+            await expect(favRoomButton).toBeFocused();
+            // The section should remain expanded.
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "true");
+        });
+
+        test("Arrow Left on the first room of a section moves focus back to the section header", async ({ page }) => {
+            const favouritesHeader = getSectionHeader(page, "Favourites");
+            const favRoomButton = getRoomList(page).getByRole("button", { name: "Open room favourite room" });
+
+            await expect(favouritesHeader).toHaveAttribute("aria-expanded", "true");
+
+            await favRoomButton.focus();
+            await expect(favRoomButton).toBeFocused();
+
+            await page.keyboard.press("ArrowLeft");
+
+            await expect(favouritesHeader).toBeFocused();
+        });
+    });
 });
@@ -0,0 +1,61 @@
+/*
+ * Copyright 2026 Element Creations Ltd.
+ *
+ * SPDX-License-Identifier: AGPL-3.0-only OR GPL-3.0-only OR LicenseRef-Element-Commercial
+ * Please see LICENSE files in the repository root for full details.
+ */
+
+import React from "react";
+import { render, screen } from "@test-utils";
+import userEvent from "@testing-library/user-event";
+import { composeStories } from "@storybook/react-vite";
+import { describe, it, expect, vi } from "vitest";
+
+import * as stories from "./RoomListItemWrapper.stories";
+
+const { Sections } = composeStories(stories);
+
+describe("<RoomListItemWrapper /> keyboard re-dispatch", () => {
+    it("ArrowLeft on the first room of a section re-dispatches as ArrowUp", async () => {
+        const user = userEvent.setup();
+        const onKeyDown = vi.fn();
+        render(
+            // eslint-disable-next-line jsx-a11y/no-static-element-interactions
+            <div onKeyDown={onKeyDown}>
+                <Sections roomIndexInSection={0} isFocused={true} />
+            </div>,
+        );
+
+        // Make sure the room's button is focused before sending the key event.
+        const button = screen.getByRole("button", { name: /Room name/ });
+        button.focus();
+
+        await user.keyboard("{ArrowLeft}");
+
+        const arrowUpEvents = onKeyDown.mock.calls.filter(([event]) => event.code === "ArrowUp");
+        expect(arrowUpEvents).toHaveLength(1);
+    });
+
+    it("ArrowLeft on a non-first room of a section does NOT re-dispatch", async () => {
+        const user = userEvent.setup();
+        const onKeyDown = vi.fn();
+        render(
+            // eslint-disable-next-line jsx-a11y/no-static-element-interactions
+            <div onKeyDown={onKeyDown}>
+                <Sections roomIndexInSection={2} isFocused={true} />
+            </div>,
+        );
+
+        const button = screen.getByRole("button", { name: /Room name/ });
+        button.focus();
+
+        await user.keyboard("{ArrowLeft}");
+
+        const arrowUpEvents = onKeyDown.mock.calls.filter(([event]) => event.code === "ArrowUp");
+        expect(arrowUpEvents).toHaveLength(0);
+
+        // The original ArrowLeft event should still bubble (the handler is a no-op for non-first items).
+        const arrowLeftEvents = onKeyDown.mock.calls.filter(([event]) => event.code === "ArrowLeft");
+        expect(arrowLeftEvents.length).toBeGreaterThan(0);
+    });
+});
@@ -41,10 +41,27 @@ export const RoomListItemWrapper = memo(function RoomListItemWrapper({
        return <RoomListItemView {...rest} {...getItemAccessibleProps("listbox", roomIndex, roomCount)} />;
    }

+    const isFirstInSection = roomIndexInSection === 0;
    return (
        <div {...getItemAccessibleProps("treegrid", roomIndex, roomIndexInSection)}>
            <div role="gridcell" aria-selected={rest.isSelected}>
-                <DraggableWrapper {...rest} />
+                <DraggableWrapper
+                    {...rest}
+                    onKeyDown={(e) => {
+                        if (e.code === "ArrowLeft" && isFirstInSection) {
+                            // Move focus to the section header
+                            e.preventDefault();
+                            e.stopPropagation();
+                            e.currentTarget.dispatchEvent(
+                                new KeyboardEvent("keydown", {
+                                    code: "ArrowUp",
+                                    key: "ArrowUp",
+                                    bubbles: true,
+                                }),
+                            );
+                        }
+                    }}
+                />
            </div>
        </div>
    );
@@ -17,7 +17,6 @@
    letter-spacing: var(--cpd-font-letter-spacing-body-sm);
    color: var(--cpd-color-text-secondary);
    padding: var(--cpd-space-1x) 0;
-    background-color: var(--cpd-color-bg-canvas-default);

    &:hover,
    &:focus-visible,
@@ -82,6 +81,12 @@

 .firstHeader {
    padding-top: 0;
+
+    /* The first item sits flush with the scroller's top edge, so the default outline
+       would be clipped by overflow:auto. Inset it so it stays inside the button. */
+    &:focus-visible {
+        outline-offset: -2px;
+    }
 }

 .lastHeader {
@@ -6,16 +6,24 @@
 */

 import React from "react";
-import { render } from "@test-utils";
+import { render, screen } from "@test-utils";
 import { composeStories } from "@storybook/react-vite";
-import { describe, it, expect } from "vitest";
+import { describe, it, expect, type Mock, afterEach, vi } from "vitest";
 import userEvent from "@testing-library/user-event";

 import * as stories from "./RoomListSectionHeaderView.stories";

-const { Default } = composeStories(stories);
+const { Default, Collapsed } = composeStories(stories);
+
+const HEADER_NAME = "Toggle Favourites section";

 describe("<RoomListSectionHeaderView /> stories", () => {
+    afterEach(() => {
+        // Storybook's fn() mocks aren't reset by vi.clearAllMocks; clear them by hand.
+        (Default.args.onClick as Mock).mockClear();
+        (Collapsed.args.onClick as Mock).mockClear();
+    });
+
    it("renders Default story", () => {
        const { container } = render(<Default />);
        expect(container).toMatchSnapshot();
@@ -24,9 +32,63 @@ describe("<RoomListSectionHeaderView /> stories", () => {
    it("should call onClick when the header is clicked", async () => {
        const user = userEvent.setup();

-        const { getByRole } = render(<Default />);
-        const button = getByRole("gridcell", { name: "Toggle Favourites section" });
+        render(<Default />);
+        const button = screen.getByRole("gridcell", { name: HEADER_NAME });
        await user.click(button);
        expect(Default.args.onClick).toHaveBeenCalled();
    });
+
+    it("focuses the button when isFocused is true", () => {
+        render(<Default isFocused={true} />);
+        const button = screen.getByRole("gridcell", { name: HEADER_NAME });
+        expect(document.activeElement).toBe(button);
+    });
+
+    it("expands a collapsed section on ArrowRight", async () => {
+        const user = userEvent.setup();
+        render(<Collapsed isFocused={true} />);
+        await user.keyboard("{ArrowRight}");
+        expect(Collapsed.args.onClick).toHaveBeenCalledTimes(1);
+    });
+
+    it("does not call onClick on ArrowRight when already expanded", async () => {
+        const user = userEvent.setup();
+        render(<Default isFocused={true} />);
+        await user.keyboard("{ArrowRight}");
+        expect(Default.args.onClick).not.toHaveBeenCalled();
+    });
+
+    it("collapses an expanded section on ArrowLeft", async () => {
+        const user = userEvent.setup();
+        render(<Default isFocused={true} />);
+        await user.keyboard("{ArrowLeft}");
+        expect(Default.args.onClick).toHaveBeenCalledTimes(1);
+    });
+
+    it("does not call onClick on ArrowLeft when already collapsed", async () => {
+        const user = userEvent.setup();
+        render(<Collapsed isFocused={true} />);
+        await user.keyboard("{ArrowLeft}");
+        expect(Collapsed.args.onClick).not.toHaveBeenCalled();
+    });
+
+    it("ArrowRight on an expanded section re-dispatches as ArrowDown", async () => {
+        const user = userEvent.setup();
+        const onKeyDown = vi.fn();
+        render(
+            // eslint-disable-next-line jsx-a11y/no-static-element-interactions
+            <div onKeyDown={onKeyDown}>
+                <Default isFocused={true} />
+            </div>,
+        );
+        await user.keyboard("{ArrowRight}");
+
+        // The re-dispatched ArrowDown should bubble up to the parent listener.
+        const arrowDownEvents = onKeyDown.mock.calls.filter(([event]) => event.code === "ArrowDown");
+        expect(arrowDownEvents).toHaveLength(1);
+
+        // The original ArrowRight handler called preventDefault/stopPropagation, so
+        // it should not have called onClick (which is reserved for the toggle branch).
+        expect(Default.args.onClick).not.toHaveBeenCalled();
+    });
 });
@@ -5,12 +5,13 @@
 * Please see LICENSE files in the repository root for full details.
 */

-import React, { memo, type JSX, type FocusEvent, type MouseEventHandler, useState } from "react";
+import React, { memo, type JSX, type FocusEvent, useEffect, useRef, useState } from "react";
 import ChevronRightIcon from "@vector-im/compound-design-tokens/assets/web/icons/chevron-right";
 import classNames from "classnames";
 import { IconButton, Menu, MenuItem } from "@vector-im/compound-web";
 import { OverflowHorizontalIcon, EditIcon, DeleteIcon } from "@vector-im/compound-design-tokens/assets/web/icons";
 import { useDroppable } from "@dnd-kit/react";
+import { useMergeRefs } from "react-merge-refs";

 import { useViewModel, type ViewModel } from "../../../core/viewmodel";
 import styles from "./RoomListSectionHeaderView.module.css";
@@ -39,8 +40,8 @@ export interface RoomListSectionHeaderViewSnapshot {
 * Actions that can be performed on a room list section header.
 */
 export interface RoomListSectionHeaderActions {
-    /** Handler invoked when the section header is clicked (toggles expand/collapse). */
-    onClick: MouseEventHandler<HTMLButtonElement>;
+    /** Handler invoked when the section header is clicked or keyboard-toggled (toggles expand/collapse). */
+    onClick: () => void;
    /** Handler invoked when the edit section button is clicked  */
    editSection: () => void;
    /** Handler invoked when the remove section button is clicked  */
@@ -104,9 +105,17 @@ export const RoomListSectionHeaderView = memo(function RoomListSectionHeaderView
    const { id, title, isExpanded, isUnread, displaySectionMenu } = useViewModel(vm);
    const isLastSection = sectionIndex === sectionCount - 1;

-    const { ref, isDropTarget } = useDroppable({
+    const { ref: droppableRef, isDropTarget } = useDroppable({
        id,
    });
+    const internalRef = useRef<HTMLButtonElement>(null);
+    const mergedRef = useMergeRefs<HTMLButtonElement>([droppableRef, internalRef]);
+
+    useEffect(() => {
+        if (isFocused) {
+            internalRef.current?.focus({ preventScroll: true });
+        }
+    }, [isFocused]);

    return (
        <div
@@ -114,7 +123,7 @@ export const RoomListSectionHeaderView = memo(function RoomListSectionHeaderView
            {...getGroupHeaderAccessibleProps(indexInList, sectionIndex, roomCountInSection)}
        >
            <button
-                ref={ref}
+                ref={mergedRef}
                type="button"
                role="gridcell"
                className={classNames(styles.header, {
@@ -124,6 +133,24 @@ export const RoomListSectionHeaderView = memo(function RoomListSectionHeaderView
                    [styles.unread]: isUnread,
                })}
                onClick={vm.onClick}
+                onKeyDown={(e) => {
+                    if ((e.code === "ArrowRight" && !isExpanded) || (e.code === "ArrowLeft" && isExpanded)) {
+                        e.preventDefault();
+                        e.stopPropagation();
+                        vm.onClick();
+                    } else if (e.code === "ArrowRight" && isExpanded && roomCountInSection > 0) {
+                        // Move focus to the first room in the section
+                        e.preventDefault();
+                        e.stopPropagation();
+                        e.currentTarget.dispatchEvent(
+                            new KeyboardEvent("keydown", {
+                                code: "ArrowDown",
+                                key: "ArrowDown",
+                                bubbles: true,
+                            }),
+                        );
+                    }
+                }}
                aria-expanded={isExpanded}
                onFocus={(e) => onFocus(id, e)}
                tabIndex={isFocused ? 0 : -1}