diff --git a/crates/engine/src/cmd/slicer.rs b/crates/engine/src/cmd/slicer.rs index 00342a5a9..cb72ded71 100644 --- a/crates/engine/src/cmd/slicer.rs +++ b/crates/engine/src/cmd/slicer.rs @@ -97,11 +97,21 @@ fn reapply_slicer(wb: &mut Workbook, sheet: usize, id: u32) { } fn insert_slicer(s: &mut Session, p: &Json) -> Result { - let sheet = target_sheet(s, p)?; + // A table named by `table` is found on any sheet, as the other table commands find it, and the slicer goes on that + // table's sheet. Looked up on the active sheet only, a named table on another sheet gave "Select a cell inside a + // table first, or pass `table`" although `table` was passed. + let named = match str_param(p, "table") { + Some(name) => Some(s.doc()?.wb.table(name).ok_or_else(|| bad("insert.slicer", format!("no table named `{name}`")))?), + None => None, + }; + let sheet = match named { + Some((si, _)) => si, + None => target_sheet(s, p)?, + }; let d = s.doc()?; let sh = d.wb.sheet(sheet).ok_or(EngineError::NoDocument)?; - let t = match str_param(p, "table") { - Some(name) => sh.tables.iter().find(|t| t.name.eq_ignore_ascii_case(name)), + let t = match named { + Some((_, ti)) => sh.tables.get(ti), None => sh.table_at(d.selection.active), } .ok_or_else(|| EngineError::Other("Select a cell inside a table first, or pass `table`.".into()))?; @@ -119,7 +129,7 @@ fn insert_slicer(s: &mut Session, p: &Json) -> Result { } }; let table = t.name.clone(); - let at = cell_param(s, p, "at")? + let at = cell_param_on(s, p, "at", sheet)? .unwrap_or_else(|| CellRef::new(t.range.start.row, t.range.end.col.saturating_add(2).min(gridcraft_core::MAX_COLS - 1))); let columns = u32_param(p, "columns").unwrap_or(1).clamp(1, 10); let width = f64_param(p, "width").unwrap_or(168.0).clamp(60.0, 10000.0) as f32; diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 0146db6ea..9acdc3d32 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1242,6 +1242,35 @@ fn table_names_are_limited_to_255_characters() { assert!(s.doc().unwrap().wb.table(&"ع".repeat(200)).is_some()); } +#[test] +fn slicer_finds_a_named_table_on_another_sheet() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Region", "Qty"], ["East", 2], ["West", 5]]})).unwrap(); + s.execute("insert.table", json!({"range": "A1:B3", "name": "Sales"})).unwrap(); + s.execute("home.insertSheet", json!({"name": "Report"})).unwrap(); + let report = s.doc().unwrap().wb.sheet_index("Report").unwrap(); + assert_eq!(s.doc().unwrap().wb.active_sheet, report); + + // With another sheet active, `table` still names the table, and the slicer goes on the table's sheet. + let r = s.execute("insert.slicer", json!({"table": "Sales", "column": "Region", "at": "D2"})).unwrap(); + let id = r["slicer"].as_u64().unwrap(); + let home = s.doc().unwrap().wb.sheet_index("Sheet1").unwrap(); + assert_eq!(s.doc().unwrap().wb.sheet(home).unwrap().slicers.len(), 1); + assert!(s.doc().unwrap().wb.sheet(report).unwrap().slicers.is_empty()); + // It filters the table: "East" leaves the East row. + s.execute("selection.set", json!({"range": "Sheet1!A1"})).unwrap(); + s.execute("slicer.toggle", json!({"slicer": id, "value": "East"})).unwrap(); + let sh = s.doc().unwrap().wb.sheet(home).unwrap(); + assert!(!sh.is_row_hidden(1) && sh.is_row_hidden(2)); + + // `at` may name the table's sheet, and a table that isn't there is named in the error. + s.execute("home.insertSheet", json!({"name": "Notes"})).unwrap(); + s.execute("table.insertSlicer", json!({"table": "sales", "column": "Qty", "at": "Sheet1!G2"})).unwrap(); + assert_eq!(s.doc().unwrap().wb.sheet(home).unwrap().slicers.len(), 2); + let e = s.execute("insert.slicer", json!({"table": "Missing", "column": "Region"})).unwrap_err(); + assert!(e.to_string().contains("Missing"), "{e}"); +} + #[test] fn slicer_filters_table() { let mut s = s();