🪄 fix: Fold activity phase entrance in one direction, flush-left label

The phase header replaced <summary> with <button>, which brought the UA
`text-align: center` with it — the label span is `flex-1`, so the text
filled the row and centered inside it. Left-align it and drop the leading
glyph: the card's border and fill already carry the weight, and the child
tool groups keep their own icons.

The entrance also read as two movements. The card, header and inset all
hard-cut in at full size, displacing the transcript below by ~57px, then
folded back up past the header that had just pushed it down. The card now
mounts in the shape of what was already on screen — zero-height header,
transparent chrome, no inset — and grows the header as the panel collapses,
so the block's height only ever decreases. Chrome, padding and both heights
share one curve.

The collapse also waits for a painted start value; a single rAF can land
before paint, and a start value the compositor never saw snaps rather than
transitions.

- Restore the e2e parent-phase selectors, which still matched `summary`
- Memoize the hoisted `groupActivityPhases` pass and its phase-index set
- Finish the amber -> `text-text-warning` sweep in ToolCallGroup and Part
This commit is contained in:
Danny Avila 2026-08-14 10:40:25 -04:00
parent b21b67e676
commit 3c084fa01d
6 changed files with 236 additions and 98 deletions

View file

@ -1,22 +1,53 @@
import { useId, useCallback, useEffect, useRef, useState } from 'react';
import { useId, useCallback, useEffect, useMemo, useRef, useState } from 'react';
import { Button } from '@librechat/client';
import { ChevronDown, ListTree } from 'lucide-react';
import { ChevronDown } from 'lucide-react';
import { ContentTypes } from 'librechat-data-provider';
import type { TMessageContentParts } from 'librechat-data-provider';
import type { ReactNode } from 'react';
import { useExpandCollapse, scheduleMessageContentLayoutReconcile } from '~/hooks';
import type { CSSProperties, ReactNode } from 'react';
import {
useExpandCollapse,
scheduleMessageContentLayoutReconcile,
EXPAND_TRANSITION,
} from '~/hooks';
import useSmoothStreaming from '~/hooks/Messages/useSmoothStreaming';
import { getActivityLabelText } from '~/utils/activityLabels';
import { EmptyText } from './Parts';
import Container from './Container';
import { cn } from '~/utils';
/** Matches `EXPAND_TRANSITION` so the header, the panel, and the card chrome
* all resolve on the same curve — three properties animating on two different
* easings is what makes a fold read as two separate movements. */
const FOLD_EASING = 'duration-300 ease-[cubic-bezier(0.16,1,0.3,1)]';
type ActivityPhasePart = Extract<TMessageContentParts, { type: ContentTypes.ACTIVITY_LABEL }> & {
activity_label_type?: 'phase';
activity_start_index?: number;
activity_end_index?: number;
};
/**
* Runs `callback` once the browser has painted the current styles. One frame
* is not enough: React can flush passive effects before paint, and a start
* value the compositor never saw produces an instant jump rather than a
* transition. Returns a canceller for whichever frame is still pending.
*/
function schedulePostPaint(callback: () => void): () => void {
let frameId: number | undefined;
frameId = window.requestAnimationFrame(() => {
frameId = window.requestAnimationFrame(() => {
frameId = undefined;
callback();
});
});
return () => {
if (frameId != null) {
window.cancelAnimationFrame(frameId);
frameId = undefined;
}
};
}
export default function ActivityPhaseGroup({
labelPart,
children,
@ -35,38 +66,43 @@ export default function ActivityPhaseGroup({
const smoothStreaming = useSmoothStreaming();
/** Capture the marker's arrival state. The parent renderer records the new
* marker after this commit; a later sibling update must not cancel the
* already-scheduled compression before its first animation frame. */
* already-scheduled fold before its first animation frame. */
const [shouldAnimateEntrance] = useState(smoothStreaming && animateEntrance && label.length > 0);
/** A newly filled phase replaces activity that was already visible in the
* transcript. Keep that content in place for the first painted frame,
* then compress it into the summary instead of replacing it with a closed
* disclosure in one jarring cut. Historical phases remain closed. */
const [isExpanded, setIsExpanded] = useState(shouldAnimateEntrance && hasContent);
/** A filled phase marker lands on top of activity the reader is already
* looking at. The card therefore mounts in the shape of what was there
* BEFORE it — header at zero height, panel open, chrome transparent — and
* folds into the summary on the next painted frame. Growing the header
* while the panel collapses keeps the block's height strictly decreasing,
* so the content compresses upward instead of being shoved down by a
* header that appeared underneath it and then yanked back up. */
const foldsIn = shouldAnimateEntrance && hasContent;
const [isExpanded, setIsExpanded] = useState(foldsIn);
const [isSettled, setIsSettled] = useState(!foldsIn);
const rootRef = useRef<HTMLDivElement | null>(null);
const panelId = useId();
const collapseFrameRef = useRef<number>();
const cancelEntranceRef = useRef<(() => void) | null>(null);
const cancelLayoutReconcileRef = useRef<(() => void) | null>(null);
const previousIsExpandedRef = useRef(isExpanded);
const userOverrideRef = useRef(false);
const { style: expandStyle, ref: expandRef } = useExpandCollapse(isExpanded);
useEffect(() => {
if (!shouldAnimateEntrance || !hasContent || userOverrideRef.current) {
if (!foldsIn || userOverrideRef.current) {
return;
}
collapseFrameRef.current = window.requestAnimationFrame(() => {
collapseFrameRef.current = undefined;
if (!userOverrideRef.current) {
setIsExpanded(false);
cancelEntranceRef.current = schedulePostPaint(() => {
cancelEntranceRef.current = null;
if (userOverrideRef.current) {
return;
}
setIsSettled(true);
setIsExpanded(false);
});
return () => {
if (collapseFrameRef.current != null) {
window.cancelAnimationFrame(collapseFrameRef.current);
collapseFrameRef.current = undefined;
}
cancelEntranceRef.current?.();
cancelEntranceRef.current = null;
};
}, [shouldAnimateEntrance, hasContent]);
}, [foldsIn]);
useEffect(() => {
const wasExpanded = previousIsExpandedRef.current;
@ -86,12 +122,26 @@ export default function ActivityPhaseGroup({
const handleToggle = useCallback(() => {
userOverrideRef.current = true;
if (collapseFrameRef.current != null) {
window.cancelAnimationFrame(collapseFrameRef.current);
collapseFrameRef.current = undefined;
}
cancelEntranceRef.current?.();
cancelEntranceRef.current = null;
setIsSettled(true);
setIsExpanded((expanded) => !expanded);
}, []);
/** Only the folding entrance drives the header off its natural height.
* History and reduced-motion render the plain, unstyled row. */
const headerStyle = useMemo<CSSProperties | undefined>(() => {
if (!foldsIn) {
return undefined;
}
return {
display: 'grid',
gridTemplateRows: isSettled ? '1fr' : '0fr',
transition: EXPAND_TRANSITION,
opacity: isSettled ? 1 : 0,
};
}, [foldsIn, isSettled]);
const cursor = showCursor ? (
<Container>
<EmptyText />
@ -103,18 +153,13 @@ export default function ActivityPhaseGroup({
const group = !hasContent ? (
<div
className={cn(
'my-2 flex min-h-10 w-full items-center gap-2 rounded-lg border border-border-light bg-surface-secondary/40 px-3 py-2 text-text-secondary',
shouldAnimateEntrance &&
'duration-300 animate-in fade-in-0 slide-in-from-bottom-1 motion-reduce:animate-none',
'my-2 flex min-h-10 w-full items-center rounded-lg border border-border-light bg-surface-secondary/40 px-3 py-2 text-text-secondary',
shouldAnimateEntrance && `animate-in fade-in-0 motion-reduce:animate-none ${FOLD_EASING}`,
)}
>
<ListTree
className={cn('size-4 shrink-0', hasFailure && 'text-text-warning')}
aria-hidden="true"
/>
<span
className={cn(
'min-w-0 flex-1 truncate text-sm font-medium',
'min-w-0 flex-1 truncate text-left text-sm font-medium',
hasFailure && 'text-text-warning',
)}
role="status"
@ -125,44 +170,44 @@ export default function ActivityPhaseGroup({
</div>
) : (
<div
className="my-2 w-full rounded-lg border border-border-light bg-surface-secondary/40"
className={cn(
'my-2 w-full rounded-lg border transition-colors',
FOLD_EASING,
isSettled ? 'border-border-light bg-surface-secondary/40' : 'border-transparent',
)}
ref={rootRef}
>
<Button
variant="ghost"
type="button"
className={cn(
'flex h-auto min-h-10 w-full items-center justify-start gap-2 rounded-lg bg-transparent px-3 py-2 text-text-secondary hover:bg-surface-hover hover:text-text-primary focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring-primary focus-visible:ring-offset-0',
shouldAnimateEntrance &&
'duration-300 animate-in fade-in-0 slide-in-from-bottom-1 motion-reduce:animate-none',
)}
onClick={handleToggle}
aria-expanded={isExpanded}
aria-controls={panelId}
aria-label={label}
title={label}
>
<ListTree
className={cn('size-4 shrink-0', hasFailure && 'text-text-warning')}
aria-hidden="true"
/>
<span
className={cn(
'min-w-0 flex-1 truncate text-sm font-medium',
hasFailure && 'text-text-warning',
)}
role="status"
>
{label}
</span>
<ChevronDown
className={cn(
'size-4 shrink-0 transition-transform duration-200 ease-out',
isExpanded && 'rotate-180',
)}
aria-hidden="true"
/>
</Button>
<div style={headerStyle}>
<div className="overflow-hidden">
<Button
variant="ghost"
type="button"
className="flex h-auto min-h-10 w-full items-center justify-start gap-2 rounded-lg bg-transparent px-3 py-2 text-left text-text-secondary hover:bg-surface-hover hover:text-text-primary focus-visible:ring-2 focus-visible:ring-inset focus-visible:ring-ring-primary focus-visible:ring-offset-0"
onClick={handleToggle}
aria-expanded={isExpanded}
aria-controls={panelId}
aria-label={label}
title={label}
>
<span
className={cn(
'min-w-0 flex-1 truncate text-left text-sm font-medium',
hasFailure && 'text-text-warning',
)}
role="status"
>
{label}
</span>
<ChevronDown
className={cn(
'size-4 shrink-0 transition-transform duration-200 ease-out',
isExpanded && 'rotate-180',
)}
aria-hidden="true"
/>
</Button>
</div>
</div>
<div
id={panelId}
style={expandStyle}
@ -170,7 +215,19 @@ export default function ActivityPhaseGroup({
data-testid="activity-phase-panel"
>
<div className="overflow-hidden" ref={expandRef}>
<div className="border-t border-border-light px-3 py-2">{children}</div>
{/** Padding and the divider ride the same curve as the fold: the
* children occupy the exact position they held before the marker
* arrived and settle into the card as it materializes, instead of
* stepping sideways by the card's inset on the first frame. */}
<div
className={cn(
'border-t transition-[border-color,padding]',
FOLD_EASING,
isSettled ? 'border-border-light px-3 py-2' : 'border-transparent px-0 py-0',
)}
>
{children}
</div>
</div>
</div>
</div>

View file

@ -215,11 +215,22 @@ const ContentParts = memo(function ContentParts({
(localIndex: number) => contentIndices?.[localIndex] ?? localIndex + contentIndexOffset,
[contentIndexOffset, contentIndices],
);
const phaseSegments = nestedActivityPhase ? undefined : groupActivityPhases(content);
const completedPhaseIndices = new Set(
phaseSegments?.flatMap((segment) => (segment.type === 'phase' ? [segment.labelIndex] : [])) ??
[],
/** Hoisted above the early returns to feed the entrance-detection hook
* below, so it is memoized rather than re-walked on every unrelated
* re-render of a message that has no phases at all. */
const phaseSegments = useMemo(
() => (nestedActivityPhase ? undefined : groupActivityPhases(content)),
[nestedActivityPhase, content],
);
const completedPhaseIndices = useMemo(() => {
const indices = new Set<number>();
for (const segment of phaseSegments ?? []) {
if (segment.type === 'phase') {
indices.add(segment.labelIndex);
}
}
return indices;
}, [phaseSegments]);
/** 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
@ -228,7 +239,7 @@ const ContentParts = memo(function ContentParts({
const previousPhaseIndices = previousPhaseIndicesRef.current;
useEffect(() => {
previousPhaseIndicesRef.current = completedPhaseIndices;
});
}, [completedPhaseIndices]);
const handleGroupExpansionChange = useCallback(
(groupId: string, state: ToolCallGroupExpansionState) => {

View file

@ -166,7 +166,7 @@ const Part = memo(function Part({
const failed = part.status === 'failed' || part.status === 'partial';
return (
<div
className={`my-1 break-words pl-1 text-sm italic ${failed ? 'text-amber-600 dark:text-amber-400' : 'text-text-secondary'}`}
className={`my-1 break-words pl-1 text-sm italic ${failed ? 'text-text-warning' : 'text-text-secondary'}`}
>
{display}
</div>

View file

@ -379,7 +379,7 @@ export default function ToolCallGroup({
<span
className={cn(
'tool-status-text min-w-0 truncate font-medium',
activityFailed && 'text-amber-600 dark:text-amber-400',
activityFailed && 'text-text-warning',
)}
role="status"
title={groupLabel}

View file

@ -12,42 +12,58 @@ jest.mock('~/hooks/Messages/useSmoothStreaming', () => ({
}));
jest.mock('~/hooks', () => {
const expandCollapse = jest.requireActual('~/hooks/Messages/useExpandCollapse');
return {
useExpandCollapse: jest.requireActual('~/hooks/Messages/useExpandCollapse').default,
useExpandCollapse: expandCollapse.default,
EXPAND_TRANSITION: expandCollapse.EXPAND_TRANSITION,
scheduleMessageContentLayoutReconcile: (target: HTMLElement | null) =>
mockScheduleLayoutReconcile(target),
};
});
const LABEL = 'Compared both release paths';
const labelPart = {
type: ContentTypes.ACTIVITY_LABEL,
[ContentTypes.ACTIVITY_LABEL]: 'Compared both release paths',
[ContentTypes.ACTIVITY_LABEL]: LABEL,
activity_label_type: 'phase',
activity_start_index: 0,
pending: false,
} as unknown as Extract<TMessageContentParts, { type: ContentTypes.ACTIVITY_LABEL }>;
describe('ActivityPhaseGroup', () => {
let collapseFrame: FrameRequestCallback | undefined;
let frames: Array<FrameRequestCallback | undefined>;
const originalRequestAnimationFrame = window.requestAnimationFrame;
const originalCancelAnimationFrame = window.cancelAnimationFrame;
/** The fold waits for a painted start value, so it spans more than one
* frame. Drain the queue the way a real compositor would. */
const flushFrames = () =>
act(() => {
for (let index = 0; index < frames.length; index += 1) {
const frame = frames[index];
frames[index] = undefined;
frame?.(index);
}
});
const pendingFrames = () => frames.filter((frame) => frame != null).length;
beforeEach(() => {
collapseFrame = undefined;
frames = [];
mockUseSmoothStreaming.mockReturnValue(true);
mockScheduleLayoutReconcile.mockClear();
Object.defineProperty(window, 'requestAnimationFrame', {
configurable: true,
writable: true,
value: jest.fn((callback: FrameRequestCallback) => {
collapseFrame = callback;
return 1;
}),
value: jest.fn((callback: FrameRequestCallback) => frames.push(callback)),
});
Object.defineProperty(window, 'cancelAnimationFrame', {
configurable: true,
writable: true,
value: jest.fn(),
value: jest.fn((handle: number) => {
frames[handle - 1] = undefined;
}),
});
});
@ -75,21 +91,43 @@ describe('ActivityPhaseGroup', () => {
expect(container.querySelector('.result-thinking')).toBeInTheDocument();
});
test('compresses visible activity into a newly streamed parent label', () => {
test('renders the label flush left with no leading glyph', () => {
render(
<ActivityPhaseGroup labelPart={labelPart} hasContent>
<div data-testid="phase-content" />
</ActivityPhaseGroup>,
);
const trigger = screen.getByRole('button', { name: LABEL });
expect(trigger).toHaveClass('justify-start', 'text-left');
expect(screen.getByText(LABEL)).toHaveClass('flex-1', 'text-left');
expect(trigger.querySelectorAll('svg')).toHaveLength(1);
});
test('mounts in the pre-marker shape, then folds the activity into the summary', () => {
const { container } = render(
<ActivityPhaseGroup labelPart={labelPart} hasContent animateEntrance>
<div data-testid="phase-content" />
</ActivityPhaseGroup>,
);
const trigger = screen.getByRole('button', { name: 'Compared both release paths' });
const card = container.querySelector('.my-2') as HTMLElement;
const trigger = screen.getByRole('button', { name: LABEL });
const header = trigger.parentElement?.parentElement as HTMLElement;
const panel = screen.getByTestId('activity-phase-panel');
/** Frame zero must be indistinguishable from the layout the marker
* replaced: no chrome, no header height, activity still open. */
expect(card).toHaveClass('border-transparent');
expect(card).not.toHaveClass('bg-surface-secondary/40');
expect(header).toHaveStyle({ gridTemplateRows: '0fr', opacity: '0' });
expect(trigger).toHaveAttribute('aria-expanded', 'true');
expect(panel).toHaveAttribute('aria-hidden', 'false');
expect(trigger).toHaveClass('animate-in', 'fade-in-0', 'slide-in-from-bottom-1');
act(() => collapseFrame?.(0));
flushFrames();
expect(header).toHaveStyle({ gridTemplateRows: '1fr', opacity: '1' });
expect(card).toHaveClass('border-border-light', 'bg-surface-secondary/40');
expect(trigger).toHaveAttribute('aria-expanded', 'false');
expect(panel).toHaveAttribute('aria-hidden', 'true');
expect(mockScheduleLayoutReconcile).toHaveBeenCalledTimes(1);
@ -102,10 +140,11 @@ describe('ActivityPhaseGroup', () => {
</ActivityPhaseGroup>,
);
const trigger = screen.getByRole('button', { name: 'Compared both release paths' });
const trigger = screen.getByRole('button', { name: LABEL });
const header = trigger.parentElement?.parentElement as HTMLElement;
expect(trigger).toHaveAttribute('aria-expanded', 'false');
expect(trigger).not.toHaveClass('animate-in');
expect(collapseFrame).toBeUndefined();
expect(header.getAttribute('style')).toBeNull();
expect(pendingFrames()).toBe(0);
fireEvent.click(trigger);
expect(trigger).toHaveAttribute('aria-expanded', 'true');
@ -114,15 +153,43 @@ describe('ActivityPhaseGroup', () => {
test('honors the smooth-streaming preference', () => {
mockUseSmoothStreaming.mockReturnValue(false);
const { container } = render(
<ActivityPhaseGroup labelPart={labelPart} hasContent animateEntrance>
<div data-testid="phase-content" />
</ActivityPhaseGroup>,
);
const trigger = screen.getByRole('button', { name: LABEL });
expect(trigger).toHaveAttribute('aria-expanded', 'false');
expect(container.querySelector('.my-2')).toHaveClass('border-border-light');
expect(pendingFrames()).toBe(0);
});
test('a click during the entrance wins over the scheduled fold', () => {
render(
<ActivityPhaseGroup labelPart={labelPart} hasContent animateEntrance>
<div data-testid="phase-content" />
</ActivityPhaseGroup>,
);
const trigger = screen.getByRole('button', { name: 'Compared both release paths' });
const trigger = screen.getByRole('button', { name: LABEL });
fireEvent.click(trigger);
expect(trigger).toHaveAttribute('aria-expanded', 'false');
expect(trigger).not.toHaveClass('animate-in');
expect(collapseFrame).toBeUndefined();
flushFrames();
expect(trigger).toHaveAttribute('aria-expanded', 'false');
});
test('a phase without activity renders a label-only header', () => {
render(
<ActivityPhaseGroup labelPart={labelPart} hasContent={false} animateEntrance>
<div data-testid="phase-content" />
</ActivityPhaseGroup>,
);
expect(screen.queryByRole('button')).not.toBeInTheDocument();
expect(screen.getByText(LABEL)).toHaveClass('text-left');
expect(pendingFrames()).toBe(0);
});
});

View file

@ -214,7 +214,7 @@ test.describe('parent activity phases', () => {
const finalTextIndex = content.findIndex((part) => contentPartText(part).includes(finalText));
expect(finalTextIndex).toBe(phasePart?.activity_end_index);
const parent = messagesView(page).locator(`summary[aria-label="${PARENT_LABEL}"]`);
const parent = messagesView(page).getByRole('button', { name: PARENT_LABEL, exact: true });
await expect(parent).toBeVisible({ timeout: 30000 });
await expect(messagesView(page).getByText(finalText)).toBeVisible({ timeout: 30000 });
await parent.click();
@ -241,7 +241,10 @@ test.describe('parent activity phases', () => {
).toBe(true);
await page.reload();
const reloadedParent = messagesView(page).locator(`summary[aria-label="${PARENT_LABEL}"]`);
const reloadedParent = messagesView(page).getByRole('button', {
name: PARENT_LABEL,
exact: true,
});
await expect(reloadedParent).toBeVisible({ timeout: 30000 });
await expect(messagesView(page).getByText(finalText)).toBeVisible();
await reloadedParent.click();