fix(tui): j/k scroll by visual row instead of logical line
When a JSON line wraps to many visual rows (e.g. 73 rows), pressing j would skip the entire logical line, making wrapped content unreadable. Now j/k scroll by 1 visual row when a VisualHeightIndex is available, with cursor tracking the viewport center. Falls back to logical-line scroll in Loading/no-index modes or when all content fits the viewport. Adds 4 tests: visual scroll down, visual scroll up, small-file fallback, j/k roundtrip.
This commit is contained in:
@@ -330,19 +330,56 @@ impl App {
|
||||
if !self.is_loaded() || self.total_lines() == 0 {
|
||||
return;
|
||||
}
|
||||
let last = self.total_lines() - 1;
|
||||
if self.cursor_line < last {
|
||||
self.cursor_line += 1;
|
||||
|
||||
// VHI present → visual-row scroll
|
||||
if self.get_visual_height_index().is_some() {
|
||||
let max_offset = self
|
||||
.total_visual_rows()
|
||||
.saturating_sub(self.content_height as usize);
|
||||
|
||||
if self.v_offset < max_offset {
|
||||
self.v_offset = self.v_offset.saturating_add(1);
|
||||
let center_visual = self
|
||||
.v_offset
|
||||
.saturating_add(self.content_height as usize / 2);
|
||||
self.cursor_line = self.visual_row_to_logical_row(center_visual);
|
||||
self.clamp_v_offset();
|
||||
} else {
|
||||
let last = self.total_lines() - 1;
|
||||
if self.cursor_line < last {
|
||||
self.cursor_line += 1;
|
||||
}
|
||||
}
|
||||
} else {
|
||||
let last = self.total_lines() - 1;
|
||||
if self.cursor_line < last {
|
||||
self.cursor_line += 1;
|
||||
}
|
||||
self.ensure_cursor_visible();
|
||||
}
|
||||
self.ensure_cursor_visible();
|
||||
}
|
||||
|
||||
pub fn scroll_up_line(&mut self) {
|
||||
if !self.is_loaded() || self.total_lines() == 0 {
|
||||
return;
|
||||
}
|
||||
self.cursor_line = self.cursor_line.saturating_sub(1);
|
||||
self.ensure_cursor_visible();
|
||||
|
||||
// VHI present → visual-row scroll
|
||||
if self.get_visual_height_index().is_some() {
|
||||
if self.v_offset > 0 {
|
||||
self.v_offset = self.v_offset.saturating_sub(1);
|
||||
let center_visual = self
|
||||
.v_offset
|
||||
.saturating_add(self.content_height as usize / 2);
|
||||
self.cursor_line = self.visual_row_to_logical_row(center_visual);
|
||||
self.clamp_v_offset();
|
||||
} else {
|
||||
self.cursor_line = self.cursor_line.saturating_sub(1);
|
||||
}
|
||||
} else {
|
||||
self.cursor_line = self.cursor_line.saturating_sub(1);
|
||||
self.ensure_cursor_visible();
|
||||
}
|
||||
}
|
||||
|
||||
pub fn scroll_down_half_page(&mut self) {
|
||||
@@ -2360,4 +2397,131 @@ plain text line
|
||||
cleanup(&path);
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
fn install_vhi(app: &mut App, heights: &[usize]) {
|
||||
let vhi = VisualHeightIndex::build(heights);
|
||||
if let AppLoadingState::Ready { reader } = &mut app.loading_state {
|
||||
if let log_viewer_core::io::progressive_reader::ReaderState::Ready {
|
||||
visual_height_index,
|
||||
..
|
||||
} = &mut reader.state
|
||||
{
|
||||
*visual_height_index = Some(vhi);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_vhi_scroll_down_line_visual_row() {
|
||||
let content = "line0\nline1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\n";
|
||||
let path = make_temp_file(content);
|
||||
let result = std::panic::catch_unwind(|| {
|
||||
let mut app = App::new();
|
||||
load_file_ready(&mut app, &path);
|
||||
app.content_height = 10;
|
||||
|
||||
// Each line wraps to 3 visual rows → 30 total, overflows viewport of 10
|
||||
install_vhi(&mut app, &[3usize; 10]);
|
||||
|
||||
assert_eq!(app.cursor_line, 0);
|
||||
assert_eq!(app.v_offset, 0);
|
||||
|
||||
for _ in 0..5 {
|
||||
app.scroll_down_line();
|
||||
}
|
||||
|
||||
assert_eq!(app.v_offset, 5, "v_offset should be 5 after 5 visual scrolls");
|
||||
// center_visual = 5 + 10/2 = 10 → maps to logical line 3 (visual rows: line0=0-2, line1=3-5, line2=6-8, line3=9-11)
|
||||
assert_eq!(app.cursor_line, 3, "cursor should track center at logical line 3");
|
||||
cleanup(&path);
|
||||
});
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_vhi_scroll_up_line_visual_row() {
|
||||
let content = "line0\nline1\nline2\nline3\nline4\nline5\nline6\nline7\nline8\nline9\n";
|
||||
let path = make_temp_file(content);
|
||||
let result = std::panic::catch_unwind(|| {
|
||||
let mut app = App::new();
|
||||
load_file_ready(&mut app, &path);
|
||||
app.content_height = 10;
|
||||
|
||||
install_vhi(&mut app, &[3usize; 10]);
|
||||
|
||||
// Start at v_offset=5, cursor_line=3 (matching center)
|
||||
app.v_offset = 5;
|
||||
app.cursor_line = 3;
|
||||
|
||||
for _ in 0..5 {
|
||||
app.scroll_up_line();
|
||||
}
|
||||
|
||||
assert_eq!(app.v_offset, 0, "v_offset should return to 0");
|
||||
// After scrolling back up, center_visual = 0 + 5 = 5 → line1
|
||||
assert!(app.cursor_line <= 2, "cursor should be near top, got {}", app.cursor_line);
|
||||
cleanup(&path);
|
||||
});
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_vhi_scroll_down_line_small_file_fallback() {
|
||||
let content = "a\nb\nc\n";
|
||||
let path = make_temp_file(content);
|
||||
let result = std::panic::catch_unwind(|| {
|
||||
let mut app = App::new();
|
||||
load_file_ready(&mut app, &path);
|
||||
app.content_height = 24;
|
||||
|
||||
// 3 lines, 1 row each → 3 visual rows, fits in viewport of 24
|
||||
install_vhi(&mut app, &[1usize; 3]);
|
||||
|
||||
assert_eq!(app.cursor_line, 0);
|
||||
assert_eq!(app.v_offset, 0);
|
||||
|
||||
app.scroll_down_line();
|
||||
|
||||
assert_eq!(app.cursor_line, 1, "cursor should move to line 1 (logical)");
|
||||
assert_eq!(app.v_offset, 0, "v_offset should stay 0 (content fits)");
|
||||
cleanup(&path);
|
||||
});
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn test_vhi_scroll_j_k_roundtrip() {
|
||||
let content: String = (0..20).map(|i| format!("line{}\n", i)).collect();
|
||||
let path = make_temp_file(&content);
|
||||
let result = std::panic::catch_unwind(|| {
|
||||
let mut app = App::new();
|
||||
load_file_ready(&mut app, &path);
|
||||
app.content_height = 10;
|
||||
|
||||
// 20 lines × 2 rows = 40 visual rows, viewport = 10
|
||||
install_vhi(&mut app, &[2usize; 20]);
|
||||
|
||||
let initial_cursor = app.cursor_line;
|
||||
let initial_offset = app.v_offset;
|
||||
|
||||
for _ in 0..15 {
|
||||
app.scroll_down_line();
|
||||
}
|
||||
assert!(app.v_offset > 0, "v_offset should have moved down");
|
||||
|
||||
for _ in 0..15 {
|
||||
app.scroll_up_line();
|
||||
}
|
||||
|
||||
assert_eq!(app.v_offset, initial_offset, "v_offset should roundtrip to {}", initial_offset);
|
||||
assert!(
|
||||
app.cursor_line <= initial_cursor + 3,
|
||||
"cursor should return near top, got {}, expected <= {}",
|
||||
app.cursor_line,
|
||||
initial_cursor + 3
|
||||
);
|
||||
cleanup(&path);
|
||||
});
|
||||
assert!(result.is_ok());
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user