From bc8933042fdb69c4f31ec7cd7f81a588007b1642 Mon Sep 17 00:00:00 2001 From: Brooklyn Nicholson Date: Mon, 27 Jul 2026 19:54:47 -0500 Subject: [PATCH] fix(desktop): stop the enter animation pinning opacity over the stylesheet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two identical tool rows, one above the other, rendered at two different opacities — and no amount of hovering would even them out. The enter animation fills forwards, so its final keyframe is held in the animation origin of the cascade for as long as the element lives, above the author stylesheet. Naming `opacity: 1` there didn't just end the fade, it permanently overruled the resting opacity of every element the sheet dims. Transcript scaffolding is dimmed exactly that way, so a row kept whichever opacity it happened to mount with: full if it animated in during the turn, faded if it was rehydrated or remounted past its one-shot key. Same for thinking headers, which is why "Thought" never matched the rows near it. Leave the end opacity out of the keyframe. It animates up to whatever CSS asks for and keeps answering to it, hover included. Then close the way the surfaces drifted in the first place: the fade named each one in its own selector, so the live status line — added later, and neither tool nor thinking nor prose — matched none of them and sat a shade brighter than the rows either side. One `data-conversation-scaffold` mark now carries it, and every surface opts in. --- .../assistant-ui/thread/message-parts.tsx | 1 + .../assistant-ui/thread/status.test.tsx | 15 ++++ .../components/assistant-ui/thread/status.tsx | 1 + .../components/assistant-ui/tool/fallback.tsx | 3 +- .../assistant-ui/tool/tool-group.test.tsx | 17 ++++ .../src/components/chat/scaffold-row.tsx | 4 + .../src/lib/use-enter-animation.test.tsx | 82 +++++++++++++++++++ apps/desktop/src/lib/use-enter-animation.ts | 12 ++- apps/desktop/src/styles.css | 26 +++--- 9 files changed, 146 insertions(+), 15 deletions(-) create mode 100644 apps/desktop/src/lib/use-enter-animation.test.tsx diff --git a/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx b/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx index 508ead3ec9d..63f2c9fb2a1 100644 --- a/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/message-parts.tsx @@ -136,6 +136,7 @@ const ThinkingDisclosure: FC<{ return (
diff --git a/apps/desktop/src/components/assistant-ui/thread/status.test.tsx b/apps/desktop/src/components/assistant-ui/thread/status.test.tsx index f6edaee0d7e..e51f39313a7 100644 --- a/apps/desktop/src/components/assistant-ui/thread/status.test.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/status.test.tsx @@ -54,3 +54,18 @@ describe('ResponseLoadingIndicator timer', () => { expect(screen.getAllByText((_, node) => node?.textContent === '8s').length).toBeGreaterThan(0) }) }) + +// The status line sits between tool rows and thinking headers, which the +// transcript rests at a fade. Without the mark it reads a shade brighter than +// both — the one line in the column claiming emphasis it hasn't earned. +describe('status line', () => { + afterEach(cleanup) + + it('is marked as transcript scaffolding', () => { + $activeSessionId.set('session-a') + $turnStartedAt.set(Date.now()) + const { container } = renderIndicator() + + expect(container.querySelector('[role="status"]')?.hasAttribute('data-conversation-scaffold')).toBe(true) + }) +}) diff --git a/apps/desktop/src/components/assistant-ui/thread/status.tsx b/apps/desktop/src/components/assistant-ui/thread/status.tsx index 62ad0940f75..83cd107a292 100644 --- a/apps/desktop/src/components/assistant-ui/thread/status.tsx +++ b/apps/desktop/src/components/assistant-ui/thread/status.tsx @@ -34,6 +34,7 @@ const StatusRow: FC<{ children: ReactNode; label: string } & React.ComponentProp 'text-(--conversation-scaffold-text)', className )} + data-conversation-scaffold="" role="status" {...rest} > diff --git a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx index 3cb3445c089..966e977b177 100644 --- a/apps/desktop/src/components/assistant-ui/tool/fallback.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/fallback.tsx @@ -510,6 +510,7 @@ function ToolEntry({ part }: ToolEntryProps) { 'group/tool-block min-w-0 max-w-full overflow-hidden text-[length:var(--conversation-tool-font-size)] text-(--ui-text-tertiary)', open && TOOL_EXPANDED_SHELL_CLASS )} + data-conversation-scaffold="" data-file-edit={isFileEdit && open ? '' : undefined} data-slot="tool-block" data-tool-open={open ? '' : undefined} @@ -795,7 +796,7 @@ function ToolRunHeader({ summary: string }) { return ( -
+
{live ? {summary} : summary} diff --git a/apps/desktop/src/components/assistant-ui/tool/tool-group.test.tsx b/apps/desktop/src/components/assistant-ui/tool/tool-group.test.tsx index 9ed31b76ae4..65e9ab34d07 100644 --- a/apps/desktop/src/components/assistant-ui/tool/tool-group.test.tsx +++ b/apps/desktop/src/components/assistant-ui/tool/tool-group.test.tsx @@ -476,6 +476,23 @@ describe('a file edit among ordinary activity', () => { }) }) +// The transcript rests its scaffolding at a fade, keyed off one attribute. A +// surface that renders without it is brighter than everything around it, which +// is how two adjacent, identical rows came to sit at two opacities. +describe('transcript fade', () => { + it('marks every row and summary as scaffolding', async () => { + const { container } = render() + + await screen.findByText('Explored 2 files') + + const unmarked = [...container.querySelectorAll('[data-tool-summary],[data-tool-row]')].filter( + node => !node.hasAttribute('data-conversation-scaffold') + ) + + expect(unmarked).toHaveLength(0) + }) +}) + describe('live tool run', () => { it('keeps its rows on screen instead of hiding them behind the summary', async () => { const { container } = render() diff --git a/apps/desktop/src/components/chat/scaffold-row.tsx b/apps/desktop/src/components/chat/scaffold-row.tsx index dd30ff1025c..23e0c737a78 100644 --- a/apps/desktop/src/components/chat/scaffold-row.tsx +++ b/apps/desktop/src/components/chat/scaffold-row.tsx @@ -11,6 +11,10 @@ import { DisclosureRow } from '@/components/chat/disclosure-row' * pick their own grey — a thinking header painted `--ui-text-secondary`, a tool * summary `--ui-text-tertiary`, both under the same opacity — which read as two * different kinds of line for what is one kind of thing. + * + * The resting fade is the other half and lives in CSS, on the *block* that + * holds the row rather than on the row: mark it `data-conversation-scaffold`. + * A surface that skips the mark reads a shade brighter than its neighbours. */ export const SCAFFOLD_LABEL_CLASS = 'text-[length:var(--conversation-tool-font-size)] leading-(--conversation-line-height) text-(--conversation-scaffold-text)' diff --git a/apps/desktop/src/lib/use-enter-animation.test.tsx b/apps/desktop/src/lib/use-enter-animation.test.tsx new file mode 100644 index 00000000000..936aeab2668 --- /dev/null +++ b/apps/desktop/src/lib/use-enter-animation.test.tsx @@ -0,0 +1,82 @@ +import { cleanup, render } from '@testing-library/react' +import { afterEach, describe, expect, it } from 'vitest' + +import { useEnterAnimation } from './use-enter-animation' + +interface PlayedAnimation { + keyframes: Keyframe[] + options: KeyframeAnimationOptions +} + +/** + * Mounts one element through the hook and reports the animation it played, if + * any. jsdom has no Web Animations API, so `animate` is the seam. + */ +function mountAnimated(enabled: boolean, animationKey?: string): PlayedAnimation | undefined { + let played: PlayedAnimation | undefined + + function Probe() { + const ref = useEnterAnimation(enabled, animationKey) + + return
+ } + + // Defined rather than spied on: jsdom ships no Web Animations API at all, so + // there is no `animate` to wrap. + Object.defineProperty(HTMLElement.prototype, 'animate', { + configurable: true, + value: (keyframes: Keyframe[], options: KeyframeAnimationOptions) => { + played = { keyframes, options } + + return {} as Animation + }, + writable: true + }) + render() + + return played +} + +afterEach(() => { + cleanup() + Reflect.deleteProperty(HTMLElement.prototype, 'animate') +}) + +describe('useEnterAnimation', () => { + it('plays once on mount', () => { + const played = mountAnimated(true, 'plays-once') + + expect(played).toBeDefined() + expect(played?.keyframes[0]).toMatchObject({ opacity: 0 }) + }) + + it('stays out of the way when disabled', () => { + expect(mountAnimated(false, 'disabled')).toBeUndefined() + }) + + // A key is only banked once the node survives a microtask, so that a mount + // React immediately tears down doesn't burn it. + it('does not replay for a key that already animated', async () => { + expect(mountAnimated(true, 'replay')).toBeDefined() + await Promise.resolve() + + expect(mountAnimated(true, 'replay')).toBeUndefined() + }) + + /** + * The animation fills forwards, so any value in its last keyframe is held in + * the animation origin of the cascade for the life of the element — above + * the stylesheet. Naming an end opacity therefore doesn't just finish the + * fade, it permanently overrules whatever opacity CSS wants the element to + * rest at, and transcript scaffolding rests dimmed. Rows that animated in + * during the turn stayed bright while their rehydrated neighbours faded, and + * no hover could lift the bright ones because the sheet had lost the + * argument. Opacity has to be left to CSS at the end. + */ + it('leaves the resting opacity to the stylesheet', () => { + const played = mountAnimated(true, 'resting-opacity') + + expect(played?.options.fill).toBe('both') + expect(played?.keyframes.at(-1)).not.toHaveProperty('opacity') + }) +}) diff --git a/apps/desktop/src/lib/use-enter-animation.ts b/apps/desktop/src/lib/use-enter-animation.ts index 259cf7a388d..267eaa4cac5 100644 --- a/apps/desktop/src/lib/use-enter-animation.ts +++ b/apps/desktop/src/lib/use-enter-animation.ts @@ -82,7 +82,17 @@ export function useEnterAnimation(enabled: boolean, animationKey?: string): (el: el.animate( [ { opacity: 0, transform: 'translateY(0.375rem)' }, - { opacity: 1, transform: 'translateY(0)' } + // No `opacity` on the way out, deliberately. A filled animation holds + // its final value in the animation origin of the cascade, which + // outranks the stylesheet for as long as the element lives — naming 1 + // here permanently pinned full opacity onto everything the sheet dims. + // Transcript scaffolding is dimmed that way, so a tool row or thinking + // header kept whichever opacity it happened to mount with: full if it + // animated in during the turn, faded if it was rehydrated or remounted + // past its one-shot key. Adjacent identical rows disagreed, and hover + // couldn't lift the pinned ones. Left neutral, opacity rises to + // whatever CSS says it should be and answers hover afterwards. + { transform: 'translateY(0)' } ], { duration: 180, easing: 'cubic-bezier(0.16, 1, 0.3, 1)', fill: 'both' } ) diff --git a/apps/desktop/src/styles.css b/apps/desktop/src/styles.css index 106b196b1af..541da35b6bc 100644 --- a/apps/desktop/src/styles.css +++ b/apps/desktop/src/styles.css @@ -1416,16 +1416,18 @@ text-* variant utilities. */ .btn-arc { background: transparent !important; } -/* Fade scaffolding so the prose reading column stays primary. Two targets: - a thinking disclosure fades as one block, and each *individual* tool row - (`[data-tool-row]`) fades on its own. We deliberately do NOT fade the tool - group wrapper (`[data-tool-group]`): opacity on a parent opens a stacking - context, so a child row can never be more opaque than the group — that made - it impossible to keep one row lit (an open diff) while its siblings faded. - With the fade per-row, each row hovers/focuses independently. */ -[data-slot='aui_assistant-message-content'] > [data-slot='aui_thinking-disclosure'], -[data-slot='aui_assistant-message-content'] [data-tool-summary], -[data-slot='aui_assistant-message-content'] [data-slot='tool-block'][data-tool-row] { +/* Fade scaffolding so the prose reading column stays primary. Each surface + opts in with `data-conversation-scaffold` — thinking header, tool row, run + summary, live status line — instead of being named by its own selector here. + Spelling them out individually is how the status line came to sit a shade + brighter than the rows either side of it. + + Marked per surface and never on a container: opacity opens a stacking + context, so nothing inside a faded parent can be more opaque than it, and + one row (an open diff) could not stay lit while its siblings dimmed. Hence + the tool group wrapper carries no mark and each row inside it carries its + own — no two marked elements nest, so the fade never compounds. */ +[data-slot='aui_assistant-message-content'] [data-conversation-scaffold] { opacity: 0.67; transition: opacity 120ms ease-out; } @@ -1433,9 +1435,7 @@ text-* variant utilities. */ .btn-arc { /* Lift on hover or *keyboard* focus only. `:focus-within` also matches the focus a mouse click leaves on the disclosure toggle, which kept a row lit after you clicked to collapse it; `:has(:focus-visible)` excludes that. */ -[data-slot='aui_assistant-message-content'] > [data-slot='aui_thinking-disclosure']:is(:hover, :has(:focus-visible)), -[data-slot='aui_assistant-message-content'] [data-tool-summary]:is(:hover, :has(:focus-visible)), -[data-slot='aui_assistant-message-content'] [data-slot='tool-block'][data-tool-row]:is(:hover, :has(:focus-visible)) { +[data-slot='aui_assistant-message-content'] [data-conversation-scaffold]:is(:hover, :has(:focus-visible)) { opacity: 1; }