From 47dbcb4910c74cf638a3f82c8fe3df9eded9a9d8 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Mon, 31 Aug 2026 21:15:28 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8E=AF=20fix:=20Keep=20the=20Thinking=20L?= =?UTF-8?q?abel=20on=20the=20Lightbulb's=20Axis=20While=20Shimmering=20(#1?= =?UTF-8?q?5437)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The shimmer PR (#15431) nested the label inside a span carrying `.shimmer`, which is `display: inline-block`, alongside `truncate`. An inline-block whose overflow is not visible takes its baseline from its bottom margin edge rather than from its last line box, so the label row's line box grew from 18px to 23px and the text lifted 2px while the lightbulb — pinned to the row's 30px centre by the copy button — stayed put. Every streaming reasoning header sat off its own icon's axis until the label settled back to "Thoughts". The sweep now rides the label row itself. That row is a flex item, so `.shimmer`'s `inline-block` is blockified away and the text lands exactly where the un-shimmered label lands: measured against the real component, both states report an 18px box and the same text centre, where the nested form reported 23px and a 2px offset. The nested span survives only for the generated-label entrance, which cannot share an element with the sweep because both drive `animation-name`. It keeps the entrance outside — the clipped element paints the glyphs itself, so a descendant's opacity cannot fade them — and takes `align-top` so the baseline rule above cannot reopen the gap there. Found by rendering the real component in a browser; a hand-built reproduction of the same classes did not show it, and neither did the DOM assertions, which is why the regression shipped. --- .../Chat/Messages/Content/Parts/Thinking.tsx | 26 ++++++++++++++----- .../Parts/__tests__/Reasoning.spec.tsx | 13 ++++++++++ 2 files changed, 32 insertions(+), 7 deletions(-) diff --git a/client/src/components/Chat/Messages/Content/Parts/Thinking.tsx b/client/src/components/Chat/Messages/Content/Parts/Thinking.tsx index 3be18b6056..6e142c11e8 100644 --- a/client/src/components/Chat/Messages/Content/Parts/Thinking.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/Thinking.tsx @@ -98,12 +98,19 @@ export const ThinkingButton = memo( aria-hidden="true" /> - {/* The entrance and the shimmer both drive `animation-name`, so they - cannot share an element — and the nesting order is not a free - choice either. The clipped element paints the glyphs itself, so a - descendant's opacity cannot fade them: entrance outside and - shimmer inside fades correctly, while the reverse leaves the label - fully lit right through its own fade. + {/* The sweep rides the label row itself. That row is a flex item, so + `.shimmer`'s `inline-block` is blockified away and the text sits + exactly where the un-shimmered label sits — which matters, because + an `inline-block` carrying `truncate` takes its baseline from its + bottom margin edge, growing the line box and lifting the label off + the icon's axis. + + Only the generated-label entrance forces a nested span: it and the + sweep both drive `animation-name`, so they cannot share an element. + There the entrance must stay outside — the clipped element paints + the glyphs itself, so a descendant's opacity cannot fade them — and + the inner span takes `align-top` to keep the baseline rule above + from reopening the same gap. `key` remounts the row so the entrance replays, and that restarts the sweep with it. Reasoning labels revise on a 3s default against @@ -114,11 +121,16 @@ export const ThinkingButton = memo( key={animateLabel ? label : undefined} className={cn( 'min-w-0 truncate text-left', + shimmerLabel && !animateLabel && 'shimmer', animateLabel && 'duration-300 ease-out animate-in fade-in-0 slide-in-from-bottom-1 motion-reduce:animate-none', )} > - {shimmerLabel ? {label} : label} + {shimmerLabel && animateLabel ? ( + {label} + ) : ( + label + )} {content && showCopyButton && ( diff --git a/client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx b/client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx index 8c1c506e21..6a77cca744 100644 --- a/client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx +++ b/client/src/components/Chat/Messages/Content/Parts/__tests__/Reasoning.spec.tsx @@ -27,6 +27,19 @@ describe('Reasoning label shimmer', () => { expect(screen.getByText('Thinking...')).toHaveClass('shimmer'); }); + /** The sweep has to ride the label row itself rather than a span nested inside + * it. That row is a flex item, so `.shimmer`'s `inline-block` is blockified + * away; nested, it is an inline-block whose `truncate` moves its baseline to + * its bottom margin edge, which grew the line box 18px -> 23px and lifted the + * label 2px off the lightbulb's axis for the whole of a generation. */ + it('keeps the sweep on the label row instead of a nested span', () => { + renderReasoning(); + const label = screen.getByText('Thinking...'); + + expect(label).toHaveClass('truncate'); + expect(label.querySelector('.shimmer')).toBeNull(); + }); + it('leaves settled thoughts unanimated', () => { renderReasoning({ isSubmitting: false }); expect(screen.getByText('Thoughts')).not.toHaveClass('shimmer');