From 3764224fc249d84218773351724179c4464d985d Mon Sep 17 00:00:00 2001 From: Florian Duros Date: Wed, 25 Mar 2026 15:33:41 +0100 Subject: [PATCH] Remove sticky headers in shared components grouped list (#32908) * fix: use a flat list instead of grouped virtuoso for the grouped list * test: update virtualized list * test: update snapshots --- .../__snapshots__/RoomListView.test.tsx.snap | 155 ++++++------------ .../GroupedVirtualizedList.tsx | 91 +++++----- .../VirtualizedList/virtualized-list.test.tsx | 13 +- 3 files changed, 99 insertions(+), 160 deletions(-) diff --git a/packages/shared-components/src/room-list/RoomListView/__snapshots__/RoomListView.test.tsx.snap b/packages/shared-components/src/room-list/RoomListView/__snapshots__/RoomListView.test.tsx.snap index e6bfaa8dad..8b50070177 100644 --- a/packages/shared-components/src/room-list/RoomListView/__snapshots__/RoomListView.test.tsx.snap +++ b/packages/shared-components/src/room-list/RoomListView/__snapshots__/RoomListView.test.tsx.snap @@ -8283,16 +8283,18 @@ exports[` > renders LargeSectionList story 1`] = ` tabindex="0" >
> renders LargeSectionList story 1`] = `
-
-
-
-
@@ -8501,8 +8492,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -8629,8 +8619,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -8763,8 +8752,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -8891,8 +8879,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9025,8 +9012,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9176,8 +9162,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9310,8 +9295,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9438,8 +9422,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9572,8 +9555,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9700,8 +9682,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9857,8 +9838,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -9985,8 +9965,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -10119,8 +10098,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -10247,8 +10225,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -10381,8 +10358,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -10532,8 +10508,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -10666,8 +10641,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -10794,8 +10768,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -10928,8 +10901,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -11056,8 +11028,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -11213,8 +11184,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -11341,8 +11311,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -11475,9 +11444,9 @@ exports[` > renders LargeSectionList story 1`] = `
> renders LargeSectionList story 1`] = `
@@ -11649,8 +11617,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -11783,8 +11750,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -11934,8 +11900,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -12068,8 +12033,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -12196,8 +12160,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -12330,8 +12293,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -12458,8 +12420,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -12615,8 +12576,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -12743,8 +12703,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -12877,8 +12836,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -13005,8 +12963,7 @@ exports[` > renders LargeSectionList story 1`] = `
@@ -13626,16 +13583,18 @@ exports[` > renders SmallSectionList story 1`] = ` tabindex="0" >
> renders SmallSectionList story 1`] = `
-
-
-
-
@@ -13844,8 +13792,7 @@ exports[` > renders SmallSectionList story 1`] = `
@@ -13972,9 +13919,9 @@ exports[` > renders SmallSectionList story 1`] = `
extends Omit } /** - * A generic grouped virtualized list component built on top of react-virtuoso's GroupedVirtuoso. + * A generic grouped virtualized list component built on top of react-virtuoso's Virtuoso. * Provides keyboard navigation (including group headers) and virtualized rendering for * performance with large lists. * @@ -129,10 +129,8 @@ export function GroupedVirtualizedList( ...restProps } = props; - const groupCounts = useMemo(() => groups.map((group) => group.items.length), [groups]); - const items = useMemo(() => groups.flatMap((group) => group.items), [groups]); - - // Build a flat navigation array interleaving group headers with items. + // Build a flat array interleaving group headers with items. + // Each entry is either { header } or { item }. const flatEntries = useMemo( () => groups.flatMap>((group) => [ @@ -142,34 +140,12 @@ export function GroupedVirtualizedList( [groups], ); - // Build both index-mapping functions in a single pass over the flat entries. - // mapScrollIndex: flat index → GroupedVirtuoso item index (headers map to their - // first item so scrollIntoView makes the sticky header visible). - // mapRangeIndex: GroupedVirtuoso item index → flat index (translates visible-range - // indices back so the hook's PageUp/PageDown and focus-restore logic works). - const { mapScrollIndex, mapRangeIndex } = useMemo(() => { - // Map each flat index to the corresponding virtuoso item index. - // Headers map to the first item of their group so scrollIntoView shows the sticky header. - const flatIndexToVirtuosoIndex: number[] = []; - - // Map the Item index (from virtuoso) to their position in the flat list - const virtuosoIndexToFlatIndex: number[] = []; - let virtuosoIndex = 0; - - for (let i = 0; i < flatEntries.length; i++) { - flatIndexToVirtuosoIndex.push(virtuosoIndex); - - if ("item" in flatEntries[i]) { - virtuosoIndexToFlatIndex.push(i); - virtuosoIndex++; - } - } - - return { - mapScrollIndex: (flatIndex: number): number => flatIndexToVirtuosoIndex[flatIndex] ?? 0, - mapRangeIndex: (virtuosoIndex: number): number => virtuosoIndexToFlatIndex[virtuosoIndex] ?? 0, - }; - }, [flatEntries]); + // Pre-compute a lookup from flat index to group index. + // Each group contributes 1 header + N items, all mapped to the same group index. + const flatIndexToGroupIndex = useMemo( + () => groups.flatMap((group, groupIdx) => new Array(1 + group.items.length).fill(groupIdx)), + [groups], + ); // Wrap getItemKey: dispatch to getHeaderKey or getItemKey based on entry type const wrappedGetEntryKey = useCallback( @@ -178,7 +154,7 @@ export function GroupedVirtualizedList( [getHeaderKey, getItemKey], ); - // Wrap isItemFocusable: headers use isHeaderFocusable (default: always true), items use isItemFocusable + // Wrap isItemFocusable: headers use isGroupHeaderFocusable, items use isItemFocusable const wrappedIsEntryFocusable = useCallback( (entry: NavigationEntry): boolean => "header" in entry ? isGroupHeaderFocusable(entry.header) : isItemFocusable(entry.item), @@ -194,8 +170,6 @@ export function GroupedVirtualizedList( items: flatEntries, isItemFocusable: wrappedIsEntryFocusable, getItemKey: wrappedGetEntryKey, - mapScrollIndex, - mapRangeIndex, }, ); @@ -215,27 +189,44 @@ export function GroupedVirtualizedList( [onFocusForGetItemComponent], ); - const getItemComponentInternal = useCallback( - (index: number, groupIndex: number, _item: unknown, context: VirtualizedListContext): JSX.Element => - getItemComponent(index, items[index], context, onFocusForItem, groupIndex), - [items, getItemComponent, onFocusForItem], - ); + // Unified item renderer that dispatches to group header or item component + // based on the entry type at the given flat index. + const itemContent = useCallback( + ( + flatIndex: number, + _entry: NavigationEntry, + context: VirtualizedListContext, + ): JSX.Element => { + const entry = flatEntries[flatIndex]; + const groupIndex = flatIndexToGroupIndex[flatIndex]; - const getGroupHeaderComponentInternal = useCallback( - (groupIndex: number, context: VirtualizedListContext): JSX.Element => - getGroupHeaderComponent(groupIndex, groups[groupIndex].header, context, onFocusForHeader), - [getGroupHeaderComponent, onFocusForHeader, groups], + if ("header" in entry) { + return getGroupHeaderComponent(groupIndex, entry.header, context, onFocusForHeader); + } + + // Item index in the flattened (non-header) items array: + // flatIndex minus the number of headers before it (groupIndex + 1). + const itemIndex = flatIndex - (groupIndex + 1); + return getItemComponent(itemIndex, entry.item, context, onFocusForItem, groupIndex); + }, + [ + flatEntries, + flatIndexToGroupIndex, + getGroupHeaderComponent, + getItemComponent, + onFocusForItem, + onFocusForHeader, + ], ); return ( - ); diff --git a/packages/shared-components/src/utils/VirtualizedList/virtualized-list.test.tsx b/packages/shared-components/src/utils/VirtualizedList/virtualized-list.test.tsx index 4a6696aaac..4cec04a1f5 100644 --- a/packages/shared-components/src/utils/VirtualizedList/virtualized-list.test.tsx +++ b/packages/shared-components/src/utils/VirtualizedList/virtualized-list.test.tsx @@ -379,12 +379,13 @@ describe.each([flatVariant, groupedVariant])("$name", (variant) // Then press PageUp to jump up by viewport size fireEvent.keyDown(container, { code: "PageUp" }); - // Verify focus moved up - const items = container.querySelectorAll(".mx_item"); - // PageUp should move back to the first item since we only have 4 items - expectTabIndex(items[0], "0"); - const lastIndex = items.length - 1; - expectTabIndex(items[lastIndex], "-1"); + // Verify focus moved up – use the variant's navigable selector so + // group headers (which are also navigable) are included. + const allNav = container.querySelectorAll(variant.navigableSelector); + // PageUp should move back to the first navigable element since we only have a few items + expectTabIndex(allNav[0], "0"); + const lastIndex = allNav.length - 1; + expectTabIndex(allNav[lastIndex], "-1"); }); it("should not handle keyboard navigation when modifier keys are pressed", () => {