fix(web): preserve scroll when composer grows - #5234
Conversation
📝 WalkthroughWalkthroughThe timeline now tracks layout-follow state by thread. Manual navigation can disable layout retention until the timeline returns to the end. ChangesTimeline scroll control
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant ChatView
participant MessagesTimeline
participant LegendList
User->>MessagesTimeline: Navigate with wheel, touch, or scrollbar
MessagesTimeline->>ChatView: Report manual navigation
ChatView->>ChatView: Update thread follow state
ChatView->>MessagesTimeline: Pass layout-follow flag
MessagesTimeline->>LegendList: Configure layout retention
LegendList-->>MessagesTimeline: Retain data and item-layout positions
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
vouch:unvouched
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/chat/MessagesTimeline.test.tsx (1)
428-442: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftAdd a focused test for the
ChatViewContenthandoff.This test verifies the
LegendListconfiguration after the prop is manually set tofalse. It does not verify thatChatViewContentdisables retention after a real scroll-away event or restores it after scroll-to-end, thread changes, and sends. Add one focused interaction test for that state handoff.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/chat/MessagesTimeline.test.tsx` around lines 428 - 442, The existing test only validates LegendList configuration with maintainScrollAtEndOnLayout manually set to false; add a focused interaction test covering the ChatViewContent handoff after a real scroll-away event. Verify retention is disabled when the user scrolls away, then restored after scrolling to the end, changing threads, and sending a message, using the existing ChatViewContent test helpers and observable state.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/components/chat/MessagesTimeline.test.tsx`:
- Around line 428-442: The existing test only validates LegendList configuration
with maintainScrollAtEndOnLayout manually set to false; add a focused
interaction test covering the ChatViewContent handoff after a real scroll-away
event. Verify retention is disabled when the user scrolls away, then restored
after scrolling to the end, changing threads, and sending a message, using the
existing ChatViewContent test helpers and observable state.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: bf36d633-3918-4460-900c-04ed0059e4f0
📒 Files selected for processing (3)
apps/web/src/components/ChatView.tsxapps/web/src/components/chat/MessagesTimeline.test.tsxapps/web/src/components/chat/MessagesTimeline.tsx
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit cebb506. Configure here.
ApprovabilityVerdict: Needs human review This is a focused UI bug fix for scroll behavior preservation, well-tested with clear intent. However, the author is a first-time contributor to these scroll-related files, and the timeline scroll system has subtle interactions that warrant review by someone familiar with it. You can customize Macroscope's approvability policy. Learn more. |
cebb506 to
b2ae555
Compare
github-actions
Bot
added
size:M
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
b2ae555 to
722cfcd
Compare
722cfcd to
5c12cf9
Compare
github-actions
Bot
added
size:L
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
apps/web/src/components/chat/timelineScrollAnchoring.test.tsx (1)
7-7: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the unnecessary explicit state annotations.
TypeScript infers both state object types. Remove
TimelineLayoutFollowStatefrom the import when no annotation uses it.Proposed cleanup
- type TimelineLayoutFollowState, } from "./timelineScrollAnchoring"; - let state: TimelineLayoutFollowState = { threadKey, enabled: true }; + let state = { threadKey, enabled: true }; - const previousThreadState: TimelineLayoutFollowState = { + const previousThreadState = {As per coding guidelines,
**/*.{ts,tsx}: “Prefer inferred types over explicit annotations and do not useany.”Also applies to: 33-33, 62-65
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/web/src/components/chat/timelineScrollAnchoring.test.tsx` at line 7, Remove the unused TimelineLayoutFollowState import and delete the explicit state type annotations at the referenced state declarations, allowing TypeScript to infer both state object types while preserving their existing values and behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@apps/web/src/components/chat/timelineScrollAnchoring.test.tsx`:
- Line 7: Remove the unused TimelineLayoutFollowState import and delete the
explicit state type annotations at the referenced state declarations, allowing
TypeScript to infer both state object types while preserving their existing
values and behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 5d2682d3-4ef6-4da7-868c-9950499416e0
📒 Files selected for processing (6)
apps/web/src/components/ChatView.tsxapps/web/src/components/chat/MessagesTimeline.logic.tsapps/web/src/components/chat/MessagesTimeline.test.tsxapps/web/src/components/chat/MessagesTimeline.tsxapps/web/src/components/chat/timelineScrollAnchoring.test.tsxapps/web/src/components/chat/timelineScrollAnchoring.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/web/src/components/chat/MessagesTimeline.test.tsx
- apps/web/src/components/ChatView.tsx
5c12cf9 to
911cd65
Compare
1 participant
What Changed
Why
When manual-navigation listeners miss the timeline during mount, scrolling upward does not cancel live following. Growing the composer with Shift+Enter then updates the timeline's bottom inset and reruns end-following logic, forcing the transcript to the bottom.
The timeline now receives navigation handlers through its normal component lifecycle and only keeps layout-based end following enabled while the reader is at the live edge. Explicit scroll-to-end, thread changes, and sends restore following.
UI Changes
Before
Shift+Enter grows the composer and moves the transcript from the reader's earlier position to the live edge.
After
The composer grows while the same earlier transcript position remains in view.
Validation
vp test run apps/web/src/components/chat/MessagesTimeline.test.tsx apps/web/src/components/chat/timelineScrollAnchoring.test.tsx apps/web/src/components/ChatView.logic.test.ts(59 passed)vp lint --report-unused-disable-directives apps/web/src/components/ChatView.tsx apps/web/src/components/chat/MessagesTimeline.tsx apps/web/src/components/chat/MessagesTimeline.test.tsxvp run --filter @t3tools/web typecheckupstream/main: real Shift+Enter grew the composer from 70px to 90.96px and movedscrollTopfrom 652.08px to the 1252.08px live edge.scrollTopat 652.08px.Checklist
Built with GPT-5.6-Sol in T3 Code through the Codex harness.
Note
Low Risk
Chat timeline scroll and composer layout behavior only; changes are localized with regression tests and no auth, data, or API impact.
Overview
Fixes the transcript jumping to the live edge when the composer grows (e.g. Shift+Enter) after the user has scrolled up.
Layout follow is now gated per thread:
ChatViewtracksTimelineLayoutFollowStateand passesmaintainScrollAtEndOnLayoutintoMessagesTimeline. LegendList still maintains scroll on data/item layout changes, but layout end-following is turned off once the reader leaves the bottom; scrolling back, sending, or switching threads restores it. A new thread defaults to follow-at-end so scrolled-up state does not leak across threads.Manual navigation is wired on the timeline list instead of deferred DOM listeners in
ChatView: wheel, touchmove, and pointer-down on the scrollbar (viaisTimelineScrollbarPointerDown) callonManualNavigationand update layout-follow state. Ordinary clicks on message content no longer cancel live follow.Reviewed by Cursor Bugbot for commit 911cd65. Bugbot is set up for automated code reviews on this repo. Configure here.
Note
Preserve scroll position in chat timeline when composer grows
isTimelineScrollbarPointerDownutility in MessagesTimeline.logic.ts distinguishes scrollbar drags from content pointer interactions so only scrollbar pointerdowns count as navigation.reduceTimelineLayoutFollowState.Macroscope summarized 911cd65.
Summary by CodeRabbit
Bug Fixes
Tests