fix: use precise column widths for viewport scrolling
Improve grid viewport calculations by using actual column widths instead of rough estimates. This ensures that the viewport scrolls correctly when the cursor moves past the visible area. - Move column width and visible column calculations to public functions in `src/ui/grid.rs`. - Update `App::cmd_context` to use these precise calculations for `visible_cols` . - Add a regression test to verify that `col_offset` scrolls when cursor moves past visible columns. Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-31B-it-GGUF:UD-Q5_K_XL)
This commit is contained in:
132
src/ui/app.rs
132
src/ui/app.rs
@ -11,6 +11,7 @@ use crate::import::wizard::ImportWizard;
|
|||||||
use crate::model::cell::CellValue;
|
use crate::model::cell::CellValue;
|
||||||
use crate::model::Model;
|
use crate::model::Model;
|
||||||
use crate::persistence;
|
use crate::persistence;
|
||||||
|
use crate::ui::grid::{compute_col_widths, compute_row_header_width, compute_visible_cols, parse_number_format};
|
||||||
use crate::view::GridLayout;
|
use crate::view::GridLayout;
|
||||||
|
|
||||||
/// Drill-down state: frozen record snapshot + pending edits that have not
|
/// Drill-down state: frozen record snapshot + pending edits that have not
|
||||||
@ -377,6 +378,137 @@ mod tests {
|
|||||||
assert_eq!(app.buffers.get("command").map(|s| s.as_str()), Some("q"));
|
assert_eq!(app.buffers.get("command").map(|s| s.as_str()), Some("q"));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn col_offset_scrolls_when_cursor_moves_past_visible_columns() {
|
||||||
|
use crate::model::cell::{CellKey, CellValue};
|
||||||
|
// Create a model with 8 wide columns. Column item names are 30 chars
|
||||||
|
// each → column widths ~31 chars. With term_width=80, row header ~4,
|
||||||
|
// data area ~76 → only ~2 columns actually fit. But the rough estimate
|
||||||
|
// (80−30)/12 = 4 over-counts, so viewport_effects never scrolls.
|
||||||
|
let mut m = Model::new("T");
|
||||||
|
m.add_category("Row").unwrap();
|
||||||
|
m.add_category("Col").unwrap();
|
||||||
|
m.category_mut("Row").unwrap().add_item("R1");
|
||||||
|
for i in 0..8 {
|
||||||
|
let name = format!("VeryLongColumnItemName_{i:03}");
|
||||||
|
m.category_mut("Col").unwrap().add_item(&name);
|
||||||
|
}
|
||||||
|
// Populate a value so the model isn't empty
|
||||||
|
let key = CellKey::new(vec![
|
||||||
|
("Row".to_string(), "R1".to_string()),
|
||||||
|
("Col".to_string(), "VeryLongColumnItemName_000".to_string()),
|
||||||
|
]);
|
||||||
|
m.set_cell(key, CellValue::Number(1.0));
|
||||||
|
|
||||||
|
let mut app = App::new(m, None);
|
||||||
|
app.term_width = 80;
|
||||||
|
|
||||||
|
// Press 'l' (right) 3 times to move cursor to column 3.
|
||||||
|
// Only ~2 columns fit in 76 chars of data area (each col ~26 chars wide),
|
||||||
|
// so column 3 is well off-screen. The buggy estimate (80−30)/12 = 4
|
||||||
|
// thinks 4 columns fit, so it won't scroll until col 4.
|
||||||
|
for _ in 0..3 {
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::Char('l'), KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
}
|
||||||
|
|
||||||
|
assert_eq!(
|
||||||
|
app.model.active_view().selected.1,
|
||||||
|
3,
|
||||||
|
"cursor should be at column 3"
|
||||||
|
);
|
||||||
|
assert!(
|
||||||
|
app.model.active_view().col_offset > 0,
|
||||||
|
"col_offset should scroll when cursor moves past visible area (only ~2 cols fit \
|
||||||
|
in 80-char terminal with 26-char-wide columns), but col_offset is {}",
|
||||||
|
app.model.active_view().col_offset
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn home_jumps_to_first_col() {
|
||||||
|
let mut app = two_col_model();
|
||||||
|
app.model.active_view_mut().selected = (1, 1);
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::Home, KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(app.model.active_view().selected, (1, 0));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn end_jumps_to_last_col() {
|
||||||
|
let mut app = two_col_model();
|
||||||
|
app.model.active_view_mut().selected = (1, 0);
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::End, KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(app.model.active_view().selected, (1, 1));
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn page_down_scrolls_by_three_quarters_visible() {
|
||||||
|
let mut app = two_col_model();
|
||||||
|
// Add enough rows
|
||||||
|
for i in 0..30 {
|
||||||
|
app.model
|
||||||
|
.category_mut("Row")
|
||||||
|
.unwrap()
|
||||||
|
.add_item(&format!("R{i}"));
|
||||||
|
}
|
||||||
|
app.term_height = 28; // ~20 visible rows → delta = 15
|
||||||
|
app.model.active_view_mut().selected = (0, 0);
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::PageDown, KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(app.model.active_view().selected.1, 0, "column preserved");
|
||||||
|
assert!(
|
||||||
|
app.model.active_view().selected.0 > 0,
|
||||||
|
"row should advance on PageDown"
|
||||||
|
);
|
||||||
|
// 3/4 of ~20 = 15
|
||||||
|
assert_eq!(app.model.active_view().selected.0, 15);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn page_up_scrolls_backward() {
|
||||||
|
let mut app = two_col_model();
|
||||||
|
for i in 0..30 {
|
||||||
|
app.model
|
||||||
|
.category_mut("Row")
|
||||||
|
.unwrap()
|
||||||
|
.add_item(&format!("R{i}"));
|
||||||
|
}
|
||||||
|
app.term_height = 28;
|
||||||
|
app.model.active_view_mut().selected = (20, 0);
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::PageUp, KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
assert_eq!(app.model.active_view().selected.0, 5);
|
||||||
|
}
|
||||||
|
|
||||||
|
#[test]
|
||||||
|
fn tab_in_edit_mode_commits_and_moves_right() {
|
||||||
|
let mut app = two_col_model();
|
||||||
|
app.model.active_view_mut().selected = (0, 0);
|
||||||
|
// Enter edit mode
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::Char('i'), KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
assert!(matches!(app.mode, AppMode::Editing { .. }));
|
||||||
|
// Type a digit
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::Char('5'), KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
// Press Tab — should commit, move right, re-enter edit mode
|
||||||
|
app.handle_key(KeyEvent::new(KeyCode::Tab, KeyModifiers::NONE))
|
||||||
|
.unwrap();
|
||||||
|
// Should be in edit mode on column 1
|
||||||
|
assert!(
|
||||||
|
matches!(app.mode, AppMode::Editing { .. }),
|
||||||
|
"should be in edit mode after Tab, but mode is {:?}",
|
||||||
|
app.mode
|
||||||
|
);
|
||||||
|
assert_eq!(
|
||||||
|
app.model.active_view().selected.1,
|
||||||
|
1,
|
||||||
|
"should have moved to column 1"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn command_mode_buffer_cleared_on_reentry() {
|
fn command_mode_buffer_cleared_on_reentry() {
|
||||||
use crossterm::event::KeyEvent;
|
use crossterm::event::KeyEvent;
|
||||||
|
|||||||
161
src/ui/grid.rs
161
src/ui/grid.rs
@ -71,56 +71,23 @@ impl<'a> GridWidget<'a> {
|
|||||||
let n_col_levels = layout.col_cats.len().max(1);
|
let n_col_levels = layout.col_cats.len().max(1);
|
||||||
let n_row_levels = layout.row_cats.len().max(1);
|
let n_row_levels = layout.row_cats.len().max(1);
|
||||||
|
|
||||||
// ── Adaptive column widths ────────────────────────────────────
|
let mut col_widths = compute_col_widths(self.model, &layout, fmt_comma, fmt_decimals);
|
||||||
// Size each column to fit its widest content (header + cell values)
|
// Records mode: also measure cell text widths (needs drill_state)
|
||||||
// plus 1 char gap. Minimum MIN_COL_WIDTH, capped at MAX_COL_WIDTH.
|
if layout.is_records_mode() {
|
||||||
let col_widths: Vec<u16> = {
|
|
||||||
let n = layout.col_count();
|
let n = layout.col_count();
|
||||||
let mut widths = vec![0u16; n];
|
for ri in 0..layout.row_count() {
|
||||||
// Measure column header labels
|
for (ci, wref) in col_widths.iter_mut().enumerate().take(n) {
|
||||||
for ci in 0..n {
|
let s = self.records_cell_text(&layout, ri, ci);
|
||||||
let header = layout.col_label(ci);
|
let w = s.width() as u16;
|
||||||
let w = header.width() as u16;
|
let needed = (w + 1).max(MIN_COL_WIDTH).min(MAX_COL_WIDTH);
|
||||||
if w > widths[ci] {
|
if needed > *wref {
|
||||||
widths[ci] = w;
|
*wref = needed;
|
||||||
}
|
|
||||||
}
|
|
||||||
// Measure cell content
|
|
||||||
if layout.is_records_mode() {
|
|
||||||
for ri in 0..layout.row_count() {
|
|
||||||
for (ci, wref) in widths.iter_mut().enumerate().take(n) {
|
|
||||||
let s = self.records_cell_text(&layout, ri, ci);
|
|
||||||
let w = s.width() as u16;
|
|
||||||
if w > *wref {
|
|
||||||
*wref = w;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
} else {
|
|
||||||
// Pivot mode: measure formatted cell values
|
|
||||||
for ri in 0..layout.row_count() {
|
|
||||||
for (ci, wref) in widths.iter_mut().enumerate().take(n) {
|
|
||||||
if let Some(key) = layout.cell_key(ri, ci) {
|
|
||||||
let value =
|
|
||||||
self.model.evaluate_aggregated(&key, &layout.none_cats);
|
|
||||||
let s = format_value(value.as_ref(), fmt_comma, fmt_decimals);
|
|
||||||
let w = s.width() as u16;
|
|
||||||
if w > *wref {
|
|
||||||
*wref = w;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
// +1 for gap between columns
|
}
|
||||||
widths
|
|
||||||
.into_iter()
|
|
||||||
.map(|w| (w + 1).max(MIN_COL_WIDTH).min(MAX_COL_WIDTH))
|
|
||||||
.collect()
|
|
||||||
};
|
|
||||||
|
|
||||||
// ── Adaptive row header widths ───────────────────────────────
|
// ── Adaptive row header widths ───────────────────────────────
|
||||||
// Measure the widest label at each row-header level.
|
|
||||||
let data_row_items: Vec<&Vec<String>> = layout
|
let data_row_items: Vec<&Vec<String>> = layout
|
||||||
.row_items
|
.row_items
|
||||||
.iter()
|
.iter()
|
||||||
@ -588,6 +555,112 @@ impl<'a> Widget for GridWidget<'a> {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// Compute adaptive column widths for pivot mode (header labels + cell values).
|
||||||
|
/// Header widths use the widest *individual* level label (not the joined
|
||||||
|
/// multi-level string), matching how the grid renderer draws each level on
|
||||||
|
/// its own row with repeat-suppression.
|
||||||
|
pub fn compute_col_widths(model: &Model, layout: &GridLayout, fmt_comma: bool, fmt_decimals: u8) -> Vec<u16> {
|
||||||
|
let n = layout.col_count();
|
||||||
|
let mut widths = vec![0u16; n];
|
||||||
|
// Measure individual header level labels
|
||||||
|
let data_col_items: Vec<&Vec<String>> = layout
|
||||||
|
.col_items
|
||||||
|
.iter()
|
||||||
|
.filter_map(|e| {
|
||||||
|
if let AxisEntry::DataItem(v) = e {
|
||||||
|
Some(v)
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
for (ci, wref) in widths.iter_mut().enumerate().take(n) {
|
||||||
|
if let Some(levels) = data_col_items.get(ci) {
|
||||||
|
let max_level_w = levels.iter().map(|s| s.width() as u16).max().unwrap_or(0);
|
||||||
|
if max_level_w > *wref {
|
||||||
|
*wref = max_level_w;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
if !layout.is_records_mode() {
|
||||||
|
for ri in 0..layout.row_count() {
|
||||||
|
for (ci, wref) in widths.iter_mut().enumerate().take(n) {
|
||||||
|
if let Some(key) = layout.cell_key(ri, ci) {
|
||||||
|
let value = model.evaluate_aggregated(&key, &layout.none_cats);
|
||||||
|
let s = format_value(value.as_ref(), fmt_comma, fmt_decimals);
|
||||||
|
let w = s.width() as u16;
|
||||||
|
if w > *wref {
|
||||||
|
*wref = w;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Measure total row (column sums) — totals can be wider than any single cell
|
||||||
|
if layout.row_count() > 0 {
|
||||||
|
for (ci, wref) in widths.iter_mut().enumerate().take(n) {
|
||||||
|
let total: f64 = (0..layout.row_count())
|
||||||
|
.filter_map(|ri| layout.cell_key(ri, ci))
|
||||||
|
.map(|key| model.evaluate_aggregated_f64(&key, &layout.none_cats))
|
||||||
|
.sum();
|
||||||
|
let s = format_f64(total, fmt_comma, fmt_decimals);
|
||||||
|
let w = s.width() as u16;
|
||||||
|
if w > *wref {
|
||||||
|
*wref = w;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
widths
|
||||||
|
.into_iter()
|
||||||
|
.map(|w| (w + 1).max(MIN_COL_WIDTH).min(MAX_COL_WIDTH))
|
||||||
|
.collect()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Compute the total row header width from the layout's row items.
|
||||||
|
pub fn compute_row_header_width(layout: &GridLayout) -> u16 {
|
||||||
|
let n_row_levels = layout.row_cats.len().max(1);
|
||||||
|
let data_row_items: Vec<&Vec<String>> = layout
|
||||||
|
.row_items
|
||||||
|
.iter()
|
||||||
|
.filter_map(|e| {
|
||||||
|
if let AxisEntry::DataItem(v) = e {
|
||||||
|
Some(v)
|
||||||
|
} else {
|
||||||
|
None
|
||||||
|
}
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
let sub_widths: Vec<u16> = (0..n_row_levels)
|
||||||
|
.map(|d| {
|
||||||
|
let max_label = data_row_items
|
||||||
|
.iter()
|
||||||
|
.filter_map(|v| v.get(d))
|
||||||
|
.map(|s| s.width() as u16)
|
||||||
|
.max()
|
||||||
|
.unwrap_or(0);
|
||||||
|
(max_label + 1).max(MIN_ROW_HEADER_W).min(MAX_ROW_HEADER_W)
|
||||||
|
})
|
||||||
|
.collect();
|
||||||
|
sub_widths.iter().sum()
|
||||||
|
}
|
||||||
|
|
||||||
|
/// Count how many columns fit starting from `col_offset` given the available width.
|
||||||
|
pub fn compute_visible_cols(col_widths: &[u16], row_header_width: u16, term_width: u16, col_offset: usize) -> usize {
|
||||||
|
// Account for grid border (2 chars)
|
||||||
|
let data_area_width = term_width.saturating_sub(2).saturating_sub(row_header_width);
|
||||||
|
let mut acc = 0u16;
|
||||||
|
let mut count = 0usize;
|
||||||
|
for ci in col_offset..col_widths.len() {
|
||||||
|
let w = col_widths[ci];
|
||||||
|
if acc + w > data_area_width {
|
||||||
|
break;
|
||||||
|
}
|
||||||
|
acc += w;
|
||||||
|
count += 1;
|
||||||
|
}
|
||||||
|
count.max(1)
|
||||||
|
}
|
||||||
|
|
||||||
fn format_value(v: Option<&CellValue>, comma: bool, decimals: u8) -> String {
|
fn format_value(v: Option<&CellValue>, comma: bool, decimals: u8) -> String {
|
||||||
match v {
|
match v {
|
||||||
Some(CellValue::Number(n)) => format_f64(*n, comma, decimals),
|
Some(CellValue::Number(n)) => format_f64(*n, comma, decimals),
|
||||||
|
|||||||
Reference in New Issue
Block a user