diff --git a/crates/apps/spreadsheet/spreadsheet-engine/src/data.rs b/crates/apps/spreadsheet/spreadsheet-engine/src/data.rs index 79480ae..38ab0e6 100644 --- a/crates/apps/spreadsheet/spreadsheet-engine/src/data.rs +++ b/crates/apps/spreadsheet/spreadsheet-engine/src/data.rs @@ -1697,16 +1697,6 @@ fn sort_cmp(a: &SortKey, b: &SortKey) -> std::cmp::Ordering { } } -/// The active row sort: which column, which direction, and how to put the -/// rows back. `restore[view_row]` is the row that data originally lived on, -/// so `unsort_rows` can restore the pre-sort order after any chain of sorts. -#[derive(Clone, Debug)] -pub struct RowSort { - pub ascending: bool, - pub key_col: u32, - restore: Vec, -} - #[derive(Clone)] pub struct SpreadsheetData { pub cells: HashMap, @@ -1743,10 +1733,6 @@ pub struct SpreadsheetData { /// error instead of inventing a date. Transient state — never /// serialized. pub now_serial: Option, - /// The active row sort, if any. Transient state — never serialized. - /// `sort_rows` sets it; `unsort_rows` restores the pre-sort order and - /// clears it. - pub sort_order: Option, } impl Default for SpreadsheetData { @@ -1766,7 +1752,6 @@ impl Default for SpreadsheetData { spills: HashMap::new(), blocked_spills: HashMap::new(), now_serial: None, - sort_order: None, } } } @@ -2028,12 +2013,6 @@ impl SpreadsheetData { WorkbookCommand::SetSlider { row, col, slider } => { self.mutate_cell(row, col, |cell| cell.style.slider = slider) } - WorkbookCommand::SetChoices { row, col, choices } => { - self.mutate_cell(row, col, |cell| cell.style.choices = choices) - } - WorkbookCommand::SetButton { row, col, button } => { - self.mutate_cell(row, col, |cell| cell.style.button = button) - } } } @@ -2969,11 +2948,6 @@ impl SpreadsheetData { /// Undo history is cleared — a sort is a destructive bulk reorder and /// cannot be cleanly undone. Row heights, column widths and named /// ranges are positional and stay put, matching Excel. - /// - /// The sort is *physical*: cells are remapped into their new rows. To - /// make the header's "off" state possible, the mapping back to the - /// pre-sort order is remembered in `sort_order` (composed across sort - /// chains) and applied by [`SpreadsheetData::unsort_rows`]. pub fn sort_rows(&mut self, ascending: bool, key_col: u32) { // Fresh computed values so the sort keys use formula results. self.recalculate_all(); @@ -3009,18 +2983,6 @@ impl SpreadsheetData { for (new_pos, &old_row) in rows.iter().enumerate() { position[old_row as usize] = new_pos as u32; } - // `restore[new_pos] = original_row`, composed through any prior - // sort so a single `unsort_rows` undoes the whole chain. - let restore: Vec = match &self.sort_order { - Some(prev) => rows.iter().map(|&old| prev.restore[old as usize]).collect(), - None => rows.clone(), - }; - self.sort_order = Some(RowSort { - ascending, - key_col, - restore, - }); - let remapped: HashMap = self .cells .iter() @@ -3042,37 +3004,6 @@ impl SpreadsheetData { self.pending_recording = None; } - /// Restore the rows to their pre-sort order (the header sort's "off" - /// state). No-op when there is no active sort. Undo history is cleared, - /// like a sort. - pub fn unsort_rows(&mut self) { - let Some(sort) = self.sort_order.take() else { - return; - }; - let restore = sort.restore; - let remapped: HashMap = self - .cells - .iter() - .map(|(&id, cell)| { - ( - CellId::new(restore[id.row() as usize], id.col()), - cell.clone(), - ) - }) - .collect(); - self.cells = remapped; - - self.recalculate_all(); - self.undo_stack.clear(); - self.redo_stack.clear(); - self.pending_recording = None; - } - - /// The active sort, if any: `(key column, ascending)`. - pub fn sort_state(&self) -> Option<(u32, bool)> { - self.sort_order.as_ref().map(|s| (s.key_col, s.ascending)) - } - /// The sort key for `(row, col)`: the computed value for a formula /// cell, the raw value otherwise. fn sort_key_for(&self, row: u32, col: u32) -> SortKey { @@ -3184,7 +3115,6 @@ impl SpreadsheetData { self.undo_stack.clear(); self.redo_stack.clear(); self.pending_recording = None; - self.sort_order = None; let trimmed = data.trim_start(); @@ -3287,10 +3217,6 @@ impl SpreadsheetData { out.push_str(if cell.style.markdown { "1" } else { "0" }); out.push('\t'); out.push_str(if cell.style.slider { "1" } else { "0" }); - out.push('\t'); - out.push_str(if cell.style.button { "1" } else { "0" }); - out.push('\t'); - out.push_str(&crate::style::choices_to_str(&cell.style.choices)); out.push('\n'); } } @@ -3379,13 +3305,10 @@ impl SpreadsheetData { cell.style.border_bottom = border_from_str(bb); cell.style.border_left = border_from_str(bl); cell.style.border_right = border_from_str(br); - // The Markdown/slider/button flags and the choice list are - // appended after the borders; older files end at `br`, so - // they default to off/empty. + // The Markdown/slider flags are appended after the borders; + // older files end at `br`, so they default to off. cell.style.markdown = it.next().unwrap_or("0") == "1"; cell.style.slider = it.next().unwrap_or("0") == "1"; - cell.style.button = it.next().unwrap_or("0") == "1"; - cell.style.choices = crate::style::choices_from_str(it.next().unwrap_or("")); if cell.formula.is_none() { cell.computed_value.clear(); @@ -5295,206 +5218,3 @@ mod rich_cell_style_tests { assert!(!sheet.cells.get(&CellId::new(0, 1)).unwrap().style.slider); } } - -#[cfg(test)] -mod unsort_tests { - use super::*; - - /// After a sort, `sort_state` reports the key and direction; after an - /// unsort it is `None` and the rows are back in their original order. - #[test] - fn unsort_restores_the_original_order() { - let mut sheet = SpreadsheetData::default(); - sheet.set_cell(0, 0, "3"); - sheet.set_cell(0, 1, "c"); - sheet.set_cell(1, 0, "1"); - sheet.set_cell(1, 1, "a"); - sheet.set_cell(2, 0, "2"); - sheet.set_cell(2, 1, "b"); - - sheet.sort_rows(true, 0); - assert_eq!(sheet.sort_state(), Some((0, true))); - assert_eq!(sheet.get_raw(0, 0), "1"); - assert_eq!(sheet.get_raw(2, 0), "3"); - - sheet.unsort_rows(); - assert_eq!(sheet.sort_state(), None); - assert_eq!(sheet.get_raw(0, 0), "3"); - assert_eq!(sheet.get_raw(0, 1), "c"); - assert_eq!(sheet.get_raw(1, 0), "1"); - assert_eq!(sheet.get_raw(2, 0), "2"); - assert_eq!(sheet.get_raw(2, 1), "b"); - } - - /// A chain of sorts composes: unsorting once undoes all of them, not - /// just the last. - #[test] - fn unsort_undoes_the_whole_sort_chain() { - let mut sheet = SpreadsheetData::default(); - // Distinct values in two columns so the two sorts actually differ. - sheet.set_cell(0, 0, "30"); - sheet.set_cell(0, 1, "1"); - sheet.set_cell(1, 0, "10"); - sheet.set_cell(1, 1, "3"); - sheet.set_cell(2, 0, "20"); - sheet.set_cell(2, 1, "2"); - - sheet.sort_rows(true, 0); // order by A asc - sheet.sort_rows(true, 1); // then by B asc - - sheet.unsort_rows(); - assert_eq!(sheet.get_raw(0, 0), "30"); - assert_eq!(sheet.get_raw(0, 1), "1"); - assert_eq!(sheet.get_raw(1, 0), "10"); - assert_eq!(sheet.get_raw(1, 1), "3"); - assert_eq!(sheet.get_raw(2, 0), "20"); - assert_eq!(sheet.get_raw(2, 1), "2"); - } - - /// Unsorting without a sort is a no-op, not a panic. - #[test] - fn unsort_without_a_sort_is_a_noop() { - let mut sheet = SpreadsheetData::default(); - sheet.set_cell(0, 0, "x"); - sheet.unsort_rows(); - assert_eq!(sheet.get_raw(0, 0), "x"); - assert_eq!(sheet.sort_state(), None); - } - - /// A sort and its unsort both clear undo history. - #[test] - fn unsort_is_not_undoable() { - let mut sheet = SpreadsheetData::default(); - sheet.snapshot(); - sheet.set_cell(0, 0, "b"); - sheet.set_cell(1, 0, "a"); - sheet.sort_rows(true, 0); - assert!(!sheet.undo(), "sort must not be undoable"); - - sheet.unsort_rows(); - assert!(!sheet.undo(), "unsort must not be undoable"); - } - - /// Unsorting also recalculates moved formulas. - #[test] - fn unsort_recalculates_formulas() { - let mut sheet = SpreadsheetData::default(); - sheet.set_cell(0, 0, "2"); - sheet.set_cell(0, 1, "=A1"); - sheet.set_cell(1, 0, "1"); - sheet.set_cell(1, 1, "=A2"); - - sheet.sort_rows(true, 0); - sheet.unsort_rows(); - - assert_eq!(sheet.get_raw(0, 0), "2"); - assert_eq!(sheet.get_display_value(0, 1), "2"); - assert_eq!(sheet.get_raw(1, 0), "1"); - assert_eq!(sheet.get_display_value(1, 1), "1"); - } - - /// The sort order is transient: it survives cell edits but is reset by - /// a deserialize (fresh data has no sort). - #[test] - fn sort_order_is_cleared_on_deserialize() { - let mut sheet = SpreadsheetData::default(); - sheet.set_cell(0, 0, "3"); - sheet.set_cell(1, 0, "1"); - sheet.sort_rows(true, 0); - assert!(sheet.sort_state().is_some()); - - let serialized = sheet.serialize(); - let mut restored = SpreadsheetData::default(); - restored.deserialize(&serialized); - assert_eq!(restored.sort_state(), None); - // Cells serialize in their sorted positions; the restored sheet - // holds them there with no active sort. - assert_eq!(restored.get_raw(0, 0), "1"); - assert_eq!(restored.get_raw(1, 0), "3"); - } -} - -#[cfg(test)] -mod dropdown_button_style_tests { - use super::*; - - /// The button flag and dropdown choice list survive a serialize → - /// deserialize round trip, including choices with escaped separators. - #[test] - fn button_and_choices_survive_round_trip() { - let mut sheet = SpreadsheetData::default(); - sheet.set_cell(0, 0, "B2+10"); - sheet.set_cell(1, 0, "Low"); - sheet - .cells - .get_mut(&CellId::new(0, 0)) - .unwrap() - .style - .button = true; - sheet - .cells - .get_mut(&CellId::new(1, 0)) - .unwrap() - .style - .choices = vec!["Low".into(), "Med|ium".into(), "High".into()]; - - let serialized = sheet.serialize(); - let mut restored = SpreadsheetData::default(); - restored.deserialize(&serialized); - - assert!(restored.cells.get(&CellId::new(0, 0)).unwrap().style.button); - assert!(!restored.cells.get(&CellId::new(1, 0)).unwrap().style.button); - assert_eq!( - restored - .cells - .get(&CellId::new(1, 0)) - .unwrap() - .style - .choices, - vec!["Low".to_string(), "Med|ium".to_string(), "High".to_string()] - ); - assert!(restored - .cells - .get(&CellId::new(0, 0)) - .unwrap() - .style - .choices - .is_empty()); - } - - /// `apply_command` routes SetChoices and SetButton to the cell style, - /// creating the cell if it was empty. - #[test] - fn apply_command_sets_choices_and_button() { - let mut sheet = SpreadsheetData::default(); - sheet.apply_command(WorkbookCommand::SetButton { - row: 0, - col: 0, - button: true, - }); - sheet.apply_command(WorkbookCommand::SetChoices { - row: 0, - col: 1, - choices: vec!["a".into(), "b".into()], - }); - assert!(sheet.cells.get(&CellId::new(0, 0)).unwrap().style.button); - assert_eq!( - sheet.cells.get(&CellId::new(0, 1)).unwrap().style.choices, - vec!["a".to_string(), "b".to_string()] - ); - - // Clearing the choices empties the list. - sheet.apply_command(WorkbookCommand::SetChoices { - row: 0, - col: 1, - choices: vec![], - }); - assert!(sheet - .cells - .get(&CellId::new(0, 1)) - .unwrap() - .style - .choices - .is_empty()); - } -} diff --git a/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs b/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs index 87990ca..0966239 100644 --- a/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs +++ b/crates/apps/spreadsheet/spreadsheet-engine/src/style.rs @@ -137,12 +137,6 @@ pub struct CellStyle { pub markdown: bool, /// Render a numeric value cell as a slider (0–100) instead of text. pub slider: bool, - /// Render the cell as a dropdown: its value is the current selection - /// and `choices` is the list it cycles through on click. - pub choices: Vec, - /// Render the cell as a button: its value is a `TARGET[+N]` spec that - /// increments the target cell on click. - pub button: bool, } // --- Serialization helpers --- @@ -187,43 +181,6 @@ pub fn color_from_str(s: &str) -> Option { Some(Color::new(r, g, b, a)) } -/// Serialize a dropdown's choice list as one `|`-separated field, escaping -/// `|` and `\` so choices containing either round-trip. -pub fn choices_to_str(choices: &[String]) -> String { - choices - .iter() - .map(|c| c.replace('\\', "\\\\").replace('|', "\\|")) - .collect::>() - .join("|") -} - -/// Inverse of [`choices_to_str`]: an empty string is an empty list. -pub fn choices_from_str(s: &str) -> Vec { - if s.is_empty() { - return Vec::new(); - } - let mut out = Vec::new(); - let mut cur = String::new(); - let mut chars = s.chars(); - while let Some(ch) = chars.next() { - match ch { - '\\' => match chars.next() { - Some('|') => cur.push('|'), - Some('\\') => cur.push('\\'), - Some(other) => { - cur.push('\\'); - cur.push(other); - } - None => cur.push('\\'), - }, - '|' => out.push(std::mem::take(&mut cur)), - other => cur.push(other), - } - } - out.push(cur); - out -} - #[cfg(test)] mod tests { use super::*; @@ -298,29 +255,4 @@ mod tests { assert_eq!(NumberFormat::default(), NumberFormat::General); assert_eq!(CellAlign::default(), CellAlign::Left); } - - /// The choice list round-trips, including choices that contain the - /// separators themselves. - #[test] - fn choices_round_trip() { - for list in [ - vec!["Low".to_string(), "Medium".to_string(), "High".to_string()], - vec!["a|b".to_string(), "c".to_string()], - vec!["back\\slash".to_string(), "pipe|back\\both".to_string()], - vec!["single".to_string()], - vec![], - ] { - assert_eq!( - choices_from_str(&choices_to_str(&list)), - list, - "round trip failed for {list:?}" - ); - } - } - - /// An empty string is an empty list. - #[test] - fn choices_empty_string_is_an_empty_list() { - assert!(choices_from_str("").is_empty()); - } } diff --git a/crates/apps/spreadsheet/spreadsheet-engine/src/workbook_api.rs b/crates/apps/spreadsheet/spreadsheet-engine/src/workbook_api.rs index 64d8510..02f3c6d 100644 --- a/crates/apps/spreadsheet/spreadsheet-engine/src/workbook_api.rs +++ b/crates/apps/spreadsheet/spreadsheet-engine/src/workbook_api.rs @@ -124,18 +124,6 @@ pub enum WorkbookCommand { col: u32, slider: bool, }, - /// Set a cell's dropdown choice list (empty clears it). - SetChoices { - row: u32, - col: u32, - choices: Vec, - }, - /// Render the cell as a button (`TARGET[+N]` action on click). - SetButton { - row: u32, - col: u32, - button: bool, - }, } pub struct Workbook { @@ -414,14 +402,6 @@ impl Workbook { self.active_data_mut() .mutate_cell(row, col, |cell| cell.style.slider = slider); } - WorkbookCommand::SetChoices { row, col, choices } => { - self.active_data_mut() - .mutate_cell(row, col, |cell| cell.style.choices = choices); - } - WorkbookCommand::SetButton { row, col, button } => { - self.active_data_mut() - .mutate_cell(row, col, |cell| cell.style.button = button); - } } // Value-changing commands may move data other sheets read. let active = self.active; @@ -575,19 +555,6 @@ impl Workbook { self.after_value_mutation(active); } - /// Restore the active sheet's rows to their pre-sort order (the header - /// sort's "off" state). Cross-sheet dependents are recalculated. - pub fn unsort_active_sheet(&mut self) { - let active = self.active; - self.sheets[active].data.unsort_rows(); - self.after_value_mutation(active); - } - - /// The active sheet's sort, if any: `(key column, ascending)`. - pub fn active_sort_state(&self) -> Option<(u32, bool)> { - self.active_data().sort_state() - } - /// Move a column of the active sheet from its current position to the /// insertion index `to`. Cross-sheet dependents are recalculated so they /// see the rearranged values. @@ -1571,14 +1538,6 @@ mod tests { // The reader must now see the sorted top value. wb.set_active_sheet(0); assert_eq!(wb.get_display_value(0, 0), "1"); - - // Unsorting restores the original order, and the reader follows. - wb.set_active_sheet(1); - assert_eq!(wb.active_sort_state(), Some((0, true))); - wb.unsort_active_sheet(); - assert_eq!(wb.active_sort_state(), None); - wb.set_active_sheet(0); - assert_eq!(wb.get_display_value(0, 0), "3"); } /// Moving a column works at the workbook level, and a cross-sheet diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/button.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/button.rs deleted file mode 100644 index 12ced44..0000000 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/button.rs +++ /dev/null @@ -1,109 +0,0 @@ -//! Button-cell logic, extracted from the grid for testability. -//! -//! A cell whose `style.button` flag is set renders as a button and its -//! value is an action spec: `TARGET[+N]` / `TARGET[-N]`, where `TARGET` is -//! an A1-style cell reference and the optional signed `N` is the increment -//! (default +1). Clicking increments the target cell's numeric value — -//! the spreadsheet-native counterpart of the reference datagrid's "+10" -//! boost button. - -use spreadsheet_engine::data::{SpreadsheetData, NUM_COLS, NUM_ROWS}; - -/// Parse a button action spec into `(target_row, target_col, step)`. -/// `None` for a malformed spec or an out-of-bounds target. -pub fn parse_button_spec(value: &str) -> Option<(u32, u32, f64)> { - let v = value.trim(); - if v.is_empty() { - return None; - } - let chars: Vec = v.chars().collect(); - // The step starts at the first '+'/'-' after the reference. - let mut split = None; - for (n, &c) in chars.iter().enumerate() { - if n > 0 && (c == '+' || c == '-') { - split = Some(n); - break; - } - } - let (ref_part, step) = match split { - Some(n) => { - let ref_s: String = chars[..n].iter().collect(); - let step_s: String = chars[n..].iter().collect(); - (ref_s, step_s.parse::().ok()?) - } - None => (v.to_string(), 1.0), - }; - let (row, col) = SpreadsheetData::parse_cell_ref(ref_part.trim())?; - if row >= NUM_ROWS || col >= NUM_COLS { - return None; - } - Some((row, col, step)) -} - -/// The target's new value after a button step: `current + step`, or the -/// step itself (never below zero) for an empty/non-numeric target. -pub fn button_step(current: Option, step: f64) -> f64 { - match current { - Some(v) => v + step, - None => step.max(0.0), - } -} - -/// Format a stepped value: integers render without a fraction, fractional -/// steps keep their digits. -pub fn format_step_value(v: f64) -> String { - if v.fract().abs() < 1e-9 { - format!("{:.0}", v) - } else { - format!("{}", v) - } -} - -#[cfg(test)] -mod tests { - use super::*; - - /// `REF`, `REF+N` and `REF-N` all parse. - #[test] - fn specs_parse_to_target_and_step() { - assert_eq!(parse_button_spec("B2"), Some((1, 1, 1.0))); - assert_eq!(parse_button_spec("B2+10"), Some((1, 1, 10.0))); - assert_eq!(parse_button_spec("B2-5"), Some((1, 1, -5.0))); - assert_eq!(parse_button_spec("A1+0.5"), Some((0, 0, 0.5))); - assert_eq!(parse_button_spec(" Z42 +3 "), Some((41, 25, 3.0))); - } - - /// Malformed specs and out-of-bounds targets yield `None`. - #[test] - fn bad_specs_are_rejected() { - assert_eq!(parse_button_spec(""), None); - assert_eq!(parse_button_spec("not a ref"), None); - assert_eq!(parse_button_spec("+10"), None, "no target before the step"); - assert_eq!(parse_button_spec("B2+abc"), None); - // A target column past the grid is rejected. - assert_eq!(parse_button_spec("A99999"), None); - } - - /// Stepping adds to an existing value and starts an empty target at the - /// step (clamped to zero for a negative step). - #[test] - fn steps_apply_to_numeric_and_empty_targets() { - assert_eq!(button_step(Some(42.0), 10.0), 52.0); - assert_eq!(button_step(Some(42.0), -10.0), 32.0); - assert_eq!(button_step(None, 10.0), 10.0); - assert_eq!( - button_step(None, -10.0), - 0.0, - "a decrement can't start negative" - ); - } - - /// The stepped value formats cleanly for a cell. - #[test] - fn step_values_format() { - assert_eq!(format_step_value(52.0), "52"); - assert_eq!(format_step_value(0.0), "0"); - assert_eq!(format_step_value(0.5), "0.5"); - assert_eq!(format_step_value(-3.25), "-3.25"); - } -} diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/dropdown.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/dropdown.rs deleted file mode 100644 index d579b67..0000000 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/dropdown.rs +++ /dev/null @@ -1,77 +0,0 @@ -//! Dropdown-cell logic, extracted from the grid for testability. -//! -//! A cell whose `style.choices` is non-empty renders as a dropdown: its -//! value is the current selection and a click cycles to the next choice. -//! The list is stored on the cell style; the toolbar's "Drop" button parses -//! the cell value as a `|`-separated list to set it. - -/// The next choice after `current`, cycling to the front at the end. When -/// `current` is not in the list (e.g. the list was just set), the first -/// choice is returned. An empty list yields `None`. -pub fn next_choice(current: &str, choices: &[String]) -> Option { - if choices.is_empty() { - return None; - } - match choices.iter().position(|c| c == current) { - Some(i) => Some(choices[(i + 1) % choices.len()].clone()), - None => Some(choices[0].clone()), - } -} - -/// Parse a pipe-separated choice list from a cell value (`"Low|Med|High"`). -/// Entries are trimmed and empty entries dropped. -pub fn parse_choice_list(value: &str) -> Vec { - value - .split('|') - .map(str::trim) - .filter(|s| !s.is_empty()) - .map(str::to_string) - .collect() -} - -#[cfg(test)] -mod tests { - use super::*; - - /// Cycling walks the list in order and wraps around. - #[test] - fn next_choice_cycles_in_order() { - let choices = ["Low", "Medium", "High"].map(String::from).to_vec(); - assert_eq!(next_choice("Low", &choices).as_deref(), Some("Medium")); - assert_eq!(next_choice("Medium", &choices).as_deref(), Some("High")); - assert_eq!(next_choice("High", &choices).as_deref(), Some("Low")); - } - - /// A current value not in the list starts at the first choice. - #[test] - fn next_choice_starts_at_the_first_when_absent() { - let choices = ["a", "b"].map(String::from).to_vec(); - assert_eq!(next_choice("zzz", &choices).as_deref(), Some("a")); - assert_eq!(next_choice("", &choices).as_deref(), Some("a")); - } - - /// An empty list has no next choice. - #[test] - fn next_choice_of_an_empty_list_is_none() { - assert_eq!(next_choice("x", &[]), None); - } - - /// The pipe list parse trims and drops empties. - #[test] - fn parse_choice_list_splits_trims_and_drops_empties() { - assert_eq!( - parse_choice_list("Low|Medium|High"), - vec!["Low".to_string(), "Medium".to_string(), "High".to_string()] - ); - assert_eq!( - parse_choice_list(" a | b "), - vec!["a".to_string(), "b".to_string()] - ); - assert_eq!( - parse_choice_list("a||b"), - vec!["a".to_string(), "b".to_string()] - ); - assert_eq!(parse_choice_list(""), Vec::::new()); - assert_eq!(parse_choice_list("only"), vec!["only".to_string()]); - } -} diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/grid.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/grid.rs index 5edbac7..eb299c1 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/grid.rs +++ b/crates/apps/spreadsheet/spreadsheet-ui/src/grid.rs @@ -548,22 +548,6 @@ impl Widget for SpreadsheetGrid { self.request_commit_edit(); } - // A button cell runs its TARGET[+N] action on click. - if self.apply_button_cell(cx, row, col) { - return; - } - - // A dropdown cell cycles to its next choice on click. - if self.cycle_dropdown_cell(cx, row, col) { - self.input_controller.record(InputIntent::Select { - row, - col, - extend: false, - }); - cx.widget_action(self.widget_uid(), CellAction::Selected(row, col)); - return; - } - // A slider cell jumps to the pressed position and then // drags; one snapshot covers the whole drag. if self.cell_slider_value(row, col).is_some() { @@ -1111,9 +1095,7 @@ impl SpreadsheetGrid { | WorkbookCommand::SetBackground { row, col, .. } | WorkbookCommand::SetBorders { row, col, .. } | WorkbookCommand::SetMarkdown { row, col, .. } - | WorkbookCommand::SetSlider { row, col, .. } - | WorkbookCommand::SetChoices { row, col, .. } - | WorkbookCommand::SetButton { row, col, .. } => Some(CellId::new(*row, *col)), + | WorkbookCommand::SetSlider { row, col, .. } => Some(CellId::new(*row, *col)), WorkbookCommand::SetColumnWidth { .. } | WorkbookCommand::SetRowHeight { .. } | WorkbookCommand::MoveColumn { .. } => None, @@ -1740,8 +1722,7 @@ impl SpreadsheetGrid { let checkbox_layout = crate::checkbox::checkbox_layout(x, y, cw, rh, checkbox_checked); - // Markdown, slider, dropdown and button are style flags on - // the cell. + // Markdown and slider are style flags on the cell. let is_markdown = cached_state .as_ref() .map(|s| s.markdown) @@ -1752,16 +1733,6 @@ impl SpreadsheetGrid { .map(|s| s.slider) .or_else(|| cell_ref.map(|c| c.style.slider)) .unwrap_or(false); - let has_choices = cached_state - .as_ref() - .map(|s| !s.choices.is_empty()) - .or_else(|| cell_ref.map(|c| !c.style.choices.is_empty())) - .unwrap_or(false); - let is_button = cached_state - .as_ref() - .map(|s| s.button) - .or_else(|| cell_ref.map(|c| c.style.button)) - .unwrap_or(false); self.draw_cell_bg.color = if is_selected { self.selected_bg_color @@ -1856,45 +1827,7 @@ impl SpreadsheetGrid { } else { None }; - if is_button && !is_editing_this_cell { - // Button: a raised box with the action spec centred. - let bx = x + 2.0; - let by = y + 2.0; - let bw = (cw - 4.0).max(1.0); - let bh = (rh - 4.0).max(1.0); - self.draw_cell_bg.color = self.selected_bg_color; - self.draw_cell_bg.draw_abs( - cx, - Rect { - pos: dvec2(bx, by), - size: dvec2(bw, bh), - }, - ); - self.draw_grid_line.color = self.selected_border_color; - for &(ex, ey, ew, eh) in &[ - (bx, by, bw, 1.0), - (bx, by + bh - 1.0, bw, 1.0), - (bx, by, 1.0, bh), - (bx + bw - 1.0, by, 1.0, bh), - ] { - self.draw_grid_line.draw_abs( - cx, - Rect { - pos: dvec2(ex, ey), - size: dvec2(ew, eh), - }, - ); - } - if !display_is_empty { - let w = self.text_measure_cache.measure(&display_buf, false); - self.draw_text.color = self.text_color; - self.draw_text.draw_abs( - cx, - dvec2(x + (cw - w) * 0.5, y + (rh - 14.0) * 0.5), - &display_buf, - ); - } - } else if let Some(v) = slider_value { + if let Some(v) = slider_value { let layout = crate::slider::slider_layout(x, y, cw, rh, v); self.draw_grid_line.color = self.header_border_color; let (tx, ty, tw, th) = layout.track; @@ -2030,8 +1963,6 @@ impl SpreadsheetGrid { sparkline: cell.sparkline.clone(), markdown: cell.style.markdown, slider: cell.style.slider, - choices: cell.style.choices.clone(), - button: cell.style.button, }, ); } @@ -2055,13 +1986,6 @@ impl SpreadsheetGrid { }; draw_text.draw_abs(cx, dvec2(text_x, text_y), &display_buf); - // A dropdown shows a trailing ▾ affordance. - if has_choices { - self.draw_header_text.color = self.header_text_color; - self.draw_header_text - .draw_abs(cx, dvec2(x + cw - 16.0, text_y), "▾"); - } - if cached_state .as_ref() .map(|state| state.underline) @@ -2319,62 +2243,6 @@ impl SpreadsheetGrid { true } - /// If `(row, col)` is a button cell, run its `TARGET[+N]` action: - /// increment the target cell's numeric value (undoable). Returns `true` - /// when an action ran. - fn apply_button_cell(&mut self, cx: &mut Cx, row: u32, col: u32) -> bool { - let spec = self.with_data(|data| { - data.cells - .get(&CellId::new(row, col)) - .filter(|c| c.style.button && c.formula.is_none()) - .map(|c| c.value.clone()) - }); - let Some(spec) = spec else { - return false; - }; - let Some((tr, tc, step)) = crate::button::parse_button_spec(&spec) else { - return false; - }; - let current = self.with_data(|data| { - data.cells - .get(&CellId::new(tr, tc)) - .and_then(|c| c.value.trim().parse::().ok()) - }); - let new_val = crate::button::button_step(current, step); - let text = crate::button::format_step_value(new_val); - self.with_data_mut(|data| data.snapshot()); - self.apply_command(WorkbookCommand::SetCell { - row: tr, - col: tc, - value: text, - }); - self.redraw(cx); - true - } - - /// If `(row, col)` is a dropdown cell, cycle its value to the next - /// choice (undoable). Returns `true` when it cycled. - fn cycle_dropdown_cell(&mut self, cx: &mut Cx, row: u32, col: u32) -> bool { - let (current, choices) = self.with_data(|data| { - data.cells - .get(&CellId::new(row, col)) - .filter(|c| c.formula.is_none() && !c.style.choices.is_empty()) - .map(|c| (c.value.clone(), c.style.choices.clone())) - .unwrap_or_default() - }); - let Some(next) = crate::dropdown::next_choice(¤t, &choices) else { - return false; - }; - self.with_data_mut(|data| data.snapshot()); - self.apply_command(WorkbookCommand::SetCell { - row, - col, - value: next, - }); - self.redraw(cx); - true - } - fn request_begin_edit(&mut self, row: u32, col: u32) { self.input_controller.record(route_edit_begin(row, col)); } @@ -2858,89 +2726,6 @@ impl SpreadsheetGrid { } } - pub fn toggle_button(&mut self, cx: &mut Cx) { - if let Some(sel) = self.selection_bounds() { - self.with_data_mut(|data| data.snapshot()); - let mut any_on = false; - for r in sel.0..=sel.2 { - for c in sel.1..=sel.3 { - let on = self.with_data(|data| { - data.cells - .get(&CellId::new(r, c)) - .map(|cell| cell.style.button) - .unwrap_or(false) - }); - any_on |= on; - } - } - let target = !any_on; - for r in sel.0..=sel.2 { - for c in sel.1..=sel.3 { - self.apply_command(WorkbookCommand::SetButton { - row: r, - col: c, - button: target, - }); - } - } - self.redraw(cx); - } - } - - /// Toggle dropdowns on the selection. Turning on parses each cell's - /// value as a `|`-separated choice list and selects the first choice; - /// turning off clears the list, leaving the value as plain text. - pub fn toggle_dropdown(&mut self, cx: &mut Cx) { - if let Some(sel) = self.selection_bounds() { - self.with_data_mut(|data| data.snapshot()); - let mut any_on = false; - for r in sel.0..=sel.2 { - for c in sel.1..=sel.3 { - let on = self.with_data(|data| { - data.cells - .get(&CellId::new(r, c)) - .map(|cell| !cell.style.choices.is_empty()) - .unwrap_or(false) - }); - any_on |= on; - } - } - let target = !any_on; - for r in sel.0..=sel.2 { - for c in sel.1..=sel.3 { - if target { - let value = self.with_data(|data| { - data.cells - .get(&CellId::new(r, c)) - .map(|cell| cell.value.clone()) - .unwrap_or_default() - }); - let choices = crate::dropdown::parse_choice_list(&value); - if let Some(first) = choices.first().cloned() { - self.apply_command(WorkbookCommand::SetChoices { - row: r, - col: c, - choices, - }); - self.apply_command(WorkbookCommand::SetCell { - row: r, - col: c, - value: first, - }); - } - } else { - self.apply_command(WorkbookCommand::SetChoices { - row: r, - col: c, - choices: Vec::new(), - }); - } - } - } - self.redraw(cx); - } - } - pub fn set_borders(&mut self, cx: &mut Cx, target: BorderTarget) { if let Some(sel) = self.selection_bounds() { self.with_data_mut(|data| data.snapshot()); diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/lib.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/lib.rs index 3354291..d53bfd9 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/lib.rs +++ b/crates/apps/spreadsheet/spreadsheet-ui/src/lib.rs @@ -9,11 +9,9 @@ //! Depends on `spreadsheet-engine` for all logic. Converts `Color` //! ↔ `Vec4f` at the rendering boundary. -pub mod button; pub mod chart; pub mod checkbox; pub mod clipboard; -pub mod dropdown; pub mod edit; pub mod event_router; pub mod formula_bar; diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/render_cache.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/render_cache.rs index d6fbac1..a542d22 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/render_cache.rs +++ b/crates/apps/spreadsheet/spreadsheet-ui/src/render_cache.rs @@ -34,10 +34,6 @@ pub struct CellRenderState { pub markdown: bool, /// The cell renders as a slider (numeric value). pub slider: bool, - /// The cell renders as a dropdown; `choices` is its cycle list. - pub choices: Vec, - /// The cell renders as a button (`TARGET[+N]` action). - pub button: bool, } #[derive(Default, Debug)] @@ -156,8 +152,6 @@ mod tests { sparkline: None, markdown: false, slider: false, - choices: Vec::new(), - button: false, }; cache.insert(id, state.clone()); assert_eq!(cache.get(id), Some(&state)); @@ -195,8 +189,6 @@ mod tests { sparkline: None, markdown: false, slider: false, - choices: Vec::new(), - button: false, }); assert_eq!(built.display_text, "built"); assert!(built.is_formula); diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/sort_state.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/sort_state.rs index 9298054..b27b1c6 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/sort_state.rs +++ b/crates/apps/spreadsheet/spreadsheet-ui/src/sort_state.rs @@ -4,20 +4,18 @@ //! unit-tested; the decision it makes — which column is sorted next, and in //! which direction — is pure, so it lives here. -/// Advance the header sort state after a click on `col`, matching the -/// datagrid reference's ascending → descending → off cycle: +/// Advance the header sort state after a click on `col`. Clicking the +/// already-sorted column flips its direction; clicking any other column +/// starts a fresh ascending sort on that column. /// -/// - clicking a new column starts ascending on it; -/// - clicking the ascending column flips it to descending; -/// - clicking the descending column turns the sort off (`None`). -/// -/// "Off" is `None` and is applied by the engine's `unsort_rows`, which -/// restores the pre-sort order. -pub fn next_sort_state(current: Option<(u32, bool)>, col: u32) -> Option<(u32, bool)> { +/// The datagrid reference cycles ascending → descending → off. We stop at +/// two states: `sort_rows` physically permutes the sheet and clears its undo +/// history, so there is no earlier order for "off" to restore. A future +/// display-order sort could widen this back to three states. +pub fn next_sort_direction(current: Option<(u32, bool)>, col: u32) -> (u32, bool) { match current { - Some((c, true)) if c == col => Some((col, false)), - Some((c, false)) if c == col => None, - _ => Some((col, true)), + Some((c, asc)) if c == col => (col, !asc), + _ => (col, true), } } @@ -25,32 +23,16 @@ pub fn next_sort_state(current: Option<(u32, bool)>, col: u32) -> Option<(u32, b mod tests { use super::*; - /// A new column (or no sort yet) starts ascending. #[test] fn a_new_column_starts_ascending() { - assert_eq!(next_sort_state(None, 3), Some((3, true))); - assert_eq!(next_sort_state(Some((1, true)), 3), Some((3, true))); - assert_eq!(next_sort_state(Some((1, false)), 3), Some((3, true))); + assert_eq!(next_sort_direction(None, 3), (3, true)); + assert_eq!(next_sort_direction(Some((1, true)), 3), (3, true)); + assert_eq!(next_sort_direction(Some((1, false)), 3), (3, true)); } - /// The same column cycles ascending → descending → off. #[test] - fn the_same_column_cycles_asc_desc_off() { - assert_eq!(next_sort_state(Some((4, true)), 4), Some((4, false))); - assert_eq!(next_sort_state(Some((4, false)), 4), None); - // Off cycles back to ascending. - assert_eq!(next_sort_state(None, 4), Some((4, true))); - } - - /// A full three-click cycle returns to where it started. - #[test] - fn a_full_cycle_returns_to_off() { - let mut state = None; - state = next_sort_state(state, 2); - assert_eq!(state, Some((2, true))); - state = next_sort_state(state, 2); - assert_eq!(state, Some((2, false))); - state = next_sort_state(state, 2); - assert_eq!(state, None); + fn the_same_column_flips_direction() { + assert_eq!(next_sort_direction(Some((4, true)), 4), (4, false)); + assert_eq!(next_sort_direction(Some((4, false)), 4), (4, true)); } } diff --git a/crates/apps/spreadsheet/spreadsheet-ui/src/workspace.rs b/crates/apps/spreadsheet/spreadsheet-ui/src/workspace.rs index 56825b0..f6d23f3 100644 --- a/crates/apps/spreadsheet/spreadsheet-ui/src/workspace.rs +++ b/crates/apps/spreadsheet/spreadsheet-ui/src/workspace.rs @@ -47,10 +47,6 @@ pub struct SpreadsheetWorkspace { /// the label when the numbers actually change (scroll/resize). #[rust] status_cache: String, - /// The last header-sort report ("sorted N rows by A ascending in 2 ms"), - /// appended to the status bar. - #[rust] - sort_status: String, // --- Dynamic tab strip (B16) --- /// Draw resources for manually rendering tab buttons. @@ -153,7 +149,6 @@ impl Widget for SpreadsheetWorkspace { self.formula_bar = FormulaBar::new(); self.formula_was_focused = false; self.status_cache.clear(); - self.sort_status.clear(); self.initialized = true; } @@ -184,17 +179,7 @@ impl Widget for SpreadsheetWorkspace { // change it). Cached so the label only re-lays-out on a change. if let Some(grid) = self.view.widget(cx, ids!(grid)).borrow::() { let (vc, vr) = grid.visible_cell_counts(); - let status = if self.sort_status.is_empty() { - format!("Ready | {} × {} visible = {} cells", vc, vr, vc * vr) - } else { - format!( - "Ready | {} × {} visible = {} cells | {}", - vc, - vr, - vc * vr, - self.sort_status - ) - }; + let status = format!("Ready | {} × {} visible = {} cells", vc, vr, vc * vr); if status != self.status_cache { self.status_cache = status.clone(); self.label(cx, ids!(status_label)).set_text(cx, &status); @@ -673,18 +658,6 @@ impl WidgetMatchEvent for SpreadsheetWorkspace { grid.toggle_slider(cx); } } - if self.button(cx, ids!(drop_btn)).clicked(actions) { - self.commit_formula_bar_if_dirty(cx); - if let Some(mut grid) = self.widget(cx, ids!(grid)).borrow_mut::() { - grid.toggle_dropdown(cx); - } - } - if self.button(cx, ids!(btn_btn)).clicked(actions) { - self.commit_formula_bar_if_dirty(cx); - if let Some(mut grid) = self.widget(cx, ids!(grid)).borrow_mut::() { - grid.toggle_button(cx); - } - } if self.button(cx, ids!(align_left_btn)).clicked(actions) { self.commit_formula_bar_if_dirty(cx); @@ -863,43 +836,26 @@ impl WidgetMatchEvent for SpreadsheetWorkspace { } } CellAction::HeaderClicked(col) => { - // Ascending → descending → off, timed so the status bar - // reports how long the sort took. + // Toggle the header sort state and re-sort the active + // sheet. Clicking the sorted column flips its direction; + // any other column starts a fresh ascending sort. let next = if let Some(grid) = self.widget(cx, ids!(grid)).borrow::() { - crate::sort_state::next_sort_state(grid.sort_state, col) + crate::sort_state::next_sort_direction(grid.sort_state, col) } else { - Some((col, true)) + (col, true) }; if let Some(mut grid) = self.widget(cx, ids!(grid)).borrow_mut::() { - grid.sort_state = next; + grid.sort_state = Some(next); grid.invalidate_all(); } - let label = spreadsheet_engine::util::col_letters(col); - match next { - Some((key_col, asc)) => { - let t0 = std::time::Instant::now(); - self.model - .borrow_mut() - .workbook_mut() - .sort_active_sheet(asc, key_col); - let ms = t0.elapsed().as_millis(); - self.sort_status = format!( - "sorted {} rows by {} {} in {} ms", - spreadsheet_engine::data::NUM_ROWS, - label, - if asc { "ascending" } else { "descending" }, - ms - ); - } - None => { - self.model.borrow_mut().workbook_mut().unsort_active_sheet(); - self.sort_status = format!("sort by {label} cleared"); - } - } + self.model + .borrow_mut() + .workbook_mut() + .sort_active_sheet(next.1, next.0); self.view.redraw(cx); } _ => {} @@ -958,8 +914,6 @@ script_mod! { separator5 := View { width: 1.0, height: 26.0, draw_bg +: { color: #x3a3a4e } } md_btn := Button { text: "Md", width: 36.0, draw_bg +: { color: #x313244 }, draw_text +: { color: #xd8d8e8, text_style +: { font_size: 11.0 } } } slider_btn := Button { text: "Slider", width: 50.0, draw_bg +: { color: #x313244 }, draw_text +: { color: #xd8d8e8, text_style +: { font_size: 10.5 } } } - drop_btn := Button { text: "Drop", width: 44.0, draw_bg +: { color: #x313244 }, draw_text +: { color: #xd8d8e8, text_style +: { font_size: 10.5 } } } - btn_btn := Button { text: "Btn", width: 38.0, draw_bg +: { color: #x313244 }, draw_text +: { color: #xd8d8e8, text_style +: { font_size: 11.0 } } } separator5b := View { width: 1.0, height: 26.0, draw_bg +: { color: #x3a3a4e } } border_all_btn := Button { text: "B-All", width: 44.0, draw_bg +: { color: #x313244 }, draw_text +: { color: #xd8d8e8, text_style +: { font_size: 10.0 } } } border_outer_btn := Button { text: "B-Out", width: 44.0, draw_bg +: { color: #x313244 }, draw_text +: { color: #xd8d8e8, text_style +: { font_size: 10.0 } } } diff --git a/tools/test-spreadsheet-coverage.sh b/tools/test-spreadsheet-coverage.sh index 30a7cd1..0d750ec 100755 --- a/tools/test-spreadsheet-coverage.sh +++ b/tools/test-spreadsheet-coverage.sh @@ -267,13 +267,13 @@ if [[ "$COVERAGE_TARGET" == "all" || "$COVERAGE_TARGET" == "ui" ]]; then # Only the controller modules are measured; see the exclusion note above. report_for "ui-controllers" "$WORK/ui.profdata" \ "$IGNORE_UI" "$UI_FLOOR" \ - "$UI/src/button.rs" "$UI/src/chart.rs" "$UI/src/checkbox.rs" "$UI/src/clipboard.rs" "$UI/src/dropdown.rs" "$UI/src/edit.rs" "$UI/src/event_router.rs" \ + "$UI/src/chart.rs" "$UI/src/checkbox.rs" "$UI/src/clipboard.rs" "$UI/src/edit.rs" "$UI/src/event_router.rs" \ "$UI/src/formula_bar.rs" \ "$UI/src/geometry.rs" "$UI/src/input.rs" "$UI/src/market.rs" "$UI/src/markdown.rs" "$UI/src/model.rs" "$UI/src/render_cache.rs" \ "$UI/src/selection.rs" "$UI/src/slider.rs" "$UI/src/sort_state.rs" "$UI/src/sparkline.rs" "$UI/src/text_measure.rs" "$UI/src/zoom.rs" if [[ "$KEEP_COVERAGE" == "1" ]]; then uncovered_listing "ui" "$WORK/ui.profdata" "$IGNORE_UI" \ - "$UI/src/button.rs" "$UI/src/chart.rs" "$UI/src/checkbox.rs" "$UI/src/clipboard.rs" "$UI/src/dropdown.rs" "$UI/src/edit.rs" "$UI/src/event_router.rs" \ + "$UI/src/chart.rs" "$UI/src/checkbox.rs" "$UI/src/clipboard.rs" "$UI/src/edit.rs" "$UI/src/event_router.rs" \ "$UI/src/formula_bar.rs" \ "$UI/src/geometry.rs" "$UI/src/input.rs" "$UI/src/market.rs" "$UI/src/markdown.rs" "$UI/src/model.rs" "$UI/src/render_cache.rs" \ "$UI/src/selection.rs" "$UI/src/slider.rs" "$UI/src/sort_state.rs" "$UI/src/sparkline.rs" "$UI/src/text_measure.rs" "$UI/src/zoom.rs"