From 441c39c0ef26488b19309e107534bc92fc061d9b Mon Sep 17 00:00:00 2001 From: srijna Date: Mon, 22 Jun 2026 14:44:42 +0530 Subject: [PATCH 1/3] feat: migrate sidebar room list to virtua --- .../client/sidebar/RoomList/RoomList.spec.tsx | 181 ++++++++++++++++++ .../client/sidebar/RoomList/RoomList.tsx | 65 ++++--- .../sidebar/RoomList/RoomListWrapper.spec.tsx | 27 +++ .../sidebar/RoomList/RoomListWrapper.tsx | 14 +- .../meteor/client/sidebar/Sidebar.stories.tsx | 107 +++++++++-- .../SidebarVirtualList.spec.tsx | 158 +++++++++++++++ .../SidebarVirtualList/SidebarVirtualList.tsx | 67 +++++++ .../components/SidebarVirtualList/index.ts | 2 + 8 files changed, 579 insertions(+), 42 deletions(-) create mode 100644 apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx create mode 100644 apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx create mode 100644 apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx create mode 100644 apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx create mode 100644 apps/meteor/client/sidebar/components/SidebarVirtualList/index.ts diff --git a/apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx b/apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx new file mode 100644 index 0000000000000..2afaba1b68a8c --- /dev/null +++ b/apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx @@ -0,0 +1,181 @@ +import { render, screen } from '@testing-library/react'; +import type { HTMLAttributes, ReactNode } from 'react'; +import { forwardRef } from 'react'; + +import RoomList from './RoomList'; + +const mockCollapsedGroups: string[] = []; +const mockHandleClick = jest.fn(); +const mockHandleKeyDown = jest.fn(); +const mockUsePreventDefault = jest.fn(); +const mockUseShortcutOpenMenu = jest.fn(); + +const rooms = [ + { _id: 'general', name: 'general' }, + { _id: 'support', name: 'support' }, + { _id: 'alice', name: 'alice' }, +]; + +const groupedUnreadInfo = [ + { userMentions: 1, groupMentions: 0, unread: 3, tunread: [], tunreadUser: [], tunreadGroup: [] }, + { userMentions: 0, groupMentions: 1, unread: 1, tunread: [], tunreadUser: [], tunreadGroup: [] }, + { userMentions: 0, groupMentions: 0, unread: 0, tunread: [], tunreadUser: [], tunreadGroup: [] }, +]; + +type MockSidebarVirtualListProps = { + groups: { + key: string; + group: { + groupTitle: string; + unreadCount: (typeof groupedUnreadInfo)[number]; + }; + items: typeof rooms; + }[]; + renderGroup: (group: { groupTitle: string; unreadCount: (typeof groupedUnreadInfo)[number] }, groupIndex: number) => ReactNode; + renderItem: (item: (typeof rooms)[number], itemIndex: number, group: { groupTitle: string }, groupIndex: number) => ReactNode; + getItemKey: (item: (typeof rooms)[number]) => string; + overscan?: number; +}; + +const mockSidebarVirtualList = jest.fn(({ groups, renderGroup, renderItem, getItemKey }: MockSidebarVirtualListProps) => ( +
+ {groups.map(({ key, group, items }, groupIndex) => ( +
+ {renderGroup(group, groupIndex)} + {items.map((item, itemIndex) => ( +
+ {renderItem(item, itemIndex, group, groupIndex)} +
+ ))} +
+ ))} +
+)); + +jest.mock('@rocket.chat/fuselage', () => ({ + Box: forwardRef>(function Box({ children, ...props }, ref) { + return ( +
+ {children} +
+ ); + }), +})); + +jest.mock('@rocket.chat/ui-contexts', () => ({ + useUserId: () => 'user-id', + useUserPreference: () => 'extended', +})); + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +jest.mock('../components/SidebarVirtualList', () => ({ + __esModule: true, + default: (props: MockSidebarVirtualListProps) => mockSidebarVirtualList(props), +})); + +jest.mock('../../lib/RoomManager', () => ({ + useOpenedRoom: () => 'GENERAL', +})); + +jest.mock('../hooks/useAvatarTemplate', () => ({ + useAvatarTemplate: () => 'AvatarTemplate', +})); + +jest.mock('../hooks/useCollapsedGroups', () => ({ + useCollapsedGroups: () => ({ + collapsedGroups: mockCollapsedGroups, + handleClick: mockHandleClick, + handleKeyDown: mockHandleKeyDown, + }), +})); + +jest.mock('../hooks/usePreventDefault', () => ({ + usePreventDefault: (ref: unknown) => mockUsePreventDefault(ref), +})); + +jest.mock('../hooks/useRoomList', () => ({ + useRoomList: () => ({ + groupsCount: [2, 1, 0], + groupsList: ['Channels', 'Direct Messages', 'Empty Group'], + roomList: rooms, + groupedUnreadInfo, + }), +})); + +jest.mock('../hooks/useShortcutOpenMenu', () => ({ + useShortcutOpenMenu: (ref: unknown) => mockUseShortcutOpenMenu(ref), +})); + +jest.mock('../hooks/useTemplateByViewMode', () => ({ + useTemplateByViewMode: () => 'SidebarItemTemplate', +})); + +jest.mock('./RoomListCollapser', () => ({ + __esModule: true, + default: ({ groupTitle, unreadCount }: { groupTitle: string; unreadCount: (typeof groupedUnreadInfo)[number] }) => ( + + ), +})); + +jest.mock('./RoomListRow', () => ({ + __esModule: true, + default: ({ item }: { item: (typeof rooms)[number] }) =>
{item.name}
, +})); + +jest.mock('./RoomListRowWrapper', () => ({ + __esModule: true, + default: ({ children }: { children: ReactNode }) =>
{children}
, +})); + +describe('RoomList', () => { + beforeEach(() => { + jest.clearAllMocks(); + }); + + it('passes grouped room data to SidebarVirtualList', () => { + render(); + + expect(screen.getByTestId('sidebar-virtual-list')).toBeInTheDocument(); + expect(mockSidebarVirtualList).toHaveBeenCalledWith( + expect.objectContaining({ + groups: [ + { + key: '0:Channels', + group: { + groupTitle: 'Channels', + unreadCount: groupedUnreadInfo[0], + }, + items: [rooms[0], rooms[1]], + }, + { + key: '1:Direct Messages', + group: { + groupTitle: 'Direct Messages', + unreadCount: groupedUnreadInfo[1], + }, + items: [rooms[2]], + }, + { + key: '2:Empty Group', + group: { + groupTitle: 'Empty Group', + unreadCount: groupedUnreadInfo[2], + }, + items: [], + }, + ], + overscan: 25, + }), + ); + expect(screen.getAllByTestId('group-header').map((element) => element.textContent)).toEqual([ + 'Channels:3', + 'Direct Messages:1', + 'Empty Group:0', + ]); + expect(screen.getAllByTestId('room-row-wrapper')).toHaveLength(3); + expect(screen.getAllByTestId('room-row').map((element) => element.textContent)).toEqual(['general', 'support', 'alice']); + }); +}); diff --git a/apps/meteor/client/sidebar/RoomList/RoomList.tsx b/apps/meteor/client/sidebar/RoomList/RoomList.tsx index 2403621675507..28613cb5736e3 100644 --- a/apps/meteor/client/sidebar/RoomList/RoomList.tsx +++ b/apps/meteor/client/sidebar/RoomList/RoomList.tsx @@ -1,16 +1,14 @@ import { Box } from '@rocket.chat/fuselage'; -import { useResizeObserver } from '@rocket.chat/fuselage-hooks'; -import { VirtualizedScrollbars } from '@rocket.chat/ui-client'; import { useUserPreference, useUserId } from '@rocket.chat/ui-contexts'; -import { useMemo } from 'react'; +import { useMemo, useRef } from 'react'; import { useTranslation } from 'react-i18next'; -import { GroupedVirtuoso } from 'react-virtuoso'; import RoomListCollapser from './RoomListCollapser'; import RoomListRow from './RoomListRow'; import RoomListRowWrapper from './RoomListRowWrapper'; import RoomListWrapper from './RoomListWrapper'; import { useOpenedRoom } from '../../lib/RoomManager'; +import SidebarVirtualList from '../components/SidebarVirtualList'; import { useAvatarTemplate } from '../hooks/useAvatarTemplate'; import { useCollapsedGroups } from '../hooks/useCollapsedGroups'; import { usePreventDefault } from '../hooks/usePreventDefault'; @@ -26,7 +24,7 @@ const RoomList = () => { const { groupsCount, groupsList, roomList, groupedUnreadInfo } = useRoomList({ collapsedGroups }); const avatarTemplate = useAvatarTemplate(); const sideBarItemTemplate = useTemplateByViewMode(); - const { ref } = useResizeObserver({ debounceDelay: 100 }); + const ref = useRef(null); const openedRoom = useOpenedRoom() ?? ''; const sidebarViewMode = useUserPreference<'extended' | 'medium' | 'condensed'>('sidebarViewMode') || 'extended'; @@ -44,29 +42,50 @@ const RoomList = () => { [avatarTemplate, extended, isAnonymous, openedRoom, sideBarItemTemplate, sidebarViewMode, t], ); + const virtualGroups = useMemo(() => { + let roomOffset = 0; + + return groupsList.map((groupTitle, groupIndex) => { + const count = groupsCount[groupIndex] ?? 0; + const items = roomList.slice(roomOffset, roomOffset + count); + roomOffset += count; + + return { + key: `${groupIndex}:${groupTitle}`, + group: { + groupTitle, + unreadCount: groupedUnreadInfo[groupIndex], + }, + items, + }; + }); + }, [groupedUnreadInfo, groupsCount, groupsList, roomList]); + usePreventDefault(ref); useShortcutOpenMenu(ref); return ( - - ( - handleClick(groupsList[index])} - onKeyDown={(e) => handleKeyDown(e, groupsList[index])} - groupTitle={groupsList[index]} - unreadCount={groupedUnreadInfo[index]} - /> - )} - {...(roomList.length > 0 && { - itemContent: (index) => roomList[index] && , - })} - components={{ Item: RoomListRowWrapper, List: RoomListWrapper }} - /> - + item._id} + renderGroup={({ groupTitle, unreadCount }) => ( + handleClick(groupTitle)} + onKeyDown={(e) => handleKeyDown(e, groupTitle)} + groupTitle={groupTitle} + unreadCount={unreadCount} + /> + )} + renderItem={(item) => ( + + + + )} + /> ); }; diff --git a/apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx new file mode 100644 index 0000000000000..b8e828070f8e8 --- /dev/null +++ b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx @@ -0,0 +1,27 @@ +import { render, screen } from '@testing-library/react'; + +import RoomListWrapper from './RoomListWrapper'; + +jest.mock('react-i18next', () => ({ + useTranslation: () => ({ t: (key: string) => key }), +})); + +jest.mock('./useSidebarListNavigation', () => ({ + useSidebarListNavigation: () => ({ + sidebarListRef: { current: null }, + }), +})); + +describe('RoomListWrapper', () => { + it('forwards Virtua container props', () => { + render( + +
general
+
, + ); + + expect(screen.getByRole('list', { name: 'Channels' })).toBeInTheDocument(); + expect(screen.getByTestId('room-list-wrapper')).toHaveStyle({ height: '100%' }); + expect(screen.getByText('general')).toBeInTheDocument(); + }); +}); diff --git a/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx index 64dc40e0d8792..ff9ee230e1f8c 100644 --- a/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx +++ b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx @@ -2,17 +2,25 @@ import { useMergedRefs } from '@rocket.chat/fuselage-hooks'; import type { ForwardedRef, HTMLAttributes } from 'react'; import { forwardRef } from 'react'; import { useTranslation } from 'react-i18next'; +import type { CustomContainerComponentProps } from 'virtua'; import { useSidebarListNavigation } from './useSidebarListNavigation'; -type RoomListWrapperProps = HTMLAttributes; +type RoomListWrapperProps = CustomContainerComponentProps & Omit, 'children' | 'style'>; -const RoomListWrapper = forwardRef(function RoomListWrapper(props: RoomListWrapperProps, ref: ForwardedRef) { +const RoomListWrapper = forwardRef(function RoomListWrapper( + { children, style, ...props }: RoomListWrapperProps, + ref: ForwardedRef, +) { const { t } = useTranslation(); const { sidebarListRef } = useSidebarListNavigation(); const mergedRefs = useMergedRefs(ref, sidebarListRef); - return
; + return ( +
+ {children} +
+ ); }); export default RoomListWrapper; diff --git a/apps/meteor/client/sidebar/Sidebar.stories.tsx b/apps/meteor/client/sidebar/Sidebar.stories.tsx index 8d404efab60b2..8a51b758ce07e 100644 --- a/apps/meteor/client/sidebar/Sidebar.stories.tsx +++ b/apps/meteor/client/sidebar/Sidebar.stories.tsx @@ -26,6 +26,20 @@ const settings: Record = { public: true, _updatedAt: new Date(), }, + Discussion_enabled: { + _id: 'Discussion_enabled', + blocked: false, + createdAt: new Date(), + env: true, + i18nLabel: 'Discussions', + packageValue: true, + sorter: 1, + ts: new Date(), + type: 'boolean', + value: true, + public: true, + _updatedAt: new Date(), + }, }; const settingContextValue: ContextType = { @@ -44,36 +58,97 @@ const userPreferences: Record = { sidebarSortby: 'activity', }; -const subscriptions: SubscriptionWithRoom[] = [ - { - _id: '3Bysd8GrmkWBdS9RT', +const createSubscription = ({ + _id, + name, + t = 'c', + f = false, + unread = 0, + userMentions = 0, + groupMentions = 0, + prid, + teamMain = false, +}: { + _id: string; + name: string; + t?: SubscriptionWithRoom['t']; + f?: boolean; + unread?: number; + userMentions?: number; + groupMentions?: number; + prid?: string; + teamMain?: boolean; +}): SubscriptionWithRoom => + ({ + _id, open: true, - alert: true, - unread: 0, - userMentions: 0, - groupMentions: 0, + alert: unread > 0 || userMentions > 0 || groupMentions > 0, + unread, + userMentions, + groupMentions, ts: new Date(), - rid: 'GENERAL', - name: 'general', - t: 'c', + rid: _id.toUpperCase(), + name, + t, + f, u: { _id: '5yLFEABCSoqR5vozz', - username: 'yyy', - name: 'yyy', + username: 'john.doe', + name: 'John Doe', }, _updatedAt: new Date(), ls: new Date(), lr: new Date(), - tunread: [], - lowerCaseName: 'general', - lowerCaseFName: 'general', + tunread: unread > 0 ? [`${_id}-thread`] : [], + tunreadUser: userMentions > 0 ? [`${_id}-mention`] : [], + tunreadGroup: groupMentions > 0 ? [`${_id}-group`] : [], + lowerCaseName: name.toLowerCase(), + lowerCaseFName: name.toLowerCase(), estimatedWaitingTimeQueue: 0, livechatData: undefined, priorityWeight: 3, + prid, responseBy: undefined, + teamMain, usersCount: 0, waitingResponse: undefined, - }, + }) as SubscriptionWithRoom; + +const subscriptions: SubscriptionWithRoom[] = [ + ...Array.from({ length: 32 }, (_, index) => + createSubscription({ + _id: `channel-${index}`, + name: `channel-${index}`, + f: index % 11 === 0, + unread: index % 5 === 0 ? index + 1 : 0, + userMentions: index % 9 === 0 ? 1 : 0, + }), + ), + ...Array.from({ length: 8 }, (_, index) => + createSubscription({ + _id: `team-${index}`, + name: `team-${index}`, + teamMain: true, + unread: index % 4 === 0 ? 2 : 0, + }), + ), + ...Array.from({ length: 10 }, (_, index) => + createSubscription({ + _id: `direct-${index}`, + name: `direct-${index}`, + t: 'd', + f: index === 0, + unread: index % 3 === 0 ? 1 : 0, + }), + ), + ...Array.from({ length: 6 }, (_, index) => + createSubscription({ + _id: `discussion-${index}`, + name: `discussion-${index}`, + prid: 'GENERAL', + groupMentions: index === 0 ? 1 : 0, + }), + ), ]; const userContextValue: ContextType = { diff --git a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx new file mode 100644 index 0000000000000..8f051dd549527 --- /dev/null +++ b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx @@ -0,0 +1,158 @@ +import { render, screen } from '@testing-library/react'; +import { axe } from 'jest-axe'; +import type { CSSProperties, ElementType, HTMLAttributes, ReactNode } from 'react'; +import { Children, createElement, forwardRef } from 'react'; +import type { CustomContainerComponentProps } from 'virtua'; + +import SidebarVirtualList from './SidebarVirtualList'; +import type { SidebarVirtualListGroup } from './SidebarVirtualList'; + +type MockVirtualizerProps = { + children: ReactNode; + bufferSize?: number; + as?: ElementType; + style?: CSSProperties; + className?: string; +}; + +jest.mock('virtua', () => { + return { + Virtualizer: ({ children, bufferSize, as: root = 'div', style, className }: MockVirtualizerProps) => + createElement( + root, + { className, 'data-buffer-size': bufferSize, 'data-testid': 'virtual-list', 'style': style ?? { height: '100%' } }, + Children.toArray(children), + ), + }; +}); + +jest.mock('@rocket.chat/ui-client', () => ({ + ...jest.requireActual('@rocket.chat/ui-client'), + CustomVirtuaScrollbars: forwardRef>(function CustomVirtuaScrollbars( + { children, ...props }, + ref, + ) { + return ( +
+ {children} +
+ ); + }), +})); + +type TestGroup = { + title: string; +}; + +type TestItem = { + _id: string; + name: string; +}; + +const groups: SidebarVirtualListGroup[] = [ + { + key: 'channels', + group: { title: 'Channels' }, + items: [ + { _id: 'general', name: 'general' }, + { _id: 'support', name: 'support' }, + ], + }, + { + key: 'direct', + group: { title: 'Direct Messages' }, + items: [{ _id: 'alice', name: 'alice' }], + }, +]; + +const defaultProps = { + groups, + getItemKey: (item: TestItem) => item._id, + renderGroup: (group: TestGroup, groupIndex: number) =>
{`group:${groupIndex}:${group.title}`}
, + renderItem: (item: TestItem, itemIndex: number, group: TestGroup, groupIndex: number) => ( +
{`item:${groupIndex}:${itemIndex}:${group.title}:${item.name}`}
+ ), +}; + +const renderVirtualList = ( + props: Partial<{ + groups: SidebarVirtualListGroup[]; + overscan: number; + as: ElementType; + }> = {}, +) => render(); + +describe('SidebarVirtualList', () => { + it('renders group rows and item rows in order', () => { + renderVirtualList(); + + expect(screen.getAllByTestId('virtual-row').map((row) => row.textContent)).toEqual([ + 'group:0:Channels', + 'item:0:0:Channels:general', + 'item:0:1:Channels:support', + 'group:1:Direct Messages', + 'item:1:0:Direct Messages:alice', + ]); + }); + + it('renders a group with no items', () => { + renderVirtualList({ + groups: [ + { + key: 'empty', + group: { title: 'Empty Group' }, + items: [], + }, + ], + }); + + expect(screen.getAllByTestId('virtual-row').map((row) => row.textContent)).toEqual(['group:0:Empty Group']); + }); + + it('passes overscan to Virtua as buffer size', () => { + renderVirtualList({ overscan: 25 }); + + expect(screen.getByTestId('virtual-list')).toHaveAttribute('data-buffer-size', '25'); + }); + + it('uses the caller-provided list container', () => { + const ListContainer = forwardRef(function ListContainer({ children, style }, ref) { + return ( +
+ {children} +
+ ); + }); + + renderVirtualList({ as: ListContainer }); + + expect(screen.getByTestId('custom-list')).toHaveAttribute('role', 'list'); + expect(screen.getByTestId('custom-list')).toHaveTextContent('group:0:Channels'); + }); + + it('has no accessibility violations for an accessible caller-provided list', async () => { + const ListContainer = forwardRef(function ListContainer({ children, style }, ref) { + return ( +
+ {children} +
+ ); + }); + + const { container } = render( + item._id} + renderGroup={(group) => ( +
+ +
+ )} + renderItem={(item) =>
{item.name}
} + />, + ); + + expect(await axe(container)).toHaveNoViolations(); + }); +}); diff --git a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx new file mode 100644 index 0000000000000..4f285a5a630d7 --- /dev/null +++ b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx @@ -0,0 +1,67 @@ +import { CustomVirtuaScrollbars } from '@rocket.chat/ui-client'; +import type { Key, ReactNode } from 'react'; +import { useMemo } from 'react'; +import { Virtualizer } from 'virtua'; +import type { VirtualizerProps } from 'virtua'; + +const scrollViewportStyle = { + height: '100%', + width: '100%', + overflow: 'auto', +} as const; + +export type SidebarVirtualListGroup = { + key: string; + group: TGroup; + items: readonly TItem[]; +}; + +type SidebarVirtualListProps = { + groups: readonly SidebarVirtualListGroup[]; + renderGroup: (group: TGroup, groupIndex: number) => ReactNode; + renderItem: (item: TItem, itemIndex: number, group: TGroup, groupIndex: number) => ReactNode; + getItemKey: (item: TItem, itemIndex: number, group: TGroup, groupIndex: number) => Key; + overscan?: number; + as?: VirtualizerProps['as']; +}; + +type SidebarVirtualListRow = { + key: string; + content: ReactNode; +}; + +function SidebarVirtualList({ + groups, + renderGroup, + renderItem, + getItemKey, + overscan, + as, +}: SidebarVirtualListProps) { + const rows = useMemo(() => { + return groups.flatMap(({ key, group, items }, groupIndex) => [ + { + key: `group:${key}`, + content: renderGroup(group, groupIndex), + }, + ...items.map((item, itemIndex) => ({ + key: `item:${key}:${String(getItemKey(item, itemIndex, group, groupIndex))}`, + content: renderItem(item, itemIndex, group, groupIndex), + })), + ]); + }, [getItemKey, groups, renderGroup, renderItem]); + + return ( + +
+ + {rows.map(({ key, content }) => ( +
{content}
+ ))} +
+
+
+ ); +} + +export default SidebarVirtualList; diff --git a/apps/meteor/client/sidebar/components/SidebarVirtualList/index.ts b/apps/meteor/client/sidebar/components/SidebarVirtualList/index.ts new file mode 100644 index 0000000000000..53a02e4a3a238 --- /dev/null +++ b/apps/meteor/client/sidebar/components/SidebarVirtualList/index.ts @@ -0,0 +1,2 @@ +export { default } from './SidebarVirtualList'; +export type { SidebarVirtualListGroup } from './SidebarVirtualList'; From 6e323fae808858c585b647dcc87e9742e8a406f2 Mon Sep 17 00:00:00 2001 From: srijna Date: Mon, 22 Jun 2026 15:27:24 +0530 Subject: [PATCH 2/3] fix: defer sidebar virtual list row rendering --- .../SidebarVirtualList.spec.tsx | 50 ++++++++++++++-- .../SidebarVirtualList/SidebarVirtualList.tsx | 58 ++++++++++++++----- 2 files changed, 88 insertions(+), 20 deletions(-) diff --git a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx index 8f051dd549527..4c075ac00b7bd 100644 --- a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx +++ b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx @@ -7,8 +7,11 @@ import type { CustomContainerComponentProps } from 'virtua'; import SidebarVirtualList from './SidebarVirtualList'; import type { SidebarVirtualListGroup } from './SidebarVirtualList'; +let mockVisibleIndexes: number[] | undefined; + type MockVirtualizerProps = { - children: ReactNode; + children: ReactNode | ((item: unknown, index: number) => ReactNode); + data?: ArrayLike; bufferSize?: number; as?: ElementType; style?: CSSProperties; @@ -17,12 +20,20 @@ type MockVirtualizerProps = { jest.mock('virtua', () => { return { - Virtualizer: ({ children, bufferSize, as: root = 'div', style, className }: MockVirtualizerProps) => - createElement( + Virtualizer: ({ children, data, bufferSize, as: root = 'div', style, className }: MockVirtualizerProps) => { + const childrenToRender = + typeof children === 'function' + ? (mockVisibleIndexes ?? Array.from({ length: data?.length ?? 0 }, (_, index) => index)).map((index) => + children(data?.[index], index), + ) + : Children.toArray(children).filter((_, index) => !mockVisibleIndexes || mockVisibleIndexes.includes(index)); + + return createElement( root, { className, 'data-buffer-size': bufferSize, 'data-testid': 'virtual-list', 'style': style ?? { height: '100%' } }, - Children.toArray(children), - ), + childrenToRender, + ); + }, }; }); @@ -79,10 +90,16 @@ const renderVirtualList = ( groups: SidebarVirtualListGroup[]; overscan: number; as: ElementType; + renderGroup: typeof defaultProps.renderGroup; + renderItem: typeof defaultProps.renderItem; }> = {}, ) => render(); describe('SidebarVirtualList', () => { + beforeEach(() => { + mockVisibleIndexes = undefined; + }); + it('renders group rows and item rows in order', () => { renderVirtualList(); @@ -109,6 +126,29 @@ describe('SidebarVirtualList', () => { expect(screen.getAllByTestId('virtual-row').map((row) => row.textContent)).toEqual(['group:0:Empty Group']); }); + it('defers item rendering until Virtua requests the row', () => { + mockVisibleIndexes = [0]; + + const renderGroup = jest.fn((group: TestGroup) =>
{group.title}
); + const renderItem = jest.fn((item: TestItem) =>
{item.name}
); + + renderVirtualList({ + groups: [ + { + key: 'channels', + group: { title: 'Channels' }, + items: Array.from({ length: 1000 }, (_, index) => ({ _id: `room-${index}`, name: `room-${index}` })), + }, + ], + renderGroup, + renderItem, + }); + + expect(renderGroup).toHaveBeenCalledTimes(1); + expect(renderItem).not.toHaveBeenCalled(); + expect(screen.getByTestId('virtual-row')).toHaveTextContent('Channels'); + }); + it('passes overscan to Virtua as buffer size', () => { renderVirtualList({ overscan: 25 }); diff --git a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx index 4f285a5a630d7..2afccf4ca246f 100644 --- a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx +++ b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx @@ -1,6 +1,6 @@ import { CustomVirtuaScrollbars } from '@rocket.chat/ui-client'; import type { Key, ReactNode } from 'react'; -import { useMemo } from 'react'; +import { useCallback, useMemo } from 'react'; import { Virtualizer } from 'virtua'; import type { VirtualizerProps } from 'virtua'; @@ -25,10 +25,21 @@ type SidebarVirtualListProps = { as?: VirtualizerProps['as']; }; -type SidebarVirtualListRow = { - key: string; - content: ReactNode; -}; +type SidebarVirtualListRow = + | { + type: 'group'; + groupKey: string; + group: TGroup; + groupIndex: number; + } + | { + type: 'item'; + groupKey: string; + group: TGroup; + groupIndex: number; + item: TItem; + itemIndex: number; + }; function SidebarVirtualList({ groups, @@ -39,25 +50,42 @@ function SidebarVirtualList({ as, }: SidebarVirtualListProps) { const rows = useMemo(() => { - return groups.flatMap(({ key, group, items }, groupIndex) => [ + return groups.flatMap>(({ key, group, items }, groupIndex) => [ { - key: `group:${key}`, - content: renderGroup(group, groupIndex), + type: 'group', + groupKey: key, + group, + groupIndex, }, ...items.map((item, itemIndex) => ({ - key: `item:${key}:${String(getItemKey(item, itemIndex, group, groupIndex))}`, - content: renderItem(item, itemIndex, group, groupIndex), + type: 'item' as const, + groupKey: key, + group, + groupIndex, + item, + itemIndex, })), ]); - }, [getItemKey, groups, renderGroup, renderItem]); + }, [groups]); + + const renderRow = useCallback( + (row: SidebarVirtualListRow) => { + if (row.type === 'group') { + return
{renderGroup(row.group, row.groupIndex)}
; + } + + const itemKey = getItemKey(row.item, row.itemIndex, row.group, row.groupIndex); + + return
{renderItem(row.item, row.itemIndex, row.group, row.groupIndex)}
; + }, + [getItemKey, renderGroup, renderItem], + ); return (
- - {rows.map(({ key, content }) => ( -
{content}
- ))} + + {renderRow}
From cb536ad3be8ba9328fb2059715537bccfe614e69 Mon Sep 17 00:00:00 2001 From: srijna Date: Mon, 22 Jun 2026 21:36:45 +0530 Subject: [PATCH 3/3] fix: restore sidebar virtual list DOM contract --- .../client/sidebar/RoomList/RoomList.spec.tsx | 56 +++++++++++++------ .../client/sidebar/RoomList/RoomList.tsx | 4 +- .../sidebar/RoomList/RoomListWrapper.spec.tsx | 11 ++++ .../sidebar/RoomList/RoomListWrapper.tsx | 9 ++- .../SidebarVirtualList.spec.tsx | 19 +++++-- .../SidebarVirtualList/SidebarVirtualList.tsx | 16 ++++-- 6 files changed, 85 insertions(+), 30 deletions(-) diff --git a/apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx b/apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx index 2afaba1b68a8c..cb6727738308e 100644 --- a/apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx +++ b/apps/meteor/client/sidebar/RoomList/RoomList.spec.tsx @@ -32,25 +32,44 @@ type MockSidebarVirtualListProps = { items: typeof rooms; }[]; renderGroup: (group: { groupTitle: string; unreadCount: (typeof groupedUnreadInfo)[number] }, groupIndex: number) => ReactNode; - renderItem: (item: (typeof rooms)[number], itemIndex: number, group: { groupTitle: string }, groupIndex: number) => ReactNode; + renderItem: ( + item: (typeof rooms)[number], + itemIndex: number, + group: { groupTitle: string }, + groupIndex: number, + rowIndex: number, + ) => ReactNode; getItemKey: (item: (typeof rooms)[number]) => string; overscan?: number; }; -const mockSidebarVirtualList = jest.fn(({ groups, renderGroup, renderItem, getItemKey }: MockSidebarVirtualListProps) => ( -
- {groups.map(({ key, group, items }, groupIndex) => ( -
- {renderGroup(group, groupIndex)} - {items.map((item, itemIndex) => ( -
- {renderItem(item, itemIndex, group, groupIndex)} -
- ))} -
- ))} -
-)); +const mockSidebarVirtualList = jest.fn(({ groups, renderGroup, renderItem, getItemKey }: MockSidebarVirtualListProps) => { + let rowIndex = 0; + + return ( +
+ {groups.map(({ key, group, items }, groupIndex) => { + rowIndex += 1; + + return ( +
+ {renderGroup(group, groupIndex)} + {items.map((item, itemIndex) => { + const itemRowIndex = rowIndex; + rowIndex += 1; + + return ( +
+ {renderItem(item, itemIndex, group, groupIndex, itemRowIndex)} +
+ ); + })} +
+ ); + })} +
+ ); +}); jest.mock('@rocket.chat/fuselage', () => ({ Box: forwardRef>(function Box({ children, ...props }, ref) { @@ -127,7 +146,11 @@ jest.mock('./RoomListRow', () => ({ jest.mock('./RoomListRowWrapper', () => ({ __esModule: true, - default: ({ children }: { children: ReactNode }) =>
{children}
, + default: ({ children, ...props }: { children: ReactNode } & HTMLAttributes) => ( +
+ {children} +
+ ), })); describe('RoomList', () => { @@ -176,6 +199,7 @@ describe('RoomList', () => { 'Empty Group:0', ]); expect(screen.getAllByTestId('room-row-wrapper')).toHaveLength(3); + expect(screen.getAllByTestId('room-row-wrapper').map((element) => element.getAttribute('data-index'))).toEqual(['1', '2', '4']); expect(screen.getAllByTestId('room-row').map((element) => element.textContent)).toEqual(['general', 'support', 'alice']); }); }); diff --git a/apps/meteor/client/sidebar/RoomList/RoomList.tsx b/apps/meteor/client/sidebar/RoomList/RoomList.tsx index 28613cb5736e3..38e6e4943847b 100644 --- a/apps/meteor/client/sidebar/RoomList/RoomList.tsx +++ b/apps/meteor/client/sidebar/RoomList/RoomList.tsx @@ -80,8 +80,8 @@ const RoomList = () => { unreadCount={unreadCount} /> )} - renderItem={(item) => ( - + renderItem={(item, _itemIndex, _group, _groupIndex, rowIndex) => ( + )} diff --git a/apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx index b8e828070f8e8..0442f730b3bb0 100644 --- a/apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx +++ b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.spec.tsx @@ -13,6 +13,17 @@ jest.mock('./useSidebarListNavigation', () => ({ })); describe('RoomListWrapper', () => { + it('uses the legacy Virtuoso item list test id by default', () => { + render( + +
general
+
, + ); + + expect(screen.getByTestId('virtuoso-item-list')).toHaveRole('list'); + expect(screen.getByTestId('virtuoso-item-list')).toHaveAttribute('aria-label', 'Channels'); + }); + it('forwards Virtua container props', () => { render( diff --git a/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx index ff9ee230e1f8c..e7b6e47804f5b 100644 --- a/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx +++ b/apps/meteor/client/sidebar/RoomList/RoomListWrapper.tsx @@ -6,10 +6,13 @@ import type { CustomContainerComponentProps } from 'virtua'; import { useSidebarListNavigation } from './useSidebarListNavigation'; -type RoomListWrapperProps = CustomContainerComponentProps & Omit, 'children' | 'style'>; +type RoomListWrapperProps = CustomContainerComponentProps & + Omit, 'children' | 'style'> & { + 'data-testid'?: string; + }; const RoomListWrapper = forwardRef(function RoomListWrapper( - { children, style, ...props }: RoomListWrapperProps, + { children, style, 'data-testid': dataTestId, ...props }: RoomListWrapperProps, ref: ForwardedRef, ) { const { t } = useTranslation(); @@ -17,7 +20,7 @@ const RoomListWrapper = forwardRef(function RoomListWrapper( const mergedRefs = useMergedRefs(ref, sidebarListRef); return ( -
+
{children}
); diff --git a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx index 4c075ac00b7bd..c0e06fbe35109 100644 --- a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx +++ b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.spec.tsx @@ -80,8 +80,8 @@ const defaultProps = { groups, getItemKey: (item: TestItem) => item._id, renderGroup: (group: TestGroup, groupIndex: number) =>
{`group:${groupIndex}:${group.title}`}
, - renderItem: (item: TestItem, itemIndex: number, group: TestGroup, groupIndex: number) => ( -
{`item:${groupIndex}:${itemIndex}:${group.title}:${item.name}`}
+ renderItem: (item: TestItem, itemIndex: number, group: TestGroup, groupIndex: number, rowIndex: number) => ( +
{`item:${groupIndex}:${itemIndex}:${rowIndex}:${group.title}:${item.name}`}
), }; @@ -105,10 +105,10 @@ describe('SidebarVirtualList', () => { expect(screen.getAllByTestId('virtual-row').map((row) => row.textContent)).toEqual([ 'group:0:Channels', - 'item:0:0:Channels:general', - 'item:0:1:Channels:support', + 'item:0:0:1:Channels:general', + 'item:0:1:2:Channels:support', 'group:1:Direct Messages', - 'item:1:0:Direct Messages:alice', + 'item:1:0:4:Direct Messages:alice', ]); }); @@ -126,6 +126,15 @@ describe('SidebarVirtualList', () => { expect(screen.getAllByTestId('virtual-row').map((row) => row.textContent)).toEqual(['group:0:Empty Group']); }); + it('exposes the legacy Virtuoso group row hook used by E2E locators', () => { + renderVirtualList(); + + expect(screen.getAllByTestId('virtuoso-top-item-list').map((row) => row.textContent)).toEqual([ + 'group:0:Channels', + 'group:1:Direct Messages', + ]); + }); + it('defers item rendering until Virtua requests the row', () => { mockVisibleIndexes = [0]; diff --git a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx index 2afccf4ca246f..1326b531cfb2e 100644 --- a/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx +++ b/apps/meteor/client/sidebar/components/SidebarVirtualList/SidebarVirtualList.tsx @@ -19,7 +19,7 @@ export type SidebarVirtualListGroup = { type SidebarVirtualListProps = { groups: readonly SidebarVirtualListGroup[]; renderGroup: (group: TGroup, groupIndex: number) => ReactNode; - renderItem: (item: TItem, itemIndex: number, group: TGroup, groupIndex: number) => ReactNode; + renderItem: (item: TItem, itemIndex: number, group: TGroup, groupIndex: number, rowIndex: number) => ReactNode; getItemKey: (item: TItem, itemIndex: number, group: TGroup, groupIndex: number) => Key; overscan?: number; as?: VirtualizerProps['as']; @@ -69,14 +69,22 @@ function SidebarVirtualList({ }, [groups]); const renderRow = useCallback( - (row: SidebarVirtualListRow) => { + (row: SidebarVirtualListRow, rowIndex: number) => { if (row.type === 'group') { - return
{renderGroup(row.group, row.groupIndex)}
; + return ( +
+ {renderGroup(row.group, row.groupIndex)} +
+ ); } const itemKey = getItemKey(row.item, row.itemIndex, row.group, row.groupIndex); - return
{renderItem(row.item, row.itemIndex, row.group, row.groupIndex)}
; + return ( +
+ {renderItem(row.item, row.itemIndex, row.group, row.groupIndex, rowIndex)} +
+ ); }, [getItemKey, renderGroup, renderItem], );