From d9dfdeb643fe9617fc5c5fb6245ed894072a1eb5 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Fri, 4 Sep 2026 17:44:23 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=92=AB=20style:=20Align=20the=20Phase=20S?= =?UTF-8?q?ummary=20Rail=20and=20Simplify=20its=20Fold=20(#15619)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 💫 style: Align the Phase Summary Rail and Simplify its Fold A phase summary was the only row in a transcript with no icon rail, so its text sat at 13px while the tool rows it stood for sat at 24px and the rows it swallowed moved out to 37px behind the card's `px-3` — three left edges in one block, with every folded row stepping 13px sideways as the box materialized. - The header takes the same 16px glyph slot every tool row has (`Check`, or `TriangleAlert` when the phase failed), which lands its text on the one rail. - The card chrome is gone: border, background, radius, body padding and divider. The summary is a row among rows, so nothing moves horizontally when it forms, and the entrance is the two grid rows trading places rather than four properties resolving at once. - The label is a ticker. A synthesized card re-titles itself every time it absorbs another finished block, and swapping that text instantly is what made the absorbed row look like it simply vanished; the retired summary now rises out of the clipped row while the new one comes up from below. - The label uses `tool-status-text`, so a summary scales with the reader's font-size setting like the rows around it instead of sitting at a fixed 14px. `ToolCallGroup`'s category glyph was a fourth rail at 28px (`h-5 w-5`); it now matches `ToolIcon` at `size-4`. `ease-[cubic-bezier(0.16,1,0.3,1)]` emitted nothing: `tailwindcss-animate` registers its own `ease` utility for `animation-timing-function` alongside Tailwind's `transition-timing-function` one, an arbitrary value matches both, and Tailwind resolves that ambiguity by dropping the class. The curve had never reached the chrome — the two-easings problem the comment warns about, caused by its own fix. It is written as an arbitrary property now, and it was the only such usage in the repo. * 🩹 fix: Settle the Phase Label in One Commit and Drop a Duplicate Media Query Review follow-ups on the phase ticker. - `useSmoothStreaming` already resolves to `smoothStreaming && !reducedMotion`; subscribing to the same media query again installed one `matchMedia` listener per phase card without changing the answer. - The label copied `text` into state from a passive effect, so a swap with no animation could paint the previous summary for a frame while the button's `aria-label` already carried the new one. It is adjusted during render now, which settles both paths in the same commit. - The incoming line kept its `animate-in` class only while the retired line existed, so clearing that line on its `animationend` could strip the class from a slide still in flight. The class now rides its own flag. - The test's label factory no longer needs `as unknown as`: every field on the ACTIVITY_LABEL part but `type` is optional, and the double cast was only there because a computed key widens to `string`. * ♿ fix: State the Phase Header's Inset Focus Ring Locally The header sits inside a permanent `overflow-hidden` wrapper — the 0fr/1fr grid needs it — so its focus ring has to be inset or it is drawn outside the border box and clipped away. That holds today only because the shared ghost variant supplies `ring-inset`; a change in `packages/client` could remove the indicator from here with no signal. It is declared on the element now, with a test that asserts both the clip and the inset ring. --- .../Messages/Content/ActivityPhaseGroup.tsx | 182 +++++++++++++----- .../Chat/Messages/Content/ToolCallGroup.tsx | 2 +- .../__tests__/ActivityPhaseGroup.test.tsx | 103 ++++++++-- .../ContentParts.integration.test.tsx | 3 + 4 files changed, 231 insertions(+), 59 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx b/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx index 5f205e066e..aa115260ec 100644 --- a/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ActivityPhaseGroup.tsx @@ -1,7 +1,7 @@ import { useId, useCallback, useEffect, useMemo, useRef, useState } from 'react'; import { Button } from '@librechat/client'; -import { ChevronDown } from 'lucide-react'; import { ContentTypes } from 'librechat-data-provider'; +import { Check, ChevronDown, TriangleAlert } from 'lucide-react'; import type { TAttachment, TMessageContentParts } from 'librechat-data-provider'; import type { CSSProperties, ReactNode } from 'react'; import { @@ -16,10 +16,17 @@ import { AttachmentGroup, 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)] motion-reduce:transition-none'; +/** Matches `EXPAND_TRANSITION` so the panel and the label ticker resolve on + * the same curve — two properties animating on two different easings is what + * makes a fold read as two separate movements. + * + * Written as an arbitrary PROPERTY, not `ease-[…]`: `tailwindcss-animate` + * registers its own `ease` utility for `animation-timing-function` alongside + * Tailwind's `transition-timing-function` one, so an arbitrary value matches + * both, and Tailwind resolves that ambiguity by emitting NOTHING. The class + * this replaces had been inert since the plugin landed — the curve it names + * never reached the fold. */ +const FOLD_EASING = 'duration-300 [animation-timing-function:cubic-bezier(0.16,1,0.3,1)]'; type ActivityPhasePart = Extract & { activity_label_type?: 'phase'; @@ -49,6 +56,100 @@ function schedulePostPaint(callback: () => void): () => void { }; } +/** The header's icon slot. Its only job is geometric: every other row in the + * transcript opens with a 16px glyph and an 8px gap, so a summary rendered + * without one sits 24px to the left of the rows it replaces — the fold then + * moves its own text sideways at the moment the reader is trying to follow + * it. */ +function PhaseGlyph({ failed }: { failed: boolean }) { + const Icon = failed ? TriangleAlert : Check; + return ( + + ); +} + +/** + * The phase header is a ticker, not a title. A client-synthesized card + * re-titles itself every time it absorbs another finished block, and swapping + * that text instantly is what makes the absorbed row look like it simply + * vanished from under the reader. The retired line rises out of the clipped + * row while the new summary comes up from below, so the work visibly moves + * into the line that now stands for it. + */ +function PhaseLabel({ + text, + animate, + failed, +}: { + text: string; + animate: boolean; + failed: boolean; +}) { + const [lines, setLines] = useState<{ current: string; retired: string | null; entered: boolean }>( + { current: text, retired: null, entered: false }, + ); + + /** Adjusted during render rather than in an effect. A passive effect runs + * after paint, so a swap with no animation would leave the previous summary + * on screen for a frame while the button's `aria-label` already carried the + * new one. React re-renders this component immediately instead. */ + if (lines.current !== text) { + const swaps = animate && lines.current.length > 0; + setLines({ current: text, retired: swaps ? lines.current : null, entered: swaps }); + } + + /** Clears only the retired line. `entered` outlives it on purpose: dropping + * the incoming line's animation class the moment its partner's + * `animationend` fires would snap a still-running slide back to its resting + * position. The class is inert once the animation has finished, and the + * element is keyed by its text, so it cannot replay. */ + const clearRetired = useCallback(() => { + setLines((previous) => (previous.retired == null ? previous : { ...previous, retired: null })); + }, []); + + return ( + + {lines.retired != null && ( + + )} + + {lines.current} + + + ); +} + export default function ActivityPhaseGroup({ labelPart, children, @@ -74,18 +175,24 @@ export default function ActivityPhaseGroup({ }) { const label = getActivityLabelText(labelPart); const hasFailure = labelPart.status === 'failed' || labelPart.status === 'partial'; + /** Already `smoothStreaming && !reducedMotion` — it owns the media query, so + * a second subscription here would install one `matchMedia` listener per + * phase card without changing the answer. */ 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 fold before its first animation frame. */ const [shouldAnimateEntrance] = useState(smoothStreaming && animateEntrance && label.length > 0); /** 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. */ + * looking at. The header therefore mounts at zero height with the panel + * open — the shape of what was there BEFORE it — and trades one for the + * other 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. Those two grid rows are the ONLY + * properties in flight: the card chrome that used to fade in alongside + * them (border, background, padding, divider) is gone, and with it the + * sideways step its inset used to impose on every folded row. */ const foldsIn = shouldAnimateEntrance && hasContent; const [isExpanded, setIsExpanded] = useState(foldsIn); const [isSettled, setIsSettled] = useState(!foldsIn); @@ -182,13 +289,15 @@ export default function ActivityPhaseGroup({ const group = !hasContent ? (
+
) : ( -
+ /** No chrome. A phase summary is another row in the same list as the tool + * groups it stands for, so it carries the same geometry: 16px glyph, 8px + * gap, no inset. Boxing it was what put its text on a third left edge and + * forced every folded row 13px sideways as the box materialized. */ +
diff --git a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx index e40ecddbc4..a6a8868a26 100644 --- a/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx +++ b/client/src/components/Chat/Messages/Content/ToolCallGroup.tsx @@ -356,7 +356,7 @@ export default function ToolCallGroup({ * rather than "tools". */