New room list: add keyboard navigation support (#29805)

* feat: support up/down arrow navigation in the new room list

* feat: support tabbing in the new room list

* test: update snapshots

* test(e2e): fix room list test

* test(new room list): add landmark navigation test

* test(e2e): update screenshot test

* test: add test to `RoomListItemView`

* test(e2e): add keyboard navigation tests

* refactor: rename `setIsHover` on `setIsHoverWithDelay`

playwright/e2e/left-panel/room-list-panel/room-list.spec.ts

@@ -43,7 +43,8 @@ test.describe("Room list", () => {
             await expect(roomListView.getByRole("gridcell", { name: "Open room room29" })).toBeVisible();
             await expect(roomListView).toMatchScreenshot("room-list.png");
 
-            await roomListView.hover();
+            // Put focus on the room list
+            await roomListView.getByRole("gridcell", { name: "Open room room29" }).click();
             // Scroll to the end of the room list
             await page.mouse.wheel(0, 1000);
             await expect(roomListView.getByRole("gridcell", { name: "Open room room0" })).toBeVisible();
@@ -105,13 +106,10 @@ test.describe("Room list", () => {
             // It should make the room muted
             await page.getByRole("menuitem", { name: "Mute room" }).click();
 
-            // Remove hover on the room list item
+            // Put focus on the room list
-            await roomListView.hover();
+            await roomListView.getByRole("gridcell", { name: "Open room room28" }).click();
-
+            // Scroll to the end of the room list
-            // Scroll to the bottom of the list
+            await page.mouse.wheel(0, 1000);
-            await page.getByRole("grid", { name: "Room list" }).evaluate((e) => {
-                e.scrollTop = e.scrollHeight;
-            });
 
             // The room decoration should have the muted icon
             await expect(roomItem.getByTestId("notification-decoration")).toBeVisible();
@@ -129,7 +127,8 @@ test.describe("Room list", () => {
 
         test("should scroll to the current room", async ({ page, app, user }) => {
             const roomListView = getRoomList(page);
-            await roomListView.hover();
+            // Put focus on the room list
+            await roomListView.getByRole("gridcell", { name: "Open room room29" }).click();
             // Scroll to the end of the room list
             await page.mouse.wheel(0, 1000);
 
@@ -183,6 +182,57 @@ test.describe("Room list", () => {
                 await expect(page.getByRole("heading", { name: "1 notification", level: 1 })).toBeVisible();
             });
         });
+
+        test.describe("Keyboard navigation", () => {
+            test("should navigate to the room list", async ({ page, app, user }) => {
+                const roomListView = getRoomList(page);
+
+                const room29 = roomListView.getByRole("gridcell", { name: "Open room room29" });
+                const room28 = roomListView.getByRole("gridcell", { name: "Open room room28" });
+
+                // open the room
+                await room29.click();
+                // put focus back on the room list item
+                await room29.click();
+                await expect(room29).toBeFocused();
+
+                await page.keyboard.press("ArrowDown");
+                await expect(room28).toBeFocused();
+                await expect(room29).not.toBeFocused();
+
+                await page.keyboard.press("ArrowUp");
+                await expect(room29).toBeFocused();
+                await expect(room28).not.toBeFocused();
+            });
+
+            test("should navigate to the notification menu", async ({ page, app, user }) => {
+                const roomListView = getRoomList(page);
+                const room29 = roomListView.getByRole("gridcell", { name: "Open room room29" });
+                const moreButton = room29.getByRole("button", { name: "More options" });
+                const notificationButton = room29.getByRole("button", { name: "Notification options" });
+
+                await room29.click();
+                // put focus back on the room list item
+                await room29.click();
+                await page.keyboard.press("Tab");
+                await expect(moreButton).toBeFocused();
+                await page.keyboard.press("Tab");
+                await expect(notificationButton).toBeFocused();
+
+                // Open the menu
+                await notificationButton.click();
+                // Wait for the menu to be open
+                await expect(page.getByRole("menuitem", { name: "Match default settings" })).toHaveAttribute(
+                    "aria-selected",
+                    "true",
+                );
+
+                // Close the menu
+                await page.keyboard.press("Escape");
+                // Focus should be back on the room list item
+                await expect(room29).toBeFocused();
+            });
+        });
     });
 
     test.describe("Avatar decoration", () => {

res/css/views/rooms/RoomListPanel/_RoomListItemView.pcss

@@ -18,10 +18,6 @@
     all: unset;
     cursor: pointer;
 
-    &:hover {
-        background-color: var(--cpd-color-bg-action-secondary-hovered);
-    }
-
     .mx_RoomListItemView_container {
         padding-left: var(--cpd-space-2x);
         font: var(--cpd-font-body-md-regular);
@@ -56,12 +52,12 @@
     }
 }
 
-.mx_RoomListItemView_menu_open {
+.mx_RoomListItemView_hover {
     background-color: var(--cpd-color-bg-action-secondary-hovered);
+}
 
-    .mx_RoomListItemView_content {
+.mx_RoomListItemView_menu_open .mx_RoomListItemView_content {
     padding-right: var(--cpd-space-1-5x);
-    }
 }
 
 .mx_RoomListItemView_selected {

src/components/views/rooms/RoomListPanel/RoomList.tsx

@@ -11,6 +11,10 @@ import { AutoSizer, List, type ListRowProps } from "react-virtualized";
 import { type RoomListViewState } from "../../../viewmodels/roomlist/RoomListViewModel";
 import { _t } from "../../../../languageHandler";
 import { RoomListItemView } from "./RoomListItemView";
+import { RovingTabIndexProvider } from "../../../../accessibility/RovingTabIndex";
+import { getKeyBindingsManager } from "../../../../KeyBindingsManager";
+import { KeyBindingAction } from "../../../../accessibility/KeyboardShortcuts";
+import { Landmark, LandmarkNavigation } from "../../../../accessibility/LandmarkNavigation";
 
 interface RoomListProps {
     /**
@@ -32,7 +36,28 @@ export function RoomList({ vm: { rooms, activeIndex } }: RoomListProps): JSX.Ele
 
     // The first div is needed to make the virtualized list take all the remaining space and scroll correctly
     return (
-        <div className="mx_RoomList" data-testid="room-list">
+        <RovingTabIndexProvider handleHomeEnd={true} handleUpDown={true}>
+            {({ onKeyDownHandler }) => (
+                <div
+                    className="mx_RoomList"
+                    data-testid="room-list"
+                    onKeyDown={(ev) => {
+                        const navAction = getKeyBindingsManager().getNavigationAction(ev);
+                        if (
+                            navAction === KeyBindingAction.NextLandmark ||
+                            navAction === KeyBindingAction.PreviousLandmark
+                        ) {
+                            LandmarkNavigation.findAndFocusNextLandmark(
+                                Landmark.ROOM_LIST,
+                                navAction === KeyBindingAction.PreviousLandmark,
+                            );
+                            ev.stopPropagation();
+                            ev.preventDefault();
+                            return;
+                        }
+                        onKeyDownHandler(ev);
+                    }}
+                >
                     <AutoSizer>
                         {({ height, width }) => (
                             <List
@@ -44,9 +69,12 @@ export function RoomList({ vm: { rooms, activeIndex } }: RoomListProps): JSX.Ele
                                 height={height}
                                 width={width}
                                 scrollToIndex={activeIndex ?? 0}
+                                tabIndex={-1}
                             />
                         )}
                     </AutoSizer>
                 </div>
+            )}
+        </RovingTabIndexProvider>
     );
 }

src/components/views/rooms/RoomListPanel/RoomListItemView.tsx

@@ -5,7 +5,7 @@
  * Please see LICENSE files in the repository root for full details.
  */
 
-import React, { type JSX, memo, useState } from "react";
+import React, { type JSX, memo, useCallback, useRef, useState } from "react";
 import { type Room } from "matrix-js-sdk/src/matrix";
 import classNames from "classnames";
 
@@ -14,6 +14,7 @@ import { Flex } from "../../../utils/Flex";
 import { RoomListItemMenuView } from "./RoomListItemMenuView";
 import { NotificationDecoration } from "../NotificationDecoration";
 import { RoomAvatarView } from "../../avatars/RoomAvatarView";
+import { useRovingTabIndex } from "../../../../accessibility/RovingTabIndex";
 
 interface RoomListItemViewProps extends React.HTMLAttributes<HTMLButtonElement> {
     /**
@@ -34,22 +35,29 @@ export const RoomListItemView = memo(function RoomListItemView({
     isSelected,
     ...props
 }: RoomListItemViewProps): JSX.Element {
+    const buttonRef = useRef<HTMLButtonElement>(null);
+    const [onFocus, isActive, ref] = useRovingTabIndex(buttonRef);
+
     const vm = useRoomListItemViewModel(room);
 
-    const [isHover, setIsHover] = useState(false);
+    const [isHover, setIsHoverWithDelay] = useIsHover();
     const [isMenuOpen, setIsMenuOpen] = useState(false);
     // The compound menu in RoomListItemMenuView needs to be rendered when the hover menu is shown
     // Using display: none; and then display:flex when hovered in CSS causes the menu to be misaligned
-    const showHoverDecoration = (isMenuOpen || isHover) && vm.showHoverMenu;
+    const showHoverDecoration = isMenuOpen || isHover;
+    const showHoverMenu = showHoverDecoration && vm.showHoverMenu;
 
-    const isNotificationDecorationVisible = !showHoverDecoration && vm.showNotificationDecoration;
+    const isInvitation = vm.notificationState.invited;
+    const isNotificationDecorationVisible = isInvitation || (!showHoverDecoration && vm.showNotificationDecoration);
 
     return (
         <button
+            ref={ref}
             className={classNames("mx_RoomListItemView", {
                 mx_RoomListItemView_empty: !isNotificationDecorationVisible && !showHoverDecoration,
                 mx_RoomListItemView_notification_decoration: isNotificationDecorationVisible,
-                mx_RoomListItemView_menu_open: showHoverDecoration,
+                mx_RoomListItemView_hover: showHoverDecoration,
+                mx_RoomListItemView_menu_open: showHoverMenu,
                 mx_RoomListItemView_selected: isSelected,
                 mx_RoomListItemView_bold: vm.isBold,
             })}
@@ -57,10 +65,17 @@ export const RoomListItemView = memo(function RoomListItemView({
             aria-selected={isSelected}
             aria-label={vm.a11yLabel}
             onClick={() => vm.openRoom()}
-            onMouseOver={() => setIsHover(true)}
+            onMouseOver={() => setIsHoverWithDelay(true)}
-            onMouseOut={() => setIsHover(false)}
+            onMouseOut={() => setIsHoverWithDelay(false)}
-            onFocus={() => setIsHover(true)}
+            onFocus={() => {
-            onBlur={() => setIsHover(false)}
+                setIsHoverWithDelay(true);
+                onFocus();
+            }}
+            // Adding a timeout because when tabbing to go to the more options and notification menu, the focus moves out of the button
+            // The blur makes the button lose the hover state and these menu are not shown
+            // We delay the blur event to give time to the focus to move to the menu
+            onBlur={() => setIsHoverWithDelay(false, 10)}
+            tabIndex={isActive ? 0 : -1}
             {...props}
         >
             {/* We need this extra div between the button and the content in order to add a padding which is not messing with the virtualized list */}
@@ -79,13 +94,19 @@ export const RoomListItemView = memo(function RoomListItemView({
                         </div>
                         <div className="mx_RoomListItemView_messagePreview">{vm.messagePreview}</div>
                     </div>
-                    {showHoverDecoration ? (
+                    {showHoverMenu ? (
                         <RoomListItemMenuView
                             room={room}
                             setMenuOpen={(isOpen) => {
-                                if (isOpen) setIsMenuOpen(isOpen);
+                                if (isOpen) {
+                                    setIsMenuOpen(isOpen);
+                                } else {
                                     // To avoid icon blinking when closing the menu, we delay the state update
-                                else setTimeout(() => setIsMenuOpen(isOpen), 0);
+                                    setTimeout(() => setIsMenuOpen(isOpen), 0);
+                                    // After closing the menu, we need to set the focus back to the button
+                                    // 10ms because the focus moves to the body and we put back the focus on the button
+                                    setTimeout(() => buttonRef.current?.focus(), 10);
+                                }
                             }}
                         />
                     ) : (
@@ -105,3 +126,33 @@ export const RoomListItemView = memo(function RoomListItemView({
         </button>
     );
 });
+
+/**
+ * Custom hook to manage the hover state of the room list item
+ * If the timeout is set, it will set the hover state after the timeout
+ * If the timeout is not set, it will set the hover state immediately
+ * When the set method is called, it will clear any existing timeout
+ *
+ * @returns {boolean} isHover - The hover state
+ */
+function useIsHover(): [boolean, (value: boolean, timeout?: number) => void] {
+    const [isHover, setIsHover] = useState(false);
+    // Store the timeout ID
+    const timeoutRef = useRef<number | undefined>(undefined);
+
+    const setIsHoverWithDelay = useCallback((value: boolean, timeout?: number): void => {
+        // Clear the timeout if it exists
+        clearTimeout(timeoutRef.current);
+
+        // No delay, set the value immediately
+        if (timeout === undefined) {
+            setIsHover(value);
+            return;
+        }
+
+        // Set a timeout to set the value after the delay
+        timeoutRef.current = setTimeout(() => setIsHover(value), timeout);
+    }, []);
+
+    return [isHover, setIsHoverWithDelay];
+}

test/unit-tests/components/views/rooms/RoomListPanel/RoomList-test.tsx

@@ -8,6 +8,7 @@
 import React from "react";
 import { type MatrixClient } from "matrix-js-sdk/src/matrix";
 import { render } from "jest-matrix-react";
+import { fireEvent } from "@testing-library/dom";
 
 import { mkRoom, stubClient } from "../../../../../test-utils";
 import { type RoomListViewState } from "../../../../../../src/components/viewmodels/roomlist/RoomListViewModel";
@@ -15,6 +16,7 @@ import { RoomList } from "../../../../../../src/components/views/rooms/RoomListP
 import DMRoomMap from "../../../../../../src/utils/DMRoomMap";
 import { SecondaryFilters } from "../../../../../../src/components/viewmodels/roomlist/useFilteredRooms";
 import { SortOption } from "../../../../../../src/components/viewmodels/roomlist/useSorter";
+import { Landmark, LandmarkNavigation } from "../../../../../../src/accessibility/LandmarkNavigation";
 
 describe("<RoomList />", () => {
     let matrixClient: MatrixClient;
@@ -53,4 +55,16 @@ describe("<RoomList />", () => {
         const { asFragment } = render(<RoomList vm={vm} />);
         expect(asFragment()).toMatchSnapshot();
     });
+
+    it.each([
+        { shortcut: { key: "F6", ctrlKey: true, shiftKey: true }, isPreviousLandmark: true, label: "PreviousLandmark" },
+        { shortcut: { key: "F6", ctrlKey: true }, isPreviousLandmark: false, label: "NextLandmark" },
+    ])("should navigate to the landmark on NextLandmark.$label action", ({ shortcut, isPreviousLandmark }) => {
+        const spyFindLandmark = jest.spyOn(LandmarkNavigation, "findAndFocusNextLandmark").mockReturnValue();
+        const { getByTestId } = render(<RoomList vm={vm} />);
+        const roomList = getByTestId("room-list");
+        fireEvent.keyDown(roomList, shortcut);
+
+        expect(spyFindLandmark).toHaveBeenCalledWith(Landmark.ROOM_LIST, isPreviousLandmark);
+    });
 });

test/unit-tests/components/views/rooms/RoomListPanel/RoomListItemView-test.tsx

@@ -91,6 +91,17 @@ describe("<RoomListItemView />", () => {
         await waitFor(() => expect(screen.getByRole("button", { name: "More Options" })).toBeInTheDocument());
     });
 
+    test("should hover decoration if focused", async () => {
+        const user = userEvent.setup();
+        render(<RoomListItemView room={room} isSelected={false} />, withClientContextRenderOptions(matrixClient));
+        const listItem = screen.getByRole("button", { name: `Open room ${room.name}` });
+        await user.click(listItem);
+        expect(listItem).toHaveClass("mx_RoomListItemView_hover");
+
+        await user.tab();
+        await waitFor(() => expect(listItem).not.toHaveClass("mx_RoomListItemView_hover"));
+    });
+
     test("should be selected if isSelected=true", async () => {
         const { asFragment } = render(<RoomListItemView room={room} isSelected={true} />);
         expect(screen.queryByRole("button", { name: `Open room ${room.name}` })).toHaveAttribute(

test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomList-test.tsx.snap

@@ -15,7 +15,7 @@ exports[`<RoomList /> should render a room list 1`] = `
         class="ReactVirtualized__Grid ReactVirtualized__List mx_RoomList_List"
         role="grid"
         style="box-sizing: border-box; direction: ltr; height: 1500px; position: relative; width: 1500px; will-change: transform; overflow-x: hidden; overflow-y: hidden;"
-        tabindex="0"
+        tabindex="-1"
       >
         <div
           class="ReactVirtualized__Grid__innerScrollContainer"
@@ -28,6 +28,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 0px; width: 100%;"
+            tabindex="0"
             type="button"
           >
             <div
@@ -79,6 +80,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 48px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -130,6 +132,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 96px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -181,6 +184,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 144px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -232,6 +236,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 192px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -283,6 +288,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 240px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -334,6 +340,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 288px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -385,6 +392,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 336px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -436,6 +444,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 384px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div
@@ -487,6 +496,7 @@ exports[`<RoomList /> should render a room list 1`] = `
             class="mx_RoomListItemView mx_RoomListItemView_empty"
             role="gridcell"
             style="height: 48px; left: 0px; position: absolute; top: 432px; width: 100%;"
+            tabindex="-1"
             type="button"
           >
             <div

test/unit-tests/components/views/rooms/RoomListPanel/__snapshots__/RoomListItemView-test.tsx.snap

@@ -6,6 +6,7 @@ exports[`<RoomListItemView /> should be selected if isSelected=true 1`] = `
     aria-label="Open room room1"
     aria-selected="true"
     class="mx_RoomListItemView mx_RoomListItemView_empty mx_RoomListItemView_selected"
+    tabindex="-1"
     type="button"
   >
     <div
@@ -60,6 +61,7 @@ exports[`<RoomListItemView /> should display notification decoration 1`] = `
     aria-label="Open room room1"
     aria-selected="false"
     class="mx_RoomListItemView mx_RoomListItemView_notification_decoration"
+    tabindex="-1"
     type="button"
   >
     <div
@@ -126,6 +128,7 @@ exports[`<RoomListItemView /> should render a room item 1`] = `
     aria-label="Open room room1"
     aria-selected="false"
     class="mx_RoomListItemView mx_RoomListItemView_empty"
+    tabindex="-1"
     type="button"
   >
     <div
@@ -180,6 +183,7 @@ exports[`<RoomListItemView /> should render a room item with a message preview 1
     aria-label="Open room room1"
     aria-selected="false"
     class="mx_RoomListItemView mx_RoomListItemView_empty"
+    tabindex="-1"
     type="button"
   >
     <div