Repository navigation
fix(tui): keep viewport steady when revealing older transcript rows - #54158
opencode-agent[bot] wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
The flash is fixed. The new test fails on v2 with a one-frame jump and passes here. bun test test/session-*.test.tsx (36 pass) and typecheck in packages/tui pass too.
Public API is unchanged.
One regression is worth fixing before merge: the extra holdAnchor() in the afterLayout callback throws away wheel input that arrives during the two-frame window and snaps the viewport back. Details and a measured repro are inline.
Related: #45976
| setHiddenRows(rowsBefore(weights(), current, TRANSCRIPT_BACKFILL_CHUNK)) | ||
| afterLayout(() => { | ||
| scroll.scrollBy(scroll.scrollHeight - before + scrollBy) | ||
| if (hold && held === hold) { |
There was a problem hiding this comment.
Calling holdAnchor() again here sets an absolute scrollTo, which drops any wheel scrolling the reader did in the two frames since the content resize. The resize handler had already corrected scrollTop. Repro: I scrolled until a chunk mounted, then sent 4 more wheel-ups before this callback ran. On v2, scrollTop goes 6 → 246 (wheel input kept). On this branch it goes 261 → 249 → 261: the reader's 12 rows are lost and the view jumps back down.
Just releasing the hold (if (hold && held === hold) held = undefined) keeps the wheel input (ends at 249), and your new test still passes.
|
Automated PR Cleanup Thank you for contributing to opencode. Due to the high volume of PRs from users and AI agents, we periodically close older PRs using automated criteria so maintainers can focus review time on the most active and community-supported contributions. This PR was closed because it matched the following cleanup criteria:
PRs created within the last month are not affected by this cleanup. If you believe this PR was closed incorrectly, or if you are still actively working on it, please leave a comment explaining why it should be reopened. A maintainer can review and reopen it if appropriate. Thanks again for taking the time to contribute. |
Issue for this PR
No separate issue. Reported in Slack with a screen recording.
Type of change
What does this PR do?
Scrolling up through a long session sometimes flashes a different part of the transcript for one frame. In the report, it showed the session's first long prompt, so the left accent bar seemed to flicker.
The transcript mounts only its newest rows. When the reader scrolls near the top of the mounted range,
revealOlderRowsmounts an older chunk above it. It then callsscrollBy(heightDelta)insideafterLayout, which runs two animation frames later. Until then, the oldscrollTopis drawn over the taller content, so the viewport briefly shows the top of the new chunk.The fix pins the current first mounted row through the existing
held/holdAnchormechanism already used for group toggles. The content resize fires during layout, soholdAnchorcorrectsscrollTopin the same frame. TheafterLayoutcallback reapplies the anchor once, releases the hold, and then applies the pending wheel delta as before.The history-page path (
createHistoryPrependinhistory.ts) uses the same two-frame correction. That path waits on a network request, so an anchor held for the whole request could conflict with the reader's own scrolling. This PR leaves it unchanged.How did you verify your code works?
test/session-reveal-older-rows.test.tsx. It records every rendered frame while scrolling up through 200 messages and fails if the top visible message jumps by more than two between frames. It fails onv2with a one-frame jump to a distant message and back, and passes with this change.bun test test/session-*.test.tsx: 36 pass.bun run typecheckinpackages/tui.v2and this branch, and replayed both recordings every 2 ms. Onv2, the top prompt went 109 → 89 → 109 for about 14 ms when the older chunk mounted. On this branch, the top prompt decreased steadily over the same range (118 → 97) with no jumps.Screenshots / recordings
The side-by-side before/after recording (real time plus 15x slow motion) is in the Slack thread. I could not attach it here.
Checklist
Requested by: @nexxeln (nexxel via Slack)
Added by the opencode bot (AI-generated).
Related: #45976