diff --git a/packages/shared-components/__vis__/linux/__baselines__/utils/VirtualizedList/VirtualizedList.stories.tsx/default-auto.png b/packages/shared-components/__vis__/linux/__baselines__/utils/VirtualizedList/VirtualizedList.stories.tsx/default-auto.png index dbab024550..9dc97a2cfd 100644 Binary files a/packages/shared-components/__vis__/linux/__baselines__/utils/VirtualizedList/VirtualizedList.stories.tsx/default-auto.png and b/packages/shared-components/__vis__/linux/__baselines__/utils/VirtualizedList/VirtualizedList.stories.tsx/default-auto.png differ diff --git a/packages/shared-components/src/room-list/RoomListItemView/RoomListItemView.module.css b/packages/shared-components/src/room-list/RoomListItemView/RoomListItemView.module.css index ad0241f6c6..ce39516d6b 100644 --- a/packages/shared-components/src/room-list/RoomListItemView/RoomListItemView.module.css +++ b/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); diff --git a/packages/shared-components/src/room-list/VirtualizedRoomListView/VirtualizedRoomListView.tsx b/packages/shared-components/src/room-list/VirtualizedRoomListView/VirtualizedRoomListView.tsx index 5eb17ab27f..6a4ec3cb9e 100644 --- a/packages/shared-components/src/room-list/VirtualizedRoomListView/VirtualizedRoomListView.tsx +++ b/packages/shared-components/src/room-list/VirtualizedRoomListView/VirtualizedRoomListView.tsx @@ -53,7 +53,7 @@ export interface VirtualizedRoomListViewProps { onKeyDown?: (e: React.KeyboardEvent) => 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; /** diff --git a/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.stories.tsx b/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.stories.tsx index a20fe188a3..e2b9ef3626 100644 --- a/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.stories.tsx +++ b/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, onFocus: (item: SimpleItem, e: React.FocusEvent) => void, - ) => ( -
onFocus(item, e)} - > - {item.label} -
- ), + ) => { + const selected = context.tabIndexKey === item.id; + + return ( + + ); + }, isItemFocusable: () => true, getItemKey: (item) => item.id, style: { height: "400px" }, diff --git a/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.test.tsx b/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.test.tsx index 4dd5125b6f..e6564f3f43 100644 --- a/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.test.tsx +++ b/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 => { + 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 ( + + ); + }, + ); + + return render( + (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 . + 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 ). + 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(); + }); + }); }); diff --git a/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.tsx b/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.tsx index 522e65bda3..67fc1b6d02 100644 --- a/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.tsx +++ b/packages/shared-components/src/utils/VirtualizedList/VirtualizedList.tsx @@ -149,8 +149,6 @@ export function VirtualizedList(props: IVirtualizedListProps(undefined); /** Map from item keys to their indices in the items array */ const [keyToIndexMap, setKeyToIndexMap] = useState>(new Map()); - /** Whether the list is currently scrolling to an item */ - const isScrollingToItem = useRef(false); /** Whether the list is currently focused */ const [isFocused, setIsFocused] = useState(false); @@ -173,28 +171,20 @@ export function VirtualizedList(props: IVirtualizedListProps { // 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(props: IVirtualizedListProps 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 { diff --git a/packages/shared-components/src/utils/VirtualizedList/story-mock.module.css b/packages/shared-components/src/utils/VirtualizedList/story-mock.module.css new file mode 100644 index 0000000000..87a9346ef4 --- /dev/null +++ b/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; +} diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list-filter-sort.spec.ts/unread-dm-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list-filter-sort.spec.ts/unread-dm-linux.png index 9951b21ad4..1a997d2a3c 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list-filter-sort.spec.ts/unread-dm-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list-filter-sort.spec.ts/unread-dm-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-activity-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-activity-linux.png index f3b3b7af59..03df202135 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-activity-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-activity-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-linux.png index d80be4840f..1233217838 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-silent-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-silent-linux.png index 4c3f016693..117a352371 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-silent-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-hover-silent-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-invited-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-invited-linux.png index b1d313bdee..bc68ca3754 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-invited-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-invited-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png index 7569db5ca2..2a387714b6 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-low-priority-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mark-as-unread-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mark-as-unread-linux.png index 92d95f52f7..a479cb30a5 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mark-as-unread-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mark-as-unread-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mention-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mention-linux.png index fe3c039fb7..f385d3729d 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mention-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-mention-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-message-preview-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-message-preview-linux.png index 5842bf50f6..fbeff277b0 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-message-preview-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-message-preview-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-notification-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-notification-linux.png index b8125ad4c5..c09f5e9e4d 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-notification-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-notification-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-more-options-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-more-options-linux.png index 9d813b03c5..65e7918ec9 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-more-options-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-more-options-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-linux.png index d946c3dea4..10e8182fb3 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-selection-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-selection-linux.png index c591eea428..aa7db5dd46 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-selection-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-open-notification-options-selection-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-public-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-public-linux.png index 653d2ddcb8..4f1092cb9f 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-public-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-public-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-silent-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-silent-linux.png index 5e433f591d..c96b407145 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-silent-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-silent-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-video-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-video-linux.png index 18ae6d4ce2..aecedc3158 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-video-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-item-video-linux.png differ diff --git a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-scrolled-linux.png b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-scrolled-linux.png index 4d2e56a921..5230140b46 100644 Binary files a/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-scrolled-linux.png and b/playwright/snapshots/left-panel/room-list-panel/room-list.spec.ts/room-list-scrolled-linux.png differ diff --git a/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-no-avatar-linux.png b/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-no-avatar-linux.png index 3210a498af..5abc059329 100644 Binary files a/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-no-avatar-linux.png and b/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-no-avatar-linux.png differ diff --git a/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-with-avatar-linux.png b/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-with-avatar-linux.png index 6e0a26cc6b..62c0d1e07c 100644 Binary files a/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-with-avatar-linux.png and b/playwright/snapshots/timeline/media-preview-settings.spec.ts/invite-room-tree-with-avatar-linux.png differ