mirror of
https://github.com/danny-avila/LibreChat.git
synced 2026-10-22 18:13:27 +00:00
🩹 fix: Scope phase-entrance history and resolve media queries at mount
Addresses both Codex findings on #14832. `MultiMessage` renders siblings without a key, so `ContentParts` survives a sibling switch with its refs intact. The recorded phase-marker set outlived the message it described, and any phase in the newly selected sibling whose index was absent from the previous sibling's set was read as a live arrival — already-loaded history mounted expanded and collapsed itself. Scope the set to its messageId and treat a mismatch as a fresh mount. `useMediaQuery` initialized to `false` and resolved only in a passive effect, so the first render always reported "no match". Anything branching once at mount — the frozen entrance flag here, and every other first-paint decision across its call sites — never saw the correction, which is how a `prefers-reduced-motion: reduce` user still got the fold. Read the query synchronously in the state initializer and guard both paths for environments without `matchMedia`.
This commit is contained in:
parent
3c084fa01d
commit
7dc8c06934
4 changed files with 131 additions and 6 deletions
|
|
@ -234,12 +234,18 @@ const ContentParts = memo(function ContentParts({
|
|||
/** A phase label can finish after the root text stream settles, so
|
||||
* `isSubmitting` is not a reliable entrance signal. Compare committed
|
||||
* phase markers instead: a marker that appears after this renderer has
|
||||
* mounted is live; markers present on the first render are history. */
|
||||
const previousPhaseIndicesRef = useRef<Set<number> | null>(null);
|
||||
const previousPhaseIndices = previousPhaseIndicesRef.current;
|
||||
* mounted is live; markers present on the first render are history.
|
||||
*
|
||||
* The recorded set is scoped to the message it described. `MultiMessage`
|
||||
* renders siblings without a key, so this instance survives a sibling
|
||||
* switch with its refs intact — an unscoped set would report the previous
|
||||
* sibling's phases and animate the newly selected sibling's history. */
|
||||
const previousPhaseRef = useRef<{ messageId: string; indices: Set<number> } | null>(null);
|
||||
const previousPhaseIndices =
|
||||
previousPhaseRef.current?.messageId === messageId ? previousPhaseRef.current.indices : null;
|
||||
useEffect(() => {
|
||||
previousPhaseIndicesRef.current = completedPhaseIndices;
|
||||
}, [completedPhaseIndices]);
|
||||
previousPhaseRef.current = { messageId, indices: completedPhaseIndices };
|
||||
}, [messageId, completedPhaseIndices]);
|
||||
|
||||
const handleGroupExpansionChange = useCallback(
|
||||
(groupId: string, state: ToolCallGroupExpansionState) => {
|
||||
|
|
|
|||
|
|
@ -467,4 +467,41 @@ describe('ContentParts — activity phase state', () => {
|
|||
);
|
||||
expect(screen.getByTestId('tool-call-group')).toHaveAttribute('data-initial-expanded', 'false');
|
||||
});
|
||||
|
||||
test('does not replay the entrance when switching to a sibling with its own phases', () => {
|
||||
const phase = (label: string) =>
|
||||
({
|
||||
type: ContentTypes.ACTIVITY_LABEL,
|
||||
[ContentTypes.ACTIVITY_LABEL]: label,
|
||||
activity_label_type: 'phase',
|
||||
activity_start_index: 0,
|
||||
activity_count: 1,
|
||||
pending: false,
|
||||
}) as unknown as TMessageContentParts;
|
||||
|
||||
const tool = {
|
||||
type: ContentTypes.TOOL_CALL,
|
||||
[ContentTypes.TOOL_CALL]: { id: 'tool-1', name: 'search', args: {}, output: 'one' },
|
||||
} as unknown as TMessageContentParts;
|
||||
|
||||
const { rerender } = render(
|
||||
<ContentParts {...baseProps} messageId="sibling-a" content={[tool, phase('First')]} />,
|
||||
);
|
||||
expect(screen.getByTestId('activity-phase-group')).toHaveAttribute(
|
||||
'data-animate-entrance',
|
||||
'false',
|
||||
);
|
||||
|
||||
/** MultiMessage swaps siblings without a key, so this instance keeps its
|
||||
* refs while messageId and content change wholesale. The incoming
|
||||
* sibling's phase is history and must not animate. */
|
||||
rerender(
|
||||
<ContentParts {...baseProps} messageId="sibling-b" content={[tool, tool, phase('Second')]} />,
|
||||
);
|
||||
|
||||
expect(screen.getByTestId('activity-phase-group')).toHaveAttribute(
|
||||
'data-animate-entrance',
|
||||
'false',
|
||||
);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
66
packages/client/src/hooks/__tests__/useMediaQuery.spec.tsx
Normal file
66
packages/client/src/hooks/__tests__/useMediaQuery.spec.tsx
Normal file
|
|
@ -0,0 +1,66 @@
|
|||
import { renderHook, act } from '@testing-library/react';
|
||||
import useMediaQuery from '../useMediaQuery';
|
||||
|
||||
type Listener = () => void;
|
||||
|
||||
/** The shared setup installs a writable (not configurable) `matchMedia`, so
|
||||
* these stubs assign over it rather than redefining the property. */
|
||||
function stubMatchMedia(matches: boolean) {
|
||||
const listeners = new Set<Listener>();
|
||||
const media = {
|
||||
matches,
|
||||
addEventListener: (_event: string, listener: Listener) => listeners.add(listener),
|
||||
removeEventListener: (_event: string, listener: Listener) => listeners.delete(listener),
|
||||
};
|
||||
window.matchMedia = jest.fn(() => media) as unknown as typeof window.matchMedia;
|
||||
return (next: boolean) => {
|
||||
media.matches = next;
|
||||
for (const listener of listeners) {
|
||||
listener();
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
describe('useMediaQuery', () => {
|
||||
const original = window.matchMedia;
|
||||
|
||||
afterEach(() => {
|
||||
window.matchMedia = original;
|
||||
});
|
||||
|
||||
/** Callers that branch once at mount — freezing an entrance animation,
|
||||
* choosing a layout before paint — only ever see the first render. */
|
||||
test('reports a match on the first render, before any effect runs', () => {
|
||||
stubMatchMedia(true);
|
||||
|
||||
const { result } = renderHook(() => useMediaQuery('(prefers-reduced-motion: reduce)'));
|
||||
|
||||
expect(result.current).toBe(true);
|
||||
});
|
||||
|
||||
test('reports no match on the first render when the query does not match', () => {
|
||||
stubMatchMedia(false);
|
||||
|
||||
const { result } = renderHook(() => useMediaQuery('(prefers-reduced-motion: reduce)'));
|
||||
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
|
||||
test('tracks later changes to the query', () => {
|
||||
const change = stubMatchMedia(false);
|
||||
const { result } = renderHook(() => useMediaQuery('(min-width: 768px)'));
|
||||
expect(result.current).toBe(false);
|
||||
|
||||
act(() => change(true));
|
||||
|
||||
expect(result.current).toBe(true);
|
||||
});
|
||||
|
||||
test('falls back to no match where matchMedia is unavailable', () => {
|
||||
window.matchMedia = undefined as unknown as typeof window.matchMedia;
|
||||
|
||||
const { result } = renderHook(() => useMediaQuery('(min-width: 768px)'));
|
||||
|
||||
expect(result.current).toBe(false);
|
||||
});
|
||||
});
|
||||
|
|
@ -1,9 +1,25 @@
|
|||
import { useEffect, useState } from 'react';
|
||||
|
||||
function readMatches(query: string): boolean {
|
||||
if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') {
|
||||
return false;
|
||||
}
|
||||
return window.matchMedia(query).matches;
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves the query on the FIRST render rather than after a passive effect.
|
||||
* Callers that branch once at mount — freezing an entrance animation, picking
|
||||
* a layout before paint — read the deferred value as "no match" and never see
|
||||
* the correction, which is how `prefers-reduced-motion` came to be ignored.
|
||||
*/
|
||||
export default function useMediaQuery(query: string): boolean {
|
||||
const [matches, setMatches] = useState(false);
|
||||
const [matches, setMatches] = useState(() => readMatches(query));
|
||||
|
||||
useEffect(() => {
|
||||
if (typeof window === 'undefined' || typeof window.matchMedia !== 'function') {
|
||||
return;
|
||||
}
|
||||
const media = window.matchMedia(query);
|
||||
if (media.matches !== matches) {
|
||||
setMatches(media.matches);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue