From 4a9af12082d43b130415a32aa0cd3074d8705b1b Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Wed, 10 Jun 2026 13:27:18 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=93=90=20fix:=20Sidebar=20Chat=20List=20W?= =?UTF-8?q?idth=20Tracking=20and=20Stale=20Row=20Measurements=20(#13655)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ๐Ÿ“ fix: Sidebar Chat List Width Tracking and Stale Row Measurements * โœ… test: Sidebar Chat List Width Tracking e2e Coverage * ๐Ÿฉน fix: Address Review โ€” Shrinkable List Wrapper, Seeded Measure, Fallback Resize * โœ… test: Scope Sidebar Grid Selector and Cover Height Shrink * ๐Ÿงช test: Settle Sidebar Sizes Before Asserting to Deflake CI --- .../Conversations/Conversations.tsx | 66 ++++++---- .../__tests__/Conversations.test.tsx | 6 +- .../Generic/__tests__/useElementSize.spec.tsx | 115 ++++++++++++++++++ client/src/hooks/Generic/index.ts | 1 + client/src/hooks/Generic/useElementSize.ts | 51 ++++++++ e2e/specs/mock/sidebar.spec.ts | 82 +++++++++++++ 6 files changed, 290 insertions(+), 31 deletions(-) create mode 100644 client/src/hooks/Generic/__tests__/useElementSize.spec.tsx create mode 100644 client/src/hooks/Generic/useElementSize.ts create mode 100644 e2e/specs/mock/sidebar.spec.ts diff --git a/client/src/components/Conversations/Conversations.tsx b/client/src/components/Conversations/Conversations.tsx index 26c670341a..68bf043c63 100644 --- a/client/src/components/Conversations/Conversations.tsx +++ b/client/src/components/Conversations/Conversations.tsx @@ -4,7 +4,7 @@ import { useRecoilValue } from 'recoil'; import { ChevronDown } from 'lucide-react'; import { QueryKeys } from 'librechat-data-provider'; import { useQueryClient } from '@tanstack/react-query'; -import { List, AutoSizer, CellMeasurer, CellMeasurerCache } from 'react-virtualized'; +import { List, CellMeasurer, CellMeasurerCache } from 'react-virtualized'; import { Spinner, TooltipAnchor, NewChatIcon, useMediaQuery } from '@librechat/client'; import type { TConversation } from 'librechat-data-provider'; import { @@ -13,6 +13,7 @@ import { useFavorites, useShowMarketplace, useNewConvo, + useElementSize, } from '~/hooks'; import { groupConversationsByDate, clearMessagesCache, cn } from '~/utils'; import FavoritesList from '~/components/Nav/Favorites/FavoritesList'; @@ -178,6 +179,11 @@ const Conversations: FC = ({ const isSmallScreen = useMediaQuery('(max-width: 768px)'); const convoHeight = isSmallScreen ? 44 : 34; const showAgentMarketplace = useShowMarketplace(); + const { + ref: listContainerRef, + width: listWidth, + height: listHeight, + } = useElementSize(); const favoritesContentKeyRef = useRef(''); @@ -245,7 +251,8 @@ const Conversations: FC = ({ return `favorites-${favoritesContentKeyRef.current}`; } if (item.type === 'header') { - return `header-${item.groupName}`; + const firstHeaderIndex = flattenedItemsRef.current[0]?.type === 'favorites' ? 1 : 0; + return `header-${item.groupName}-${index === firstHeaderIndex ? 'first' : 'sub'}`; } if (item.type === 'convo') { return `convo-${item.convo.conversationId}`; @@ -285,6 +292,17 @@ const Conversations: FC = ({ return () => cancelAnimationFrame(frameId); }, [search.query, cache, containerRef]); + /** Grid only re-derives row offsets when the row count changes; reorders that + * keep the count (e.g. a convo bumped across date groups) need an explicit recompute. */ + useEffect(() => { + const frameId = requestAnimationFrame(() => { + if (containerRef.current && 'recomputeRowHeights' in containerRef.current) { + containerRef.current.recomputeRowHeights(0); + } + }); + return () => cancelAnimationFrame(frameId); + }, [flattenedItems, containerRef]); + const rowRenderer = useCallback( ({ index, key, parent, style }) => { const item = flattenedItems[index]; @@ -308,7 +326,7 @@ const Conversations: FC = ({ if (item.type === 'header') { // First date header index depends on whether the favorites row is included - const firstHeaderIndex = shouldShowFavorites ? 1 : 0; + const firstHeaderIndex = flattenedItems[0]?.type === 'favorites' ? 1 : 0; return ( @@ -332,7 +350,7 @@ const Conversations: FC = ({ return null; }, - [cache, flattenedItems, moveToTop, toggleNav, isSmallScreen, shouldShowFavorites, activeJobIds], + [cache, flattenedItems, moveToTop, toggleNav, isSmallScreen, activeJobIds], ); const getRowHeight = useCallback( @@ -368,28 +386,24 @@ const Conversations: FC = ({ {localize('com_ui_loading')} ) : ( -
- - {({ width, height }) => ( - - )} - +
+
)}
diff --git a/client/src/components/Conversations/__tests__/Conversations.test.tsx b/client/src/components/Conversations/__tests__/Conversations.test.tsx index 42d0a378ef..4deb911d73 100644 --- a/client/src/components/Conversations/__tests__/Conversations.test.tsx +++ b/client/src/components/Conversations/__tests__/Conversations.test.tsx @@ -10,11 +10,6 @@ jest.mock('react-virtualized', () => { const actual = jest.requireActual('react-virtualized'); return { ...actual, - AutoSizer: ({ - children, - }: { - children: (size: { width: number; height: number }) => React.ReactNode; - }) => children({ width: 300, height: 600 }), CellMeasurer: ({ children, }: { @@ -75,6 +70,7 @@ jest.mock('~/hooks', () => ({ useLocalize: () => (key: string) => key, useShowMarketplace: () => mockShowMarketplace, useNewConvo: () => ({ newConversation: jest.fn() }), + useElementSize: () => ({ ref: jest.fn(), width: 300, height: 600 }), TranslationKeys: {}, })); diff --git a/client/src/hooks/Generic/__tests__/useElementSize.spec.tsx b/client/src/hooks/Generic/__tests__/useElementSize.spec.tsx new file mode 100644 index 0000000000..57a08edc02 --- /dev/null +++ b/client/src/hooks/Generic/__tests__/useElementSize.spec.tsx @@ -0,0 +1,115 @@ +import { renderHook, act } from '@testing-library/react'; +import useElementSize from '../useElementSize'; + +class MockResizeObserver { + static instances: MockResizeObserver[] = []; + callback: ResizeObserverCallback; + observed: Element[] = []; + disconnected = false; + + constructor(callback: ResizeObserverCallback) { + this.callback = callback; + MockResizeObserver.instances.push(this); + } + + observe(element: Element) { + this.observed.push(element); + } + + unobserve() {} + + disconnect() { + this.disconnected = true; + } + + trigger(contentRect: { width: number; height: number }) { + this.callback( + [{ contentRect, target: this.observed[0] } as ResizeObserverEntry], + this as unknown as ResizeObserver, + ); + } +} + +describe('useElementSize', () => { + const originalResizeObserver = window.ResizeObserver; + + beforeEach(() => { + MockResizeObserver.instances = []; + window.ResizeObserver = MockResizeObserver as unknown as typeof ResizeObserver; + }); + + afterAll(() => { + window.ResizeObserver = originalResizeObserver; + }); + + it('reports zero size before an element is attached', () => { + const { result } = renderHook(() => useElementSize()); + expect(result.current.width).toBe(0); + expect(result.current.height).toBe(0); + }); + + it('observes the attached element and reports floored content-box size', () => { + const { result } = renderHook(() => useElementSize()); + const node = document.createElement('div'); + + act(() => result.current.ref(node)); + + const observer = MockResizeObserver.instances[MockResizeObserver.instances.length - 1]; + expect(observer.observed).toContain(node); + + act(() => observer.trigger({ width: 320.6, height: 480.2 })); + expect(result.current.width).toBe(320); + expect(result.current.height).toBe(480); + + act(() => observer.trigger({ width: 360, height: 480 })); + expect(result.current.width).toBe(360); + }); + + it('re-observes when the element remounts', () => { + const { result } = renderHook(() => useElementSize()); + const first = document.createElement('div'); + const second = document.createElement('div'); + + act(() => result.current.ref(first)); + const firstObserver = MockResizeObserver.instances[MockResizeObserver.instances.length - 1]; + + act(() => result.current.ref(null)); + expect(firstObserver.disconnected).toBe(true); + + act(() => result.current.ref(second)); + const secondObserver = MockResizeObserver.instances[MockResizeObserver.instances.length - 1]; + expect(secondObserver).not.toBe(firstObserver); + expect(secondObserver.observed).toContain(second); + }); + + it('disconnects the observer on unmount', () => { + const { result, unmount } = renderHook(() => useElementSize()); + const node = document.createElement('div'); + + act(() => result.current.ref(node)); + const observer = MockResizeObserver.instances[MockResizeObserver.instances.length - 1]; + + unmount(); + expect(observer.disconnected).toBe(true); + }); + + it('falls back to offset measurements when ResizeObserver is unavailable', () => { + window.ResizeObserver = undefined as unknown as typeof ResizeObserver; + + const { result } = renderHook(() => useElementSize()); + const node = document.createElement('div'); + let offsetWidth = 280; + Object.defineProperty(node, 'offsetWidth', { get: () => offsetWidth, configurable: true }); + Object.defineProperty(node, 'offsetHeight', { value: 500 }); + + act(() => result.current.ref(node)); + expect(result.current.width).toBe(280); + expect(result.current.height).toBe(500); + + offsetWidth = 320; + act(() => { + window.dispatchEvent(new Event('resize')); + }); + expect(result.current.width).toBe(320); + }); +}); diff --git a/client/src/hooks/Generic/index.ts b/client/src/hooks/Generic/index.ts index 9674014afa..2dc02f273b 100644 --- a/client/src/hooks/Generic/index.ts +++ b/client/src/hooks/Generic/index.ts @@ -1,2 +1,3 @@ export * from './useLazyEffect'; export { default as useShiftKey } from './useShiftKey'; +export { default as useElementSize } from './useElementSize'; diff --git a/client/src/hooks/Generic/useElementSize.ts b/client/src/hooks/Generic/useElementSize.ts new file mode 100644 index 0000000000..57587696b3 --- /dev/null +++ b/client/src/hooks/Generic/useElementSize.ts @@ -0,0 +1,51 @@ +import { useCallback, useLayoutEffect, useState } from 'react'; + +interface ElementSize { + width: number; + height: number; +} + +interface UseElementSizeResult { + ref: (node: T | null) => void; + width: number; + height: number; +} + +/** + * Tracks an element's content-box size via ResizeObserver. Returns a callback + * ref so conditionally rendered elements are re-observed when they remount. + */ +export default function useElementSize(): UseElementSizeResult { + const [node, setNode] = useState(null); + const [size, setSize] = useState({ width: 0, height: 0 }); + + const ref = useCallback((element: T | null) => setNode(element), []); + + useLayoutEffect(() => { + if (!node) { + return; + } + const apply = (width: number, height: number) => + setSize((prev) => + prev.width === width && prev.height === height ? prev : { width, height }, + ); + const measure = () => apply(node.offsetWidth, node.offsetHeight); + + measure(); + if (typeof ResizeObserver === 'undefined') { + window.addEventListener('resize', measure); + return () => window.removeEventListener('resize', measure); + } + const observer = new ResizeObserver((entries) => { + const entry = entries[entries.length - 1]; + if (!entry) { + return; + } + apply(Math.floor(entry.contentRect.width), Math.floor(entry.contentRect.height)); + }); + observer.observe(node); + return () => observer.disconnect(); + }, [node]); + + return { ref, width: size.width, height: size.height }; +} diff --git a/e2e/specs/mock/sidebar.spec.ts b/e2e/specs/mock/sidebar.spec.ts new file mode 100644 index 0000000000..e622145d4b --- /dev/null +++ b/e2e/specs/mock/sidebar.spec.ts @@ -0,0 +1,82 @@ +import { expect, test } from '@playwright/test'; +import type { Page } from '@playwright/test'; + +/** Size of the virtualized chat list grid vs. its measured container. */ +const sizes = (page: Page) => + page.evaluate(() => { + const grid = document.querySelector('aside .ReactVirtualized__Grid'); + const wrap = grid?.parentElement ?? null; + const gridRect = grid?.getBoundingClientRect(); + const wrapRect = wrap?.getBoundingClientRect(); + return { + grid: gridRect ? gridRect.width : -1, + wrap: wrapRect ? wrapRect.width : -1, + gridH: gridRect ? gridRect.height : -1, + wrapH: wrapRect ? wrapRect.height : -1, + }; + }); + +/** + * Polls until the grid matches its container AND the size has stopped changing + * between samples โ€” the sidebar expand/collapse animation runs for 300ms, and a + * tracking-only check can match mid-animation on slow CI machines. + */ +const settledSizes = async (page: Page) => { + let prev = await sizes(page); + for (let attempt = 0; attempt < 40; attempt++) { + await page.waitForTimeout(350); + const next = await sizes(page); + const tracked = + next.wrap > 0 && + next.wrapH > 0 && + Math.abs(next.grid - next.wrap) <= 1 && + Math.abs(next.gridH - next.wrapH) <= 1; + const stable = Math.abs(next.grid - prev.grid) <= 1 && Math.abs(next.gridH - prev.gridH) <= 1; + if (tracked && stable) { + return next; + } + prev = next; + } + throw new Error(`Sidebar chat list never settled: ${JSON.stringify(prev)}`); +}; + +test.describe('sidebar chat list', () => { + test('chat list width tracks the sidebar through resize and collapse cycles', async ({ + page, + }) => { + test.setTimeout(60000); + await page.goto('/c/new', { timeout: 10000 }); + await expect(page.locator('aside .ReactVirtualized__Grid').first()).toBeVisible({ + timeout: 20000, + }); + + const initial = await settledSizes(page); + + const separator = page.locator('[role="separator"][aria-label="Resize sidebar"]'); + const sepBox = await separator.boundingBox(); + expect(sepBox).not.toBeNull(); + const startX = (sepBox?.x ?? 0) + (sepBox?.width ?? 0) / 2; + const y = (sepBox?.y ?? 0) + (sepBox?.height ?? 0) / 2; + + await page.mouse.move(startX, y); + await page.mouse.down(); + for (let i = 1; i <= 5; i++) { + await page.mouse.move(startX + i * 20, y); + await page.waitForTimeout(50); + } + await page.mouse.up(); + + const widened = await settledSizes(page); + expect(widened.grid).toBeGreaterThan(initial.grid); + + await page.locator('aside').getByTestId('close-sidebar-button').click(); + await page.locator('aside').getByTestId('open-sidebar-button').click(); + + const reopened = await settledSizes(page); + expect(reopened.grid).toBeGreaterThan(initial.grid); + + await page.setViewportSize({ width: 1280, height: 540 }); + const shrunken = await settledSizes(page); + expect(shrunken.gridH).toBeLessThan(reopened.gridH); + }); +});