Room list: fix keyboard navigation (#32585)

* feat: add visual feedback of selection in `VirtualizedList` story

* fix: keyboard navigation when focused element is no longer in the dom

* fix: selection delay when the list is scrolled

* fix: room list item scroll issue

Avoid to use margin https://virtuoso.dev/react-virtuoso/#caveats

* test: add tests

* test(e2e): update snapshots

* test: update room list item screenshot

packages/shared-components/src/room-list/RoomListItemView/RoomListItemView.module.css

@@ -15,10 +15,11 @@
     /* Needed to position the selected marker */
     position: relative;
     cursor: pointer;
-    min-height: 44px;
     width: 100%;
     /* Gap between items, accounted for in ROOM_LIST_ITEM_HEIGHT */
-    margin-bottom: var(--cpd-space-2x);
+    padding-bottom: var(--cpd-space-2x);
+    /* 44px height + padding bottom */
+    min-height: calc(44px + var(--cpd-space-2x));
 
     font: var(--cpd-font-body-md-regular);
     letter-spacing: var(--cpd-font-letter-spacing-body-md);
@@ -92,9 +93,10 @@
     .container::before {
         content: "";
         position: absolute;
-        /* Marker height is 34px, room list item height is 44px. So position is at 5px and top and bottom */
+        /* Marker height is 34px, room list item height is 44px. So position is at 5px at and bottom */
         top: 5px;
-        bottom: 5px;
+        /* Add the padding bottom to center */
+        bottom: calc(5px + var(--cpd-space-2x));
         left: 0;
         width: 4px;
         background-color: var(--cpd-color-bg-accent-rest);

packages/shared-components/src/room-list/VirtualizedRoomListView/VirtualizedRoomListView.tsx

@@ -53,7 +53,7 @@ export interface VirtualizedRoomListViewProps {
     onKeyDown?: (e: React.KeyboardEvent<HTMLDivElement>) => void;
 }
 
-/** Height of a single room list item in pixels (44px item + 8px margin bottom) */
+/** Height of a single room list item in pixels (44px item + 8px padding bottom) */
 const ROOM_LIST_ITEM_HEIGHT = 52;
 
 /**

packages/shared-components/src/utils/VirtualizedList/VirtualizedList.stories.tsx

@@ -6,9 +6,11 @@ Please see LICENSE files in the repository root for full details.
 */
 
 import React from "react";
+import classNames from "classnames";
 
 import type { Meta, StoryObj } from "@storybook/react-vite";
 import { VirtualizedList, type IVirtualizedListProps, type VirtualizedListContext } from "./VirtualizedList";
+import styles from "./story-mock.module.css";
 
 interface SimpleItem {
     id: string;
@@ -30,16 +32,21 @@ const meta = {
             item: SimpleItem,
             context: VirtualizedListContext<undefined>,
             onFocus: (item: SimpleItem, e: React.FocusEvent) => void,
-        ) => (
-            <div
-                key={item.id}
-                style={{ padding: "12px 16px", borderBottom: "1px solid #e0e0e0" }}
-                tabIndex={context.tabIndexKey === item.id ? 0 : -1}
-                onFocus={(e) => onFocus(item, e)}
-            >
-                {item.label}
-            </div>
-        ),
+        ) => {
+            const selected = context.tabIndexKey === item.id;
+
+            return (
+                <button
+                    type="button"
+                    key={item.id}
+                    className={classNames(styles.item, { [styles.itemSelected]: selected })}
+                    tabIndex={selected ? 0 : -1}
+                    onFocus={(e) => onFocus(item, e)}
+                >
+                    {item.label}
+                </button>
+            );
+        },
         isItemFocusable: () => true,
         getItemKey: (item) => item.id,
         style: { height: "400px" },

packages/shared-components/src/utils/VirtualizedList/VirtualizedList.test.tsx

@@ -6,7 +6,7 @@
  */
 
 import React, { type PropsWithChildren } from "react";
-import { render, screen, fireEvent } from "@test-utils";
+import { render, screen, fireEvent, waitFor, act } from "@test-utils";
 import { VirtuosoMockContext } from "react-virtuoso";
 import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
 
@@ -492,4 +492,171 @@ describe("VirtualizedList", () => {
             expectAttribute(container, "aria-label", "Custom list label");
         });
     });
+
+    describe("Focus preservation during keyboard navigation", () => {
+        /**
+         * Renders a 50-item list using real Virtuoso (no mock context) inside a
+         * fixed-height container.  Because the tests run in real Chromium,
+         * Virtuoso will measure the viewport, virtualise items, and honour
+         * scrollIntoView calls exactly as it does in production.
+         */
+        const ITEM_HEIGHT = 52;
+        const VIEWPORT_HEIGHT = 400;
+
+        const renderRealVirtualizedList = (): ReturnType<typeof render> => {
+            const largeItems: TestItemWithSeparator[] = Array.from({ length: 50 }, (_, i) => ({
+                id: `item-${i}`,
+                name: `Item ${i}`,
+            }));
+
+            mockIsItemFocusable.mockReturnValue(true);
+
+            mockGetItemComponent.mockImplementation(
+                (
+                    index: number,
+                    item: TestItemWithSeparator,
+                    context: any,
+                    onFocus: (item: TestItemWithSeparator, e: React.FocusEvent) => void,
+                ) => {
+                    const itemKey = typeof item === "string" ? item : item.id;
+                    const isFocused = context.tabIndexKey === itemKey;
+                    return (
+                        <button
+                            type="button"
+                            className="mx_item"
+                            data-testid={`row-${index}`}
+                            tabIndex={isFocused ? 0 : -1}
+                            role="gridcell"
+                            style={{ height: `${ITEM_HEIGHT}px`, display: "block", width: "100%" }}
+                            onFocus={(e) => onFocus(item, e)}
+                        >
+                            {item === SEPARATOR_ITEM ? "---" : (item as TestItem).name}
+                        </button>
+                    );
+                },
+            );
+
+            return render(
+                <VirtualizedList
+                    items={largeItems}
+                    getItemComponent={mockGetItemComponent}
+                    isItemFocusable={mockIsItemFocusable}
+                    getItemKey={(item) => (typeof item === "string" ? item : item.id)}
+                    role="grid"
+                    style={{ height: `${VIEWPORT_HEIGHT}px` }}
+                    fixedItemHeight={ITEM_HEIGHT}
+                />,
+            );
+        };
+
+        it("should scroll down through many items with ArrowDown and virtualise earlier items out of the DOM", async () => {
+            const { container } = renderRealVirtualizedList();
+            const listContainer = screen.getByRole("grid");
+
+            // Wait for Virtuoso to finish its initial render.
+            await waitFor(() => {
+                expect(screen.getByTestId("row-0")).toBeDefined();
+            });
+
+            fireEvent.focus(listContainer);
+
+            const TARGET_INDEX = 20;
+
+            // Press ArrowDown many times — each press calls scrollIntoView which
+            // makes Virtuoso scroll and re-virtualise automatically.
+            for (let i = 0; i < TARGET_INDEX; i++) {
+                await act(async () => {
+                    fireEvent.keyDown(listContainer, { code: "ArrowDown" });
+                });
+            }
+
+            // The focused item should be item-20.
+            await waitFor(() => {
+                const focused = Array.from(container.querySelectorAll(".mx_item")).find(
+                    (el) => el.getAttribute("tabindex") === "0",
+                );
+                expect(focused).toBeDefined();
+                expect(focused!.textContent).toBe(`Item ${TARGET_INDEX}`);
+            });
+
+            // The first item should have been virtualised out of the DOM.
+            expect(container.querySelector("[data-testid='row-0']")).toBeNull();
+        });
+
+        it("should move focus from a focused child element to the scroller on keyboard navigation", async () => {
+            renderRealVirtualizedList();
+            const listContainer = screen.getByRole("grid");
+
+            // Wait for Virtuoso to finish its initial render.
+            await waitFor(() => {
+                expect(screen.getByTestId("row-0")).toBeDefined();
+            });
+
+            // Directly focus a child button (not the scroller itself).
+            // This simulates a user clicking/tabbing into a button inside the list.
+            const firstButton = screen.getByTestId("row-0");
+            await act(async () => {
+                firstButton.focus();
+            });
+
+            // Verify the child button has DOM focus, not the scroller.
+            expect(document.activeElement).toBe(firstButton);
+            expect(document.activeElement).not.toBe(listContainer);
+
+            // Press ArrowDown — the handler should detect that a child element
+            // has focus and move it to the scroller before scrolling, so that
+            // Virtuoso unmounting the child doesn't send focus to <body>.
+            await act(async () => {
+                fireEvent.keyDown(listContainer, { code: "ArrowDown" });
+            });
+
+            // After the keyDown, focus should have moved to the scroller element
+            // (not remain on the child button, and not escape to <body>).
+            expect(document.activeElement).toBe(listContainer);
+        });
+
+        it("should scroll up through many items with ArrowUp and virtualise later items out of the DOM", async () => {
+            const { container } = renderRealVirtualizedList();
+            const listContainer = screen.getByRole("grid");
+
+            await waitFor(() => {
+                expect(screen.getByTestId("row-0")).toBeDefined();
+            });
+
+            fireEvent.focus(listContainer);
+
+            // First navigate down to item-30.
+            for (let i = 0; i < 30; i++) {
+                await act(async () => {
+                    fireEvent.keyDown(listContainer, { code: "ArrowDown" });
+                });
+            }
+
+            await waitFor(() => {
+                const focused = Array.from(container.querySelectorAll(".mx_item")).find(
+                    (el) => el.getAttribute("tabindex") === "0",
+                );
+                expect(focused!.textContent).toBe("Item 30");
+            });
+
+            // Now navigate back up 20 times to item-10.
+            for (let i = 0; i < 20; i++) {
+                await act(async () => {
+                    fireEvent.keyDown(listContainer, { code: "ArrowUp" });
+                });
+            }
+
+            // The focused item should be item-10.
+            await waitFor(() => {
+                const focused = Array.from(container.querySelectorAll(".mx_item")).find(
+                    (el) => el.getAttribute("tabindex") === "0",
+                );
+                expect(focused).toBeDefined();
+                expect(focused!.textContent).toBe("Item 10");
+            });
+
+            // Items near the bottom (e.g. item-30) should have been virtualised out.
+            expect(container.querySelector("[data-testid='row-30']")).toBeNull();
+        });
+    });
 });

packages/shared-components/src/utils/VirtualizedList/VirtualizedList.tsx

@@ -149,8 +149,6 @@ export function VirtualizedList<Item, Context>(props: IVirtualizedListProps<Item
     const [visibleRange, setVisibleRange] = useState<ListRange | undefined>(undefined);
     /** Map from item keys to their indices in the items array */
     const [keyToIndexMap, setKeyToIndexMap] = useState<Map<string, number>>(new Map());
-    /** Whether the list is currently scrolling to an item */
-    const isScrollingToItem = useRef<boolean>(false);
     /** Whether the list is currently focused */
     const [isFocused, setIsFocused] = useState<boolean>(false);
 
@@ -173,28 +171,20 @@ export function VirtualizedList<Item, Context>(props: IVirtualizedListProps<Item
 
     /**
      * Scrolls to a specific item index and sets it as focused.
-     * Uses Virtuoso's scrollIntoView method for smooth scrolling.
+     * Updates tabIndexKey immediately so the UI reflects the new focus
+     * synchronously, then asks Virtuoso to scroll the item into view.
      */
     const scrollToIndex = useCallback(
         (index: number, align?: "center" | "end" | "start"): void => {
             // Ensure index is within bounds
             const clampedIndex = Math.max(0, Math.min(index, items.length - 1));
-            if (isScrollingToItem.current) {
-                // If already scrolling to an item drop this request. Adding further requests
-                // causes the event to bubble up and be handled by other components(unintentional timeline scrolling was observed).
-                return;
-            }
             if (items[clampedIndex]) {
                 const key = getItemKey(items[clampedIndex]);
-                isScrollingToItem.current = true;
+                setTabIndexKey(key);
                 virtuosoHandleRef.current?.scrollIntoView({
                     index: clampedIndex,
                     align: align,
                     behavior: "auto",
-                    done: () => {
-                        setTabIndexKey(key);
-                        isScrollingToItem.current = false;
-                    },
                 });
             }
         },
@@ -266,6 +256,17 @@ export function VirtualizedList<Item, Context>(props: IVirtualizedListProps<Item
             }
 
             if (handled) {
+                // If a child element (e.g. a button) currently has DOM focus rather than the
+                // scroller itself, move focus to the scroller before the scroll takes effect.
+                // Without this, when Virtuoso unmounts the focused child because it has been
+                // scrolled out of the visible range, the browser moves focus to <body> and
+                // subsequent keyboard events no longer reach this handler.
+                if (virtuosoDomRef.current instanceof HTMLElement) {
+                    const activeEl = document.activeElement;
+                    if (activeEl && activeEl !== virtuosoDomRef.current && virtuosoDomRef.current.contains(activeEl)) {
+                        virtuosoDomRef.current.focus({ preventScroll: true });
+                    }
+                }
                 e.stopPropagation();
                 e.preventDefault();
             } else {

packages/shared-components/src/utils/VirtualizedList/story-mock.module.css

@@ -0,0 +1,17 @@
+/*
+ * Copyright 2025 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.
+ */
+
+.item {
+    all: unset;
+    width: 100%;
+    padding: 12px 16px;
+    border-bottom: 1px solid #e0e0e0;
+}
+
+.itemSelected {
+    background-color: #559f24;
+}