fix(desktop): scroll chat to the latest reply on session resume - #10501
Conversation
A smooth programmatic scrollToBottom emitted intermediate scroll events that handleScroll mistook for a manual scroll-up, disabling auto-follow before late-loading content (images, syntax highlighting) finished growing, leaving the conversation parked at an earlier point on resume. Use an instant scroll for programmatic auto-scroll (no intermediate events to misread) and add a ResizeObserver that re-pins to the bottom while following, covering async height growth that doesn't trigger a React re-render. Closes aaif-goose#10483 Signed-off-by: Seydi Charyyev <seydi.charyev@gmail.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 126fa800ff
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| if (isFollowing && !userScrolledUpRef.current) { | ||
| viewport.scrollTo({ top: viewport.scrollHeight, behavior: 'auto' }); |
There was a problem hiding this comment.
Respect active user scrolling before resize auto-follow
When auto-scrolled chat content is still growing and the user starts scrolling up, handleScroll only marks userScrolledUpRef after the viewport is more than the 200px bottom threshold away; until then it relies on isActivelyScrollingRef to avoid fighting the user, as the children-change path above already does. This new resize observer ignores that active-scroll guard, so streamed text wrapping or late media/code sizing can immediately scrollTo the bottom during those first wheel/trackpad events and make it difficult to manually scroll away from the live tail.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Addressed: added the isActivelyScrolling guard to the resize re-pin so it matches the [children] effect. Verified against the ScrollArea component — a user scrolling up within the 200px threshold while content is still growing is no longer pulled back to the bottom, and resume still lands on the latest reply.
Guard the ResizeObserver re-pin with isActivelyScrolling, matching the [children] auto-scroll effect, so streamed or late content growth doesn't yank the viewport to the bottom while the user is scrolling up within the bottom threshold. Signed-off-by: Seydi Charyyev <seydi.charyev@gmail.com>
lifeizhou-ap
left a comment
There was a problem hiding this comment.
Thank you so much for the prompt fix!
The instant programmatic scroll seems good and isn’t jarring, so I’m happy with keeping behavior: 'auto' instead of adding the complexity needed to preserve smooth scrolling.
Summary
On session resume the chat could load scrolled to an earlier point in the conversation instead of the latest reply (#10483).
Root cause:
scrollToBottominScrollAreaanimated withbehavior: 'smooth'. A smooth programmatic scroll emits intermediatescrollevents where the viewport isn't at the bottom yet, andhandleScrollcan't tell that from the user scrolling up — so it setsisFollowing = false/userScrolledUp = truemid-animation. Content that grows after the first paint (images, syntax-highlighted code) then isn't followed, because that late growth resizes the DOM without a React re-render, so the[children]auto-scroll effect never fires. The view stays parked above the bottom.Fix:
behavior: 'auto'for programmatic auto-scroll (resume + follow), so there are no intermediate frames to misread as a manual scroll-up.ResizeObserveron the scroll content that re-pins to the bottom while following, covering async height growth the[children]effect can't see.Manual scroll-up still disables auto-follow (unchanged): the resize re-pin is gated on
isFollowing && !userScrolledUp.Trade-off for maintainers: this makes programmatic auto-scroll instant rather than animated (resume and the follow-during-streaming path). For resume it avoids animating on every open. If you'd rather keep the smooth animation, the alternative is to guard the scroll handler against in-flight programmatic scrolls so a smooth
scrollToBottomisn't misread as a scroll-up — I can switch to that.Testing
Verified in Chromium against the real
ScrollAreacomponent (standalone Playwright harness): after late content growth the resume scenario ends at the bottom (distanceFromBottom = 0); before the fix it parked ~800px above the bottom. A user-scroll-up check confirms the view stays where the user left it when content later grows.pnpm run lint:check,pnpm run typecheck, andpnpm run test:run(564 passed) all pass.This is layout/timing behaviour the jsdom unit tests can't exercise (no layout) and the Electron
tests/e2esuite isn't a natural fit; I can share the standalone reproduction or add an e2e test if there's a preferred pattern.Related Issues
Closes #10483