fix: preserve scroll position across terminal resize#1724
Conversation
Previously handle_resize unconditionally forced TranscriptScroll::to_bottom() whenever the user was scrolled up, causing viewport drift on every resize. Removed the force-to-bottom so the existing resolve_top() clamp handles the new geometry naturally on the next render. Also seed last_transcript_visible from the resize event's height so PageUp/ PageDown use a sensible page size immediately after resize instead of the 1-line max(1) fallback.
There was a problem hiding this comment.
Code Review
This pull request updates the handle_resize logic in crates/tui/src/tui/app.rs to seed the visible transcript height from the resize event, allowing paging keys to work correctly before the next render. It also removes the code that forced scrolling to the bottom on resize. A review comment correctly identifies that using the _height variable will trigger a compiler warning due to its underscore prefix and suggests renaming the parameter to resolve this.
| // use a sensible page size immediately, before the next render updates | ||
| // it. Subtract 2 to roughly account for borders/status bar so the | ||
| // fallback is conservative rather than over-shooting. | ||
| self.viewport.last_transcript_visible = (_height as usize).saturating_sub(2).max(1); |
There was a problem hiding this comment.
You're using _height here. Since _height is prefixed with an underscore in the function signature on line 2441, this will likely cause a compiler warning (unused_variables).
To fix this, you should remove the underscore from the parameter name in the function signature, and then use height here.
Example change in handle_resize:
pub fn handle_resize(&mut self, _width: u16, height: u16) {
// ...
self.viewport.last_transcript_visible = (height as usize).saturating_sub(2).max(1);
// ...
}Bump workspace, inter-crate, and npm package versions 0.8.38 -> 0.8.39. Roll CHANGELOG [Unreleased] into [0.8.39] with all fixes: - Revert v0.8.38 /model picker rework (back to instant curated picker) - Restore approval grouping (lossy v0.8.37 logic for approvals, exact key for denials) - Thinking-only turn surface fix (#1727) - ACP server JSON-RPC id stringification (#1696) - Chat client: reasoning_content for generic providers (#1673) - Compaction: user text query preservation (#1704) - Engine: system prompt override survival (#1688) - Pager: G/End overshoot fix (#1706), mouse scroll (#1716) - Composer: scroll with text (#1677), multiline arrows (#1721) - macOS system theme detection (#1670) - rlm_open blank source fields (#1712) - Terminal resize paging fix (#1724) - Docker first-run permission (#1684) - README Rust 1.88+ requirement note (#1718) Tests: 3149 passed, 0 failed (deepseek-tui crate) clippy: clean on --all-targets --all-features
|
Thanks for isolating the resize paging issue. The fix was harvested into v0.8.39 via #1734 and credited in the changelog, so I am closing this PR to keep the queue clean. Please reopen with terminal dimensions and key sequence if latest still falls back to one-line PageUp/PageDown after resize. |
Problem
Terminal resize (even slight window-manager adjustments) was destroying the user's scroll position in the transcript. The
handle_resizemethod unconditionally forcedTranscriptScroll::to_bottom()whenever the user had scrolled up, causing:last_transcript_visiblewas reset to0Fix
Two changes in
App::handle_resize(crates/tui/src/tui/app.rs):Removed the force-to-bottom block. Scroll position is now preserved across resize. On the next render,
resolve_top()naturally clamps the offset to the new valid range — collapsing to tail only when geometrically necessary, not unconditionally.Seeded
last_transcript_visiblefrom the resize height. Previously reset to0, which made the first PageUp/Down after resize fall back to a 1-line default. Now uses(_height as usize).saturating_sub(2).max(1)so paging keys work correctly immediately after resize.Existing infrastructure (unchanged)
The full scroll infrastructure was already in place:
TranscriptScrollwith flat line-offset model andTAIL_SENTINELViewportStatewithpending_scroll_deltalazy-resolutionTranscriptScrollbarvisual indicatorThe resize handler was the single point sabotaging all of it.
Testing