Skip to content

fix(react): drain released tail spacer after manual scroll during anchored stream - #11465

Open
gonzoblasco wants to merge 9 commits into
shadcn-ui:mainfrom
gonzoblasco:fix/message-scroller-drain-released-spacer
Open

fix(react): drain released tail spacer after manual scroll during anchored stream#11465
gonzoblasco wants to merge 9 commits into
shadcn-ui:mainfrom
gonzoblasco:fix/message-scroller-drain-released-spacer

Conversation

@gonzoblasco

Copy link
Copy Markdown

Fixes #11125

Problem

With autoScroll + scrollAnchor (the message-scroller-streaming example), a manual scroll during an anchored streaming turn releases the anchor and strands the reserved tail spacer. What happens next depends on autoScroll:

  • autoScroll on: the scroll releases the anchor, follow-mode re-arms, and scrollToEnd zeroes the spacer in a single frame - the reserved room collapses and the viewport is yanked to the live edge, even when the reply is still small.
  • autoScroll off: nothing re-arms follow, so the spacer freezes at its current height - a viewport of dead space is stranded below the reply and never resolves.

Either way, a normal reader action (scrolling while a reply streams) destroys the reserved room.

Root cause

Two gaps in the controller:

  1. The re-arm has no spacer condition. In reconcileFollowMode, the branch that arms following-bottom only excludes settling-jump and anchored-to-message. A commit arriving with the mode in free-scrolling and !scrollable.end re-arms anyway. And since scrollable.end measures against contentBottom (which excludes the tail spacer), a reader sitting inside the spacer zone reads end: false and their own gesture flips them back into following. The next chunk calls scrollToEnd -> collapse in a single frame -> the yank.

  2. A released spacer is never drained. The tail spacer height is only consumed by scrollToEnd in the follow-output path. When follow releases during a manual scroll, nothing reconciles the spacer back to zero.

Fix

  • Gate the re-arm on spacerHeight === 0 in reconcileFollowMode, so a reader still inside the spacer zone cannot re-arm following with their own gesture.
  • Drain a released spacer in handleResize: when not in follow-mode and a spacer is reserved, shrink it toward zero as the reply grows (spacerHeight = max(0, scrollTop + clientHeight - contentBottom)), so the reply fills the reserved room without moving the reader. Follow re-engages once the spacer hits zero at the real bottom.

This mirrors how reanchorToAnchoredMessage already consumes the spacer in the hold mode, and reuses the existing setTailSpacerHeight (now exposed from useMessageScrollerCommands).

Testing

  • Added a regression test: a manual scroll during an anchored stream releases the anchor, the reply drains the spacer instead of freezing it, and follow re-engages once the reply fills the viewport.
  • 61 message-scroller tests pass, typecheck clean.

@vercel

vercel Bot commented Aug 10, 2026

Copy link
Copy Markdown
Contributor

@gonzoblasco is attempting to deploy a commit to the shadcn-pro Team on Vercel.

A member of the Team first needs to authorize it.

daltino

This comment was marked as resolved.

Addresses review feedback: a manual scroll during an anchored stream can
leave the reserved tail spacer stranded if the reply stops growing before
it drains (no more resizes to drive the drain). Extract the drain into
drainReleasedSpacer and run it on user scroll too, so a scroll to the real
bottom reclaims the dead space and re-arms following once the spacer hits
zero. Document why setTailSpacerHeight lives on the commands surface.
@gonzoblasco

Copy link
Copy Markdown
Author

Thanks for the careful review - both points are valid, and the edge case is real.

On the spacerHeightRef.current === 0 guard: you're right that the drain only lived in handleResize, which fires off ResizeObserver on the viewport and the content. If the stream ends with the spacer mid-drain (a small non-zero value), no further resizes drive the drain, and the guard permanently blocks follow-mode re-arming. The test covered the happy path (stream continues until the spacer fully drains), not that cut.

The fix extracts the drain into drainReleasedSpacer and runs it on two paths:

  • handleResize (reply growing): the reply grows into the reserved room, so the spacer shrinks by the absolute room left (getTailSpacerHeight).
  • syncAfterScroll (user scroll): with content fixed, the spacer shrinks by the amount the reader has already scrolled past within the room (spacer - consumed).

So if the stream cuts short, a scroll to the real bottom - contentBottom plus the remaining spacer - finishes the job: it reclaims the dead space and re-arms following once the spacer hits zero. I added a test that exercises exactly that scenario (stream ends mid-drain, then a scroll to the bottom drains the residual and re-engages following).

On setTailSpacerHeight on the commands surface: agreed, I added comments. The reason it lives there: it's a low-level spacer primitive (it doesn't move the viewport, it only writes the reserved height and hides the spacer at zero), and both reanchorToAnchoredMessage (reserving room for the anchored turn) and the new drainReleasedSpacer (consuming it) need the same write. Exposing it through the commands keeps that write in one place instead of duplicating spacer manipulation in the controller. It's not a user-facing scroll command - more an internal handle the orchestrator threads through.

Both changes are in the updated branch (62 message-scroller tests pass, typecheck clean).

@gonzoblasco
gonzoblasco requested a review from daltino August 11, 2026 02:51
@vercel

vercel Bot commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated
ui Ready Ready Preview Sep 2, 2026 1:34am UTC

Request Review

@shadcn

shadcn commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

@gonzoblasco thanks for the PR. reconcileFollowMode looks correct.

Unfortunately the new drainReleasedSpacer has a problem that the tests can't see: syncAfterScroll runs on every scroll event, including the programmatic one that places an anchored turn. In a real browser with autoScroll on, every anchored turn gets yanked to the bottom, which is the exact bug we're trying to fix. The jsdom tests pass because jsdom doesn't fire scroll events for programmatic scrolls.

Two smaller issues in the same function:

On resize, if the reader has scrolled up past the spacer zone, consumed is 0, so the spacer collapses and scrollToEnd yanks them mid-transcript.
On scroll, the drain subtracts an absolute overlap on every event instead of a delta, so repeated scroll events (like inertial scrolling) over-drain and cascade to a yank.

To get this mergeable, the drain needs to skip the anchored-to-message/settling-jump modes and programmatic scrolls, only hand off to scrollToEnd when the reader is actually at the bottom, use idempotent math on the scroll path — and add a browser-mode test with autoScroll on, since jsdom structurally can't catch this class of bug.

Happy to look again after an update!

This review was written partly with the help of Fable 5.

gonzoblasco and others added 2 commits September 2, 2026 19:49
…tently

The drain ran on every scroll event, including the programmatic one that
places an anchored turn. In a real browser that yanked every anchored turn
to the bottom with autoScroll on. Skip the drain while a programmatic scroll
is in flight (anchored-to-message, settling-jump, autoscrolling) and recompute
the remaining spacer room from absolute geometry on every event so inertial
scrolling converges instead of over-draining. Hand off to scrollToEnd only
when the reader reaches the real bottom; leave the spacer untouched while the
reader is above the spacer zone.

Adds browser-mode regression tests: anchored placement does not yank, and a
manual scroll during an anchored stream drains the spacer without moving the
reader.
@gonzoblasco

gonzoblasco commented Sep 2, 2026

Copy link
Copy Markdown
Author

@shadcn all three points are addressed in 1b97764:

1. Programmatic scrolls no longer drain. drainReleasedSpacer now skips anchored-to-message and settling-jump modes and autoscrollingRef, so the scroll event fired by the placement scroll can no longer collapse the spacer and yank the anchored turn. The hold path (reanchorToAnchoredMessage) remains the only consumer of the spacer while anchored.

2. Idempotent math. The drain now recomputes the remaining room from absolute geometry on every event - max(0, contentBottom + spacerHeight - (scrollTop + clientHeight)) - instead of subtracting an absolute overlap. Inertial scrolling converges instead of over-draining.

3. Hand-off only at the real bottom. The drain returns early when the reader is above the spacer zone (overlap <= 0), leaving the spacer untouched, and only calls scrollToEnd when the recomputed spacer hits zero - which by construction means the reader is at the live edge.

4. Browser-mode regression tests. Two tests in message-scroller.browser.test.tsx with autoScroll on: anchored placement does not yank to the bottom (the spacer stays reserved), and a manual scroll during an anchored stream releases the anchor, the reply drains the spacer without moving the reader, and scrolling to the real bottom re-engages following.

Verification: jsdom 51/51, browser mode 48/48, typecheck clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[bug]: MessageScroller: a manual scroll during an anchored streaming turn destroys the reserved tail spacer

4 participants