mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-06-15 23:43:06 +03:00
📐 fix: Sidebar Chat List Width Tracking and Stale Row Measurements (#13655)
* 📐 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
This commit is contained in:
@@ -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<ConversationsProps> = ({
|
||||
const isSmallScreen = useMediaQuery('(max-width: 768px)');
|
||||
const convoHeight = isSmallScreen ? 44 : 34;
|
||||
const showAgentMarketplace = useShowMarketplace();
|
||||
const {
|
||||
ref: listContainerRef,
|
||||
width: listWidth,
|
||||
height: listHeight,
|
||||
} = useElementSize<HTMLDivElement>();
|
||||
|
||||
const favoritesContentKeyRef = useRef('');
|
||||
|
||||
@@ -245,7 +251,8 @@ const Conversations: FC<ConversationsProps> = ({
|
||||
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<ConversationsProps> = ({
|
||||
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<ConversationsProps> = ({
|
||||
|
||||
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 (
|
||||
<MeasuredRow key={key} {...rowProps}>
|
||||
<DateLabel groupName={item.groupName} isFirst={index === firstHeaderIndex} />
|
||||
@@ -332,7 +350,7 @@ const Conversations: FC<ConversationsProps> = ({
|
||||
|
||||
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<ConversationsProps> = ({
|
||||
<span className="ml-2 text-text-primary">{localize('com_ui_loading')}</span>
|
||||
</div>
|
||||
) : (
|
||||
<div className="flex-1">
|
||||
<AutoSizer>
|
||||
{({ width, height }) => (
|
||||
<List
|
||||
ref={containerRef}
|
||||
width={width}
|
||||
height={height}
|
||||
deferredMeasurementCache={cache}
|
||||
rowCount={flattenedItems.length}
|
||||
rowHeight={getRowHeight}
|
||||
rowRenderer={rowRenderer}
|
||||
overscanRowCount={10}
|
||||
aria-readonly={false}
|
||||
className="outline-none"
|
||||
aria-label="Conversations"
|
||||
onRowsRendered={handleRowsRendered}
|
||||
tabIndex={-1}
|
||||
style={{ outline: 'none' }}
|
||||
containerRole="rowgroup"
|
||||
/>
|
||||
)}
|
||||
</AutoSizer>
|
||||
<div ref={listContainerRef} className="min-h-0 flex-1 overflow-hidden">
|
||||
<List
|
||||
ref={containerRef}
|
||||
width={listWidth}
|
||||
height={listHeight}
|
||||
deferredMeasurementCache={cache}
|
||||
rowCount={flattenedItems.length}
|
||||
rowHeight={getRowHeight}
|
||||
rowRenderer={rowRenderer}
|
||||
overscanRowCount={10}
|
||||
aria-readonly={false}
|
||||
className="outline-none"
|
||||
aria-label="Conversations"
|
||||
onRowsRendered={handleRowsRendered}
|
||||
tabIndex={-1}
|
||||
style={{ outline: 'none' }}
|
||||
containerRole="rowgroup"
|
||||
/>
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
|
||||
@@ -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: {},
|
||||
}));
|
||||
|
||||
|
||||
115
client/src/hooks/Generic/__tests__/useElementSize.spec.tsx
Normal file
115
client/src/hooks/Generic/__tests__/useElementSize.spec.tsx
Normal file
@@ -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<HTMLDivElement>());
|
||||
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<HTMLDivElement>());
|
||||
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<HTMLDivElement>());
|
||||
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<HTMLDivElement>());
|
||||
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<HTMLDivElement>());
|
||||
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);
|
||||
});
|
||||
});
|
||||
@@ -1,2 +1,3 @@
|
||||
export * from './useLazyEffect';
|
||||
export { default as useShiftKey } from './useShiftKey';
|
||||
export { default as useElementSize } from './useElementSize';
|
||||
|
||||
51
client/src/hooks/Generic/useElementSize.ts
Normal file
51
client/src/hooks/Generic/useElementSize.ts
Normal file
@@ -0,0 +1,51 @@
|
||||
import { useCallback, useLayoutEffect, useState } from 'react';
|
||||
|
||||
interface ElementSize {
|
||||
width: number;
|
||||
height: number;
|
||||
}
|
||||
|
||||
interface UseElementSizeResult<T extends HTMLElement> {
|
||||
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<T extends HTMLElement>(): UseElementSizeResult<T> {
|
||||
const [node, setNode] = useState<T | null>(null);
|
||||
const [size, setSize] = useState<ElementSize>({ 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 };
|
||||
}
|
||||
82
e2e/specs/mock/sidebar.spec.ts
Normal file
82
e2e/specs/mock/sidebar.spec.ts
Normal file
@@ -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<HTMLElement>('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);
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user