From edd6431444d9d6a2ba4d05c052a6469f4c8b04ef Mon Sep 17 00:00:00 2001 From: Edward Langley Date: Thu, 2 Apr 2026 10:21:41 -0700 Subject: [PATCH] refactor: use data_col_to_visual via group_for helpers, add column group toggle Add row_group_for/col_group_for to GridLayout, replacing inline backward-search logic. Refactor grid renderer to use col_group_for instead of pre-filtering col_items. Add gz keybinding for column group collapse toggle, symmetric with z for rows. Co-Authored-By: Claude Opus 4.6 (1M context) --- src/model/category.rs | 3 +- src/ui/app.rs | 49 ++++++++++------- src/ui/grid.rs | 42 ++++++++------- src/view/layout.rs | 121 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 174 insertions(+), 41 deletions(-) diff --git a/src/model/category.rs b/src/model/category.rs index 76403c5..bac860f 100644 --- a/src/model/category.rs +++ b/src/model/category.rs @@ -164,8 +164,7 @@ mod tests { assert_eq!(c.groups.len(), 1); } - -#[test] + #[test] fn item_index_reflects_insertion_order() { let mut c = cat(); c.add_item("East"); diff --git a/src/ui/app.rs b/src/ui/app.rs index 124556e..b5fe2ef 100644 --- a/src/ui/app.rs +++ b/src/ui/app.rs @@ -395,6 +395,10 @@ impl App { view.selected = (0, view.selected.1); view.row_offset = 0; } + // gz = toggle column group under cursor + ('g', KeyCode::Char('z')) => { + self.toggle_col_group_under_cursor(); + } // yy = yank current cell ('y', KeyCode::Char('y')) => { if let Some(key) = self.selected_cell_key() { @@ -1222,28 +1226,35 @@ impl App { fn toggle_group_under_cursor(&mut self) { let layout = GridLayout::new(&self.model, self.model.active_view()); let sel_row = self.model.active_view().selected.0; - let Some(vi) = layout.data_row_to_visual(sel_row) else { + let Some((cat, group)) = layout.row_group_for(sel_row) else { return; }; - let group = layout.row_items[..vi].iter().rev().find_map(|e| { - if let AxisEntry::GroupHeader { - cat_name, - group_name, - } = e - { - Some((cat_name.clone(), group_name.clone())) - } else { - None - } - }); - if let Some((cat, group)) = group { - let cmd = Command::ToggleGroup { - category: cat, - group, - }; - command::dispatch(&mut self.model, &cmd); - self.dirty = true; + let cmd = Command::ToggleGroup { + category: cat, + group, + }; + command::dispatch(&mut self.model, &cmd); + self.dirty = true; + } + + fn toggle_col_group_under_cursor(&mut self) { + let layout = GridLayout::new(&self.model, self.model.active_view()); + let sel_col = self.model.active_view().selected.1; + let Some((cat, group)) = layout.col_group_for(sel_col) else { + return; + }; + let cmd = Command::ToggleGroup { + category: cat, + group, + }; + command::dispatch(&mut self.model, &cmd); + // Clamp selection if col_count shrank + let new_count = GridLayout::new(&self.model, self.model.active_view()).col_count(); + let view = self.model.active_view_mut(); + if view.selected.1 >= new_count && new_count > 0 { + view.selected.1 = new_count - 1; } + self.dirty = true; } fn hide_selected_row_item(&mut self) { diff --git a/src/ui/grid.rs b/src/ui/grid.rs index 6277f50..25ee1e3 100644 --- a/src/ui/grid.rs +++ b/src/ui/grid.rs @@ -79,19 +79,10 @@ impl<'a> GridWidget<'a> { }) .collect(); - // Map each data-col index to its group name (None if ungrouped) - let col_groups: Vec> = { - let mut groups = Vec::new(); - let mut current: Option = None; - for entry in &layout.col_items { - match entry { - AxisEntry::GroupHeader { group_name, .. } => current = Some(group_name.clone()), - AxisEntry::DataItem(_) => groups.push(current.clone()), - } - } - groups - }; - let has_col_groups = col_groups.iter().any(|g| g.is_some()); + let has_col_groups = layout + .col_items + .iter() + .any(|e| matches!(e, AxisEntry::GroupHeader { .. })); let available_cols = ((area.width.saturating_sub(ROW_HEADER_WIDTH)) / COL_WIDTH) as usize; let visible_col_range = @@ -117,24 +108,35 @@ impl<'a> GridWidget<'a> { Style::default(), ); let mut x = area.x + ROW_HEADER_WIDTH; - let mut prev_group: Option<&str> = None; + let mut prev_group: Option = None; for ci in visible_col_range.clone() { if x >= area.x + area.width { break; } - let group = col_groups[ci].as_deref(); - let label = if group != prev_group { - group.unwrap_or("") + let col_group = layout.col_group_for(ci); + let group_name = col_group.as_ref().map(|(_, g)| g.clone()); + let label = if group_name != prev_group { + match &col_group { + Some((cat, g)) => { + let indicator = if view.is_group_collapsed(cat, g) { + GROUP_COLLAPSED + } else { + GROUP_EXPANDED + }; + format!("{indicator} {g}") + } + None => String::new(), + } } else { - "" + String::new() }; - prev_group = group; + prev_group = group_name; buf.set_string( x, y, format!( "{: Option<(String, String)> { + let vi = self.data_row_to_visual(data_row)?; + self.row_items[..vi].iter().rev().find_map(|e| { + if let AxisEntry::GroupHeader { + cat_name, + group_name, + } = e + { + Some((cat_name.clone(), group_name.clone())) + } else { + None + } + }) + } + + /// Find the group containing the Nth data column. + /// Returns `(cat_name, group_name)` of the nearest preceding GroupHeader. + pub fn col_group_for(&self, data_col: usize) -> Option<(String, String)> { + let vi = self.data_col_to_visual(data_col)?; + self.col_items[..vi].iter().rev().find_map(|e| { + if let AxisEntry::GroupHeader { + cat_name, + group_name, + } = e + { + Some((cat_name.clone(), group_name.clone())) + } else { + None + } + }) + } } /// Expand a single category into `AxisEntry` values, given a coordinate prefix. @@ -483,4 +517,91 @@ mod tests { assert_eq!(layout.data_row_to_visual(1), Some(3)); // Apr is at visual index 3 assert_eq!(layout.data_row_to_visual(2), None); } + + #[test] + fn data_col_to_visual_skips_headers() { + let mut m = Model::new("T"); + m.add_category("Type").unwrap(); // Row + m.add_category("Month").unwrap(); // Column + m.category_mut("Type").unwrap().add_item("Food"); + m.category_mut("Month") + .unwrap() + .add_item_in_group("Jan", "Q1"); + m.category_mut("Month") + .unwrap() + .add_item_in_group("Apr", "Q2"); + let layout = GridLayout::new(&m, m.active_view()); + // col_items: [GroupHeader(Q1), DataItem(Jan), GroupHeader(Q2), DataItem(Apr)] + assert_eq!(layout.data_col_to_visual(0), Some(1)); + assert_eq!(layout.data_col_to_visual(1), Some(3)); + assert_eq!(layout.data_col_to_visual(2), None); + } + + #[test] + fn row_group_for_finds_enclosing_group() { + let mut m = Model::new("T"); + m.add_category("Month").unwrap(); + m.add_category("Type").unwrap(); + m.category_mut("Month") + .unwrap() + .add_item_in_group("Jan", "Q1"); + m.category_mut("Month") + .unwrap() + .add_item_in_group("Apr", "Q2"); + m.category_mut("Type").unwrap().add_item("Food"); + let layout = GridLayout::new(&m, m.active_view()); + assert_eq!( + layout.row_group_for(0), + Some(("Month".to_string(), "Q1".to_string())) + ); + assert_eq!( + layout.row_group_for(1), + Some(("Month".to_string(), "Q2".to_string())) + ); + } + + #[test] + fn row_group_for_returns_none_for_ungrouped() { + let mut m = Model::new("T"); + m.add_category("Type").unwrap(); + m.add_category("Month").unwrap(); + m.category_mut("Type").unwrap().add_item("Food"); + m.category_mut("Month").unwrap().add_item("Jan"); + let layout = GridLayout::new(&m, m.active_view()); + assert_eq!(layout.row_group_for(0), None); + } + + #[test] + fn col_group_for_finds_enclosing_group() { + let mut m = Model::new("T"); + m.add_category("Type").unwrap(); // Row + m.add_category("Month").unwrap(); // Column + m.category_mut("Type").unwrap().add_item("Food"); + m.category_mut("Month") + .unwrap() + .add_item_in_group("Jan", "Q1"); + m.category_mut("Month") + .unwrap() + .add_item_in_group("Apr", "Q2"); + let layout = GridLayout::new(&m, m.active_view()); + assert_eq!( + layout.col_group_for(0), + Some(("Month".to_string(), "Q1".to_string())) + ); + assert_eq!( + layout.col_group_for(1), + Some(("Month".to_string(), "Q2".to_string())) + ); + } + + #[test] + fn col_group_for_returns_none_for_ungrouped() { + let mut m = Model::new("T"); + m.add_category("Type").unwrap(); + m.add_category("Month").unwrap(); + m.category_mut("Type").unwrap().add_item("Food"); + m.category_mut("Month").unwrap().add_item("Jan"); + let layout = GridLayout::new(&m, m.active_view()); + assert_eq!(layout.col_group_for(0), None); + } }