fiddlerwoaroof/improvise
1
0
Fork 0
Code Issues Pull Requests Actions 2 Packages Projects Releases Wiki Activity

refactor(ui): integrate centralized layout and display logic

Browse Source 
Update UI components and view layout to use the new centralized layout and
display logic.

- Update `CategoryPanel` to remove redundant title text.
- Update `ViewPanel` to remove redundant title text.
- Refactor `Effect` implementations to use `display_value` and `Rc` for
  records.
- Update `GridWidget` to use the centralized `layout` and `display_text` .
- Refactor `GridLayout` to support synthetic keys for records mode and
  unified display.
- Update `view` module to re-export `synthetic_record_info` .

Co-Authored-By: fiddlerwoaroof/git-smart-commit (unsloth/gemma-4-26B-A4B-it-GGUF:UD-Q5_K_XL)
This commit is contained in:
Edward Langley
2026-04-07 09:29:45 -07:00
parent d32a6140b8
commit 42d869e4c2
6 changed files with 237 additions and 173 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
src/ui/category_panel.rs
View File

@ -49,7 +49,7 @@ impl<'a> Widget for CategoryPanel<'a> {
let is_active = matches!(self.mode, AppMode::CategoryPanel) || is_item_add || is_cat_add;
let (border_color, title) = if is_active {
(Color::Cyan, " Categories n:new d:del Space:axis ")
(Color::Cyan, " Categories ")
} else {
(Color::DarkGray, " Categories ")
};

22
src/ui/effect.rs
View File

@ -97,15 +97,7 @@ pub struct EnterEditAtCursor;
impl Effect for EnterEditAtCursor {
fn apply(&self, app: &mut App) {
let ctx = app.cmd_context(crossterm::event::KeyCode::Null, crossterm::event::KeyModifiers::NONE);
let value = if let Some(v) = &ctx.records_value {
v.clone()
} else {
ctx.cell_key
.as_ref()
.and_then(|k| ctx.model.get_cell(k).cloned())
.map(|v| v.to_string())
.unwrap_or_default()
};
let value = ctx.display_value.clone();
drop(ctx);
app.buffers.insert("edit".to_string(), value);
app.mode = AppMode::Editing {
@ -406,7 +398,7 @@ pub struct StartDrill(pub Vec<(CellKey, CellValue)>);
impl Effect for StartDrill {
fn apply(&self, app: &mut App) {
app.drill_state = Some(super::app::DrillState {
records: self.0.clone(),
records: std::rc::Rc::new(self.0.clone()),
pending_edits: std::collections::HashMap::new(),
});
}
@ -838,6 +830,16 @@ pub enum Panel {
View,
}
impl Panel {
pub fn mode(self) -> AppMode {
match self {
Panel::Formula => AppMode::FormulaPanel,
Panel::Category => AppMode::CategoryPanel,
Panel::View => AppMode::ViewPanel,
}
}
}
impl Effect for SetPanelOpen {
fn apply(&self, app: &mut App) {
match self.panel {

240
src/ui/grid.rs
View File

@ -6,7 +6,6 @@ use ratatui::{
};
use unicode_width::UnicodeWidthStr;
use crate::model::cell::CellValue;
use crate::model::Model;
use crate::ui::app::AppMode;
use crate::view::{AxisEntry, GridLayout};
@ -23,6 +22,7 @@ const GROUP_COLLAPSED: &str = "▶";
pub struct GridWidget<'a> {
pub model: &'a Model,
pub layout: &'a GridLayout,
pub mode: &'a AppMode,
pub search_query: &'a str,
pub buffers: &'a std::collections::HashMap<String, String>,
@ -32,6 +32,7 @@ pub struct GridWidget<'a> {
impl<'a> GridWidget<'a> {
pub fn new(
model: &'a Model,
layout: &'a GridLayout,
mode: &'a AppMode,
search_query: &'a str,
buffers: &'a std::collections::HashMap<String, String>,
@ -39,6 +40,7 @@ impl<'a> GridWidget<'a> {
) -> Self {
Self {
model,
layout,
mode,
search_query,
buffers,
@ -46,23 +48,9 @@ impl<'a> GridWidget<'a> {
}
}
/// In records mode, get the display text for (row, col): pending edit if
/// staged, otherwise the underlying record's value for that column.
fn records_cell_text(&self, layout: &GridLayout, row: usize, col: usize) -> String {
let col_name = layout.col_label(col);
let pending = self
.drill_state
.and_then(|s| s.pending_edits.get(&(row, col_name.clone())).cloned());
pending
.or_else(|| layout.records_display(row, col))
.unwrap_or_default()
}
fn render_grid(&self, area: Rect, buf: &mut Buffer) {
let view = self.model.active_view();
let frozen = self.drill_state.map(|s| s.records.clone());
let layout = GridLayout::with_frozen_records(self.model, view, frozen);
let layout = self.layout;
let (sel_row, sel_col) = view.selected;
let row_offset = view.row_offset;
let col_offset = view.col_offset;
@ -71,56 +59,9 @@ impl<'a> GridWidget<'a> {
let n_col_levels = layout.col_cats.len().max(1);
let n_row_levels = layout.row_cats.len().max(1);
// ── Adaptive column widths ────────────────────────────────────
// Size each column to fit its widest content (header + cell values)
// plus 1 char gap. Minimum MIN_COL_WIDTH, capped at MAX_COL_WIDTH.
let col_widths: Vec<u16> = {
let n = layout.col_count();
let mut widths = vec![0u16; n];
// Measure column header labels
for ci in 0..n {
let header = layout.col_label(ci);
let w = header.width() as u16;
if w > widths[ci] {
widths[ci] = w;
}
}
// 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()
};
let col_widths = compute_col_widths(self.model, &layout, fmt_comma, fmt_decimals);
// ── Adaptive row header widths ───────────────────────────────
// Measure the widest label at each row-header level.
let data_row_items: Vec<&Vec<String>> = layout
.row_items
.iter()
@ -410,23 +351,15 @@ impl<'a> GridWidget<'a> {
}
let cw = col_w_at(ci) as usize;
let (cell_str, value) = if layout.is_records_mode() {
let s = self.records_cell_text(&layout, ri, ci);
// In records mode the value is a string, not aggregated
let v = if !s.is_empty() {
Some(crate::model::cell::CellValue::Text(s.clone()))
} else {
None
};
(s, v)
// Check pending drill edits first, then use display_text
let cell_str = if let Some(ds) = self.drill_state {
let col_name = layout.col_label(ci);
ds.pending_edits
.get(&(ri, col_name))
.cloned()
.unwrap_or_else(|| layout.display_text(self.model, ri, ci, fmt_comma, fmt_decimals))
} else {
let key = match layout.cell_key(ri, ci) {
Some(k) => k,
None => continue,
};
let value = self.model.evaluate_aggregated(&key, &layout.none_cats);
let s = format_value(value.as_ref(), fmt_comma, fmt_decimals);
(s, value)
layout.display_text(self.model, ri, ci, fmt_comma, fmt_decimals)
};
let is_selected = ri == sel_row && ci == sel_col;
let is_search_match = !self.search_query.is_empty()
@ -453,13 +386,13 @@ impl<'a> GridWidget<'a> {
} else if is_search_match {
Style::default().fg(Color::Black).bg(Color::Yellow)
} else if is_sel_row {
let fg = if value.is_none() {
let fg = if cell_str.is_empty() {
Color::DarkGray
} else {
Color::White
};
Style::default().fg(fg).bg(ROW_HIGHLIGHT_BG)
} else if value.is_none() {
} else if cell_str.is_empty() {
Style::default().fg(Color::DarkGray)
} else {
Style::default()
@ -588,53 +521,111 @@ impl<'a> Widget for GridWidget<'a> {
}
}
fn format_value(v: Option<&CellValue>, comma: bool, decimals: u8) -> String {
match v {
Some(CellValue::Number(n)) => format_f64(*n, comma, decimals),
Some(CellValue::Text(s)) => s.clone(),
None => String::new(),
}
}
pub fn parse_number_format(fmt: &str) -> (bool, u8) {
let comma = fmt.contains(',');
let decimals = fmt
.rfind('.')
.and_then(|i| fmt[i + 1..].parse::<u8>().ok())
.unwrap_or(0);
(comma, decimals)
}
pub fn format_f64(n: f64, comma: bool, decimals: u8) -> String {
let formatted = format!("{:.prec$}", n, prec = decimals as usize);
if !comma {
return formatted;
}
// Split integer and decimal parts
let (int_part, dec_part) = if let Some(dot) = formatted.find('.') {
(&formatted[..dot], Some(&formatted[dot..]))
} else {
(&formatted[..], None)
};
let is_neg = int_part.starts_with('-');
let digits = if is_neg { &int_part[1..] } else { int_part };
let mut result = String::new();
for (idx, c) in digits.chars().rev().enumerate() {
if idx > 0 && idx % 3 == 0 {
result.push(',');
/// 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;
}
}
result.push(c);
}
if is_neg {
result.push('-');
// Measure cell content widths (works for both pivot and records modes)
for ri in 0..layout.row_count() {
for (ci, wref) in widths.iter_mut().enumerate().take(n) {
let s = layout.display_text(model, ri, ci, fmt_comma, fmt_decimals);
let w = s.width() as u16;
if w > *wref {
*wref = w;
}
}
}
let mut out: String = result.chars().rev().collect();
if let Some(dec) = dec_part {
out.push_str(dec);
// Measure total row (column sums) — pivot mode only
if !layout.is_records_mode() && 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;
}
}
}
out
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)
}
// Re-export shared formatting functions
pub use crate::format::{format_f64, parse_number_format};
fn truncate(s: &str, max_width: usize) -> String {
let w = s.width();
if w <= max_width {
@ -674,7 +665,8 @@ mod tests {
let area = Rect::new(0, 0, width, height);
let mut buf = Buffer::empty(area);
let bufs = std::collections::HashMap::new();
GridWidget::new(model, &AppMode::Normal, "", &bufs, None).render(area, &mut buf);
let layout = GridLayout::new(model, model.active_view());
GridWidget::new(model, &layout, &AppMode::Normal, "", &bufs, None).render(area, &mut buf);
buf
}

2
src/ui/view_panel.rs
View File

@ -36,7 +36,7 @@ impl<'a> Widget for ViewPanel<'a> {
let block = Block::default()
.borders(Borders::ALL)
.border_style(border_style)
.title(" Views [Enter] switch [n]ew [d]elete ");
.title(" Views ");
let inner = block.inner(area);
block.render(area, buf);

142
src/view/layout.rs
View File

@ -1,7 +1,17 @@
use std::rc::Rc;
use crate::model::cell::{CellKey, CellValue};
use crate::model::Model;
use crate::view::{Axis, View};
/// Extract (record_index, dim_name) from a synthetic records-mode CellKey.
/// Returns None for normal pivot-mode keys.
pub fn synthetic_record_info(key: &CellKey) -> Option<(usize, String)> {
let idx: usize = key.get("_Index")?.parse().ok()?;
let dim = key.get("_Dim")?.to_string();
Some((idx, dim))
}
/// One entry on a grid axis: either a visual group header or a data-item tuple.
///
/// `GroupHeader` entries are always visible so the user can see the group label
@ -30,8 +40,8 @@ pub struct GridLayout {
/// Categories on `Axis::None` — hidden, implicitly aggregated.
pub none_cats: Vec<String>,
/// In records mode: the filtered cell list, one per row.
/// None for normal pivot views.
pub records: Option<Vec<(CellKey, CellValue)>>,
/// None for normal pivot views. Rc for cheap sharing.
pub records: Option<Rc<Vec<(CellKey, CellValue)>>>,
}
impl GridLayout {
@ -40,12 +50,11 @@ impl GridLayout {
pub fn with_frozen_records(
model: &Model,
view: &View,
frozen_records: Option<Vec<(CellKey, CellValue)>>,
frozen_records: Option<Rc<Vec<(CellKey, CellValue)>>>,
) -> Self {
let mut layout = Self::new(model, view);
if layout.is_records_mode() {
if let Some(records) = frozen_records {
// Re-build with the frozen records instead
let row_items: Vec<AxisEntry> = (0..records.len())
.map(|i| AxisEntry::DataItem(vec![i.to_string()]))
.collect();
@ -175,7 +184,7 @@ impl GridLayout {
row_items,
col_items,
none_cats,
records: Some(records),
records: Some(Rc::new(records)),
}
}
@ -220,14 +229,10 @@ impl GridLayout {
let mut has_value = vec![vec![false; cc]; rc];
for ri in 0..rc {
for ci in 0..cc {
has_value[ri][ci] = if self.is_records_mode() {
let s = self.records_display(ri, ci).unwrap_or_default();
!s.is_empty()
} else {
self.cell_key(ri, ci)
.and_then(|k| model.evaluate_aggregated(&k, &self.none_cats))
.is_some()
};
has_value[ri][ci] = self
.cell_key(ri, ci)
.and_then(|k| model.evaluate_aggregated(&k, &self.none_cats))
.is_some();
}
}
@ -297,7 +302,7 @@ impl GridLayout {
.map(|i| AxisEntry::DataItem(vec![i.to_string()]))
.collect();
self.row_items = new_row_items;
self.records = Some(new_records);
self.records = Some(Rc::new(new_records));
}
}
@ -352,18 +357,57 @@ impl GridLayout {
.unwrap_or_default()
}
/// Resolve the display string for a synthetic records-mode CellKey.
/// Returns None for non-synthetic (pivot) keys.
pub fn resolve_display(&self, key: &CellKey) -> Option<String> {
let (idx, dim) = synthetic_record_info(key)?;
let records = self.records.as_ref()?;
let (orig_key, value) = records.get(idx)?;
if dim == "Value" {
Some(value.to_string())
} else {
Some(orig_key.get(&dim).unwrap_or("").to_string())
}
}
/// Unified display text for a cell at (row, col). Handles both pivot and
/// records modes. In pivot mode, evaluates and formats the cell value.
/// In records mode, resolves via the frozen records snapshot.
pub fn display_text(
&self,
model: &Model,
row: usize,
col: usize,
fmt_comma: bool,
fmt_decimals: u8,
) -> String {
if self.is_records_mode() {
self.records_display(row, col).unwrap_or_default()
} else {
self.cell_key(row, col)
.and_then(|key| model.evaluate_aggregated(&key, &self.none_cats))
.map(|v| crate::format::format_value(Some(&v), fmt_comma, fmt_decimals))
.unwrap_or_default()
}
}
/// Build the CellKey for the data cell at (row, col), including the active
/// page-axis filter. Returns None if row or col is out of bounds.
/// In records mode: returns the real underlying CellKey when the column
/// is "Value" (editable); returns None for coord columns (read-only).
/// In records mode: returns a synthetic `(_Index, _Dim)` key for every column.
pub fn cell_key(&self, row: usize, col: usize) -> Option<CellKey> {
if let Some(records) = &self.records {
// Records mode: only the Value column maps to a real, editable cell.
if self.col_label(col) == "Value" {
return records.get(row).map(|(k, _)| k.clone());
} else {
if self.records.is_some() {
let records = self.records.as_ref().unwrap();
if row >= records.len() {
return None;
}
let col_label = self.col_label(col);
if col_label.is_empty() {
return None;
}
return Some(CellKey::new(vec![
("_Index".to_string(), row.to_string()),
("_Dim".to_string(), col_label),
]));
}
let row_item = self
.row_items
@ -527,7 +571,7 @@ fn cross_product(model: &Model, view: &View, cats: &[String]) -> Vec<AxisEntry>
#[cfg(test)]
mod tests {
use super::{AxisEntry, GridLayout};
use super::{synthetic_record_info, AxisEntry, GridLayout};
use crate::model::cell::{CellKey, CellValue};
use crate::model::Model;
use crate::view::Axis;
@ -592,40 +636,66 @@ mod tests {
}
#[test]
fn records_mode_cell_key_editable_for_value_column() {
fn records_mode_cell_key_returns_synthetic_for_all_columns() {
let mut m = records_model();
let v = m.active_view_mut();
v.set_axis("_Index", Axis::Row);
v.set_axis("_Dim", Axis::Column);
let layout = GridLayout::new(&m, m.active_view());
assert!(layout.is_records_mode());
// Find the "Value" column index
let cols: Vec<String> = (0..layout.col_count()).map(|i| layout.col_label(i)).collect();
// All columns return synthetic keys
let value_col = cols.iter().position(|c| c == "Value").unwrap();
// cell_key should be Some for Value column
let key = layout.cell_key(0, value_col);
assert!(key.is_some(), "Value column should be editable");
// cell_key should be None for coord columns
let key = layout.cell_key(0, value_col).unwrap();
assert_eq!(key.get("_Index"), Some("0"));
assert_eq!(key.get("_Dim"), Some("Value"));
let region_col = cols.iter().position(|c| c == "Region").unwrap();
assert!(
layout.cell_key(0, region_col).is_none(),
"Region column should not be editable"
);
let key = layout.cell_key(0, region_col).unwrap();
assert_eq!(key.get("_Index"), Some("0"));
assert_eq!(key.get("_Dim"), Some("Region"));
}
#[test]
fn records_mode_cell_key_maps_to_real_cell() {
fn records_mode_resolve_display_returns_values() {
let mut m = records_model();
let v = m.active_view_mut();
v.set_axis("_Index", Axis::Row);
v.set_axis("_Dim", Axis::Column);
let layout = GridLayout::new(&m, m.active_view());
let cols: Vec<String> = (0..layout.col_count()).map(|i| layout.col_label(i)).collect();
// Value column resolves to the cell value
let value_col = cols.iter().position(|c| c == "Value").unwrap();
// The CellKey at (0, Value) should look up a real cell value
let key = layout.cell_key(0, value_col).unwrap();
let val = m.evaluate(&key);
assert!(val.is_some(), "cell_key should resolve to a real cell");
let display = layout.resolve_display(&key);
assert!(display.is_some(), "Value column should resolve");
// Category column resolves to the coordinate value
let region_col = cols.iter().position(|c| c == "Region").unwrap();
let key = layout.cell_key(0, region_col).unwrap();
let display = layout.resolve_display(&key).unwrap();
assert!(!display.is_empty(), "Region column should resolve to a value");
}
#[test]
fn synthetic_record_info_returns_none_for_pivot_keys() {
let key = CellKey::new(vec![
("Region".to_string(), "East".to_string()),
("Product".to_string(), "Shoes".to_string()),
]);
assert!(synthetic_record_info(&key).is_none());
}
#[test]
fn synthetic_record_info_extracts_index_and_dim() {
let key = CellKey::new(vec![
("_Index".to_string(), "3".to_string()),
("_Dim".to_string(), "Region".to_string()),
]);
let (idx, dim) = synthetic_record_info(&key).unwrap();
assert_eq!(idx, 3);
assert_eq!(dim, "Region");
}
fn coord(pairs: &[(&str, &str)]) -> CellKey {

2
src/view/mod.rs
View File

@ -3,5 +3,5 @@ pub mod layout;
pub mod types;
pub use axis::Axis;
pub use layout::{AxisEntry, GridLayout};
pub use layout::{synthetic_record_info, AxisEntry, GridLayout};
pub use types::View;