Repository navigation
Conversation
Motivation: With "Collapse composer on scroll" on, a thread that reopens above its end before its last rows have rendered never rests the composer on scroll. timelineContentOverflowsViewport needs the last row's size, and LegendList only knows sizes of rows that were rendered and measured, so the overflow check reported "fits" until the last row mounted at the end. Approach: In timelineContentOverflowsViewport only, when the last row has a finite (estimated) position but no measured size, use position + 1px as a lower bound. If that is past the visible area the thread overflows; if the top is still inside the viewport it keeps reporting "fits", so short threads are unaffected. getRowBottom is unchanged, so anchored-turn metrics still return null for unmeasured rows. Validation: `vp test run --project unit src/components/chat/timelineScrollAnchoring.test.tsx` (via the local vite-plus binary): the new test fails before the change (1 failed, 9 passed) and passes after (10 passed). Lint and format checks are clean on the two changed files. I did not run the app in a browser or a full typecheck. Impact: Narrow. The composer now rests on the first scroll in this case instead of after scrolling to the very end. Nothing else changes. Report: pingdotgg#14232 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5-5 (via Claude Code)
github-actions
Bot
added
vouch:unvouched
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Approved at Macroscope's review found this PR approvable — This is a narrowly scoped bug fix that improves overflow detection only for an unmeasured final timeline row, with existing measured-row and anchored-scroll behavior preserved. Targeted tests cover both the overflow and non-overflow cases, and the PR does not change product defaults or static-analysis configuration. You can add or adjust custom eligibility rules. Learn more. |
|
Navigate logical layers of code changes, visualize relationships, and explore their blast radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository: pingdotgg/t3code/.coderabbit.yaml Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (2)
Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughThe timeline overflow check now uses the last row’s finite position plus one pixel when its size is unmeasured. Tests cover positions inside and beyond the viewport. ChangesTimeline overflow detection
Priority: ⬇️ Low Estimated code review effort: 2 (Simple) | ~8 minutes Change: Bug fix · Severity of issue fixed: Low Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change enables the composer to collapse when an unmeasured final row extends beyond the visible area, while preserving the existing behavior for invalid positions. No material merge risk is evident. Architecture SummaryArchitecture risk: 🔵 Low · up to The change affects 1 system. Changed systems: Architecture concerns Review detailsSystems and components
Before / after behavior
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
|
Note This comment is posted by Julius' dot The unit regression matches the approved overflow fix, but the PR explicitly says it was not checked in a running client and includes no visual evidence. Please add before/after screenshots and a short recording showing a thread opened above its unmeasured end now rests the composer on scroll, while a short thread still stays expanded. The verification rule requires evidence for this timing-dependent UI behavior. Closing pending that evidence; add it and request reconsideration. |
2 participants
What Changed
timelineContentOverflowsViewportnow treats a last row that has an estimated position but no measured size as a 1px lower bound.getRowBottomis unchanged.Why
When a thread opens above its end, the last row hasn't been measured, so the overflow check reported "fits" and the composer never rested on scroll until the last row rendered at the end. Following the maintainer triage, the fallback applies only inside the overflow check, and a last row whose top is still inside the viewport keeps reporting "fits".
Validation: the new unit test fails before the change and passes after (
vp test run --project unitontimelineScrollAnchoring.test.tsx); lint and format are clean. I did not check it in a running client.Fixes #14232
UI Changes
No visual change beyond the fix. No before/after screenshots or video are included.
Checklist
Summary by CodeRabbit