From fb23e4c7cbdc0717a12b0769519de22a9bbec432 Mon Sep 17 00:00:00 2001 From: dailz Date: Fri, 24 Apr 2026 19:04:30 +0800 Subject: [PATCH] fix(tui): smooth visual-row scrolling during Loading state MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit During Loading state (before VHI is built), j/k used to jump by logical line, visually skipping multiple wrapped rows. Now uses v_sub_offset to track position within a wrapped line, enabling smooth 1-visual-row scroll. - Add v_sub_offset field to App for sub-line visual position tracking - scroll_down/up_line else branch: advance v_sub_offset, wrap to next line - ensure_viewport_cache Loading path: pass v_sub_offset as offset_in_line - ensure_cursor_visible: skip during Loading (scroll functions manage it) - Reset v_sub_offset on Loading→Ready, scroll_to_top, scroll_to_bottom - Add 3 tests for Loading-state sub-offset scrolling behavior --- crates/tui/src/app.rs | 143 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 135 insertions(+), 8 deletions(-) diff --git a/crates/tui/src/app.rs b/crates/tui/src/app.rs index 60fde5d..40fe529 100644 --- a/crates/tui/src/app.rs +++ b/crates/tui/src/app.rs @@ -91,6 +91,7 @@ pub struct App { // Scroll state pub(crate) cursor_line: usize, pub(crate) v_offset: usize, + pub(crate) v_sub_offset: usize, // Viewport cache (on-demand, viewport-sized) pub(crate) viewport_cache: ViewportCache, @@ -126,6 +127,7 @@ impl App { file_path: None, cursor_line: 0, v_offset: 0, + v_sub_offset: 0, viewport_cache: ViewportCache::new(), content_width: 0, content_height: 0, @@ -180,6 +182,7 @@ impl App { self.file_path = Some(path.to_string()); self.cursor_line = 0; self.v_offset = 0; + self.v_sub_offset = 0; self.viewport_cache.invalidate(); self.last_g_press = None; // reset gg state machine self.json_format = false; @@ -259,8 +262,7 @@ impl App { // Find start logical line from v_offset let (start_logical, offset_in_line) = if self.is_loading() { - // Sampling: 1 visual row per line - (v_offset.min(self.total_lines().saturating_sub(1)), 0) + (v_offset.min(self.total_lines().saturating_sub(1)), self.v_sub_offset) } else { self.find_logical_line_at_visual_row(self.v_offset, width) }; @@ -351,11 +353,21 @@ impl App { } } } else { - let last = self.total_lines() - 1; - if self.cursor_line < last { - self.cursor_line += 1; + // Loading/no-index: visual-row scroll via v_sub_offset + let width = self.get_content_width(); + if width > 0 && self.total_lines() > 0 { + let current_height = self.compute_visual_height(self.v_offset, width); + if self.v_sub_offset + 1 < current_height { + self.v_sub_offset += 1; + } else { + let last = self.total_lines() - 1; + if self.v_offset < last { + self.v_offset += 1; + self.v_sub_offset = 0; + } + } + self.cursor_line = self.v_offset; } - self.ensure_cursor_visible(); } } @@ -377,8 +389,19 @@ impl App { self.cursor_line = self.cursor_line.saturating_sub(1); } } else { - self.cursor_line = self.cursor_line.saturating_sub(1); - self.ensure_cursor_visible(); + // Loading/no-index: visual-row scroll via v_sub_offset + if self.v_sub_offset > 0 { + self.v_sub_offset -= 1; + } else if self.v_offset > 0 { + self.v_offset -= 1; + let width = self.get_content_width(); + self.v_sub_offset = if width > 0 { + self.compute_visual_height(self.v_offset, width).saturating_sub(1) + } else { + 0 + }; + } + self.cursor_line = self.v_offset; } } @@ -440,6 +463,7 @@ impl App { } self.cursor_line = 0; self.v_offset = 0; + self.v_sub_offset = 0; } pub fn scroll_to_bottom(&mut self) { @@ -447,6 +471,7 @@ impl App { return; } self.cursor_line = self.total_lines().saturating_sub(1); + self.v_sub_offset = 0; self.ensure_cursor_visible(); self.clamp_v_offset(); } @@ -457,6 +482,9 @@ impl App { if !self.is_loaded() || self.total_lines() == 0 { return; } + if self.is_loading() { + return; + } let cursor_first = self.cursor_to_first_visual_row(self.cursor_line); let height = if let Some(index) = self.get_visual_height_index() { index.visual_height_of_line(self.cursor_line) @@ -943,6 +971,7 @@ impl App { // the index is absent, which is the case right after invalidate). self.v_offset = self.cursor_to_first_visual_row(self.cursor_line); self.clamp_v_offset(); + self.v_sub_offset = 0; // Gutter width changes (~N → N) shift content_width, so any // VisualHeightIndex built with the old width is stale. @@ -1095,6 +1124,7 @@ mod tests { let mut app = App::new(); app.load_file(path.to_str().unwrap()).unwrap(); app.cursor_line = 2; // last line + app.v_offset = 2; app.scroll_down_line(); assert_eq!(app.cursor_line, 2); // stays at last @@ -2524,4 +2554,101 @@ plain text line }); assert!(result.is_ok()); } + + fn app_in_loading_with_long_lines(app: &mut App, line_count: usize, line_width: usize) -> std::path::PathBuf { + let content: String = (0..line_count) + .map(|_| "x".repeat(line_width)) + .collect::>() + .join("\n"); + let path = make_temp_file(&content); + app.load_file(path.to_str().unwrap()).unwrap(); + path + } + + #[test] + fn test_loading_scroll_down_advances_sub_offset() { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let mut app = App::new(); + let path = app_in_loading_with_long_lines(&mut app, 10000, 200); + app.content_height = 24; + + if !app.is_loading() { + cleanup(&path); + return; + } + + assert_eq!(app.v_offset, 0); + assert_eq!(app.v_sub_offset, 0); + + app.scroll_down_line(); + assert_eq!(app.v_offset, 0, "v_offset should stay 0 after first j"); + assert_eq!(app.v_sub_offset, 1, "v_sub_offset should advance to 1"); + + app.scroll_down_line(); + assert_eq!(app.v_offset, 0, "v_offset should stay 0 after second j"); + assert_eq!(app.v_sub_offset, 2, "v_sub_offset should advance to 2"); + + app.scroll_down_line(); + assert_eq!(app.v_offset, 1, "v_offset should advance to 1"); + assert_eq!(app.v_sub_offset, 0, "v_sub_offset should reset to 0"); + + cleanup(&path); + })); + assert!(result.is_ok()); + } + + #[test] + fn test_loading_scroll_up_decrements_sub_offset() { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let mut app = App::new(); + let path = app_in_loading_with_long_lines(&mut app, 10000, 200); + app.content_height = 24; + + if !app.is_loading() { + cleanup(&path); + return; + } + + // Scroll down to v_offset=1, v_sub_offset=0 + for _ in 0..3 { app.scroll_down_line(); } + assert_eq!(app.v_offset, 1); + assert_eq!(app.v_sub_offset, 0); + + app.scroll_up_line(); + assert_eq!(app.v_offset, 0, "v_offset should go back to 0"); + assert!(app.v_sub_offset > 0, "v_sub_offset should be at end of line 0"); + + let prev_sub = app.v_sub_offset; + app.scroll_up_line(); + assert_eq!(app.v_offset, 0, "v_offset should stay 0"); + assert_eq!(app.v_sub_offset, prev_sub - 1); + + cleanup(&path); + })); + assert!(result.is_ok()); + } + + #[test] + fn test_loading_scroll_to_top_resets_sub_offset() { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| { + let mut app = App::new(); + let path = app_in_loading_with_long_lines(&mut app, 10000, 200); + app.content_height = 24; + + if !app.is_loading() { + cleanup(&path); + return; + } + + for _ in 0..5 { app.scroll_down_line(); } + assert!(app.v_offset > 0 || app.v_sub_offset > 0); + + app.scroll_to_top(); + assert_eq!(app.v_offset, 0, "v_offset should be 0 after scroll_to_top"); + assert_eq!(app.v_sub_offset, 0, "v_sub_offset should be 0 after scroll_to_top"); + + cleanup(&path); + })); + assert!(result.is_ok()); + } }