From c45212ac688e0118d7656668062e8fe08e662786 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:27:27 +0200 Subject: [PATCH 01/17] Refuse inserting cells that would push values off the worksheet (#580) --- crates/engine/src/cmd/sheet.rs | 9 +++++++++ crates/engine/src/tests.rs | 18 ++++++++++++++++++ 2 files changed, 27 insertions(+) diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index f05b77d91..14bb37f87 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -427,6 +427,15 @@ fn insert_cells(s: &mut Session, p: &Json) -> Result { let sheet = target_sheet(s, p)?; if let Some(sh) = s.doc()?.wb.sheet(sheet) { check_table_shift(sh, &e)?; + // Refuse to push data off the sheet: cells in the last `width`/`height` positions would be dropped. + let doomed = if shift == "right" { + RangeRef::new(CellRef::new(r.start.row, r.start.col.max(MAX_COLS.saturating_sub(r.width()))), CellRef::new(r.end.row, MAX_COLS - 1)) + } else { + RangeRef::new(CellRef::new(r.start.row.max(MAX_ROWS.saturating_sub(r.height())), r.start.col), CellRef::new(MAX_ROWS - 1, r.end.col)) + }; + if sh.cells.iter_range(doomed).any(|(_, x)| x.formula.is_some() || !x.value.is_empty()) { + return Err(EngineError::Other("To prevent possible loss of data, Excel cannot shift nonblank cells off of the worksheet.".into())); + } } let right = shift == "right"; let out = edit(s, |cx| { diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index 33fdc113e..c1173fff4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1121,6 +1121,24 @@ fn a_whole_column_table_without_headers_stays_on_the_sheet() { assert!(e.to_string().contains("pushed off the end"), "{e}"); } +#[test] +fn inserting_cells_refuses_to_push_values_off_the_sheet() { + for (shift, edge, last_fit, last_ok) in [("right", "XFD1", "XFC1", "XFD1"), ("down", "A1048576", "A1048575", "A1048576")] { + let mut sess = s(); + sess.execute("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); + sess.execute("cell.set", json!({"cell": edge, "input": "2"})).unwrap(); + let e = sess.execute("home.insertCells", json!({"range": "A1", "shift": shift})).unwrap_err(); + assert!(e.to_string().contains("loss of data"), "{e}"); + assert_eq!(v(&sess, "A1"), Value::Number(1.0)); + assert_eq!(v(&sess, edge), Value::Number(2.0)); + // A value that still fits moves to the last valid cell. + let mut sess = s(); + sess.execute("cell.set", json!({"cell": last_fit, "input": "2"})).unwrap(); + sess.execute("home.insertCells", json!({"range": "A1", "shift": shift})).unwrap(); + assert_eq!(v(&sess, last_ok), Value::Number(2.0)); + } +} + #[test] fn autosum_over_whole_rows_is_one_undo_step_within_the_data() { // Found by the command-sequence fuzzer: AutoSum over rows `3:5` ran one `cell.set` per column From 561299bfad8bb3c442796f22bf9c233dd3cf4594 Mon Sep 17 00:00:00 2001 From: jonathanwxh-cell Date: Mon, 12 Oct 2026 22:59:53 +0800 Subject: [PATCH 02/17] sheet-navigation: keep boundary navigation on visible worksheets --- ROADMAP.md | 4 +- crates/engine/src/cmd/sheet.rs | 17 ++-- .../engine/tests/hidden_sheet_navigation.rs | 78 +++++++++++++++++++ docs/gaps.md | 5 +- docs/ui-parity.md | 5 +- 5 files changed, 97 insertions(+), 12 deletions(-) create mode 100644 crates/engine/tests/hidden_sheet_navigation.rs diff --git a/ROADMAP.md b/ROADMAP.md index fbe48d35d..2b4f5dc76 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -2,7 +2,7 @@ **Stage: alpha** · next: beta, ~25 points of "ready for real work" and ~160–240 h away -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (password-encrypted workbooks open, #11) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (sheet navigation stays on visible worksheets at boundaries) · **Target:** Microsoft Excel (Microsoft 365) GridCraft aims at full Microsoft Excel parity, and at being better in places: faster, open (XLSX is the native format), fully scriptable by agents (MCP, control channel, CLI), and on the web. @@ -132,6 +132,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | What landed | |---|---| +| 2026-10-12 | Next/previous sheet navigation stops at the first/last visible worksheet instead of activating a hidden boundary sheet; the current selection is retained | | 2026-10-11 | Command-sequence fuzzing: random command sequences checked against undo/redo, XLSX round-trip and full recalculation; fixes for the table, legacy-array, spill, AutoSum, Watch Window and sheet-copy bugs it found | | 2026-10-11 | Password-protected workbooks open: GridCraft asks for the password and decrypts Agile and Standard encryption (#11) | | 2026-10-11 | Text boxes keep their formatting: run size, bold, italic, typeface, text colour and outline, alignment, vertical anchor and "no outline" (#377) | @@ -177,6 +178,7 @@ Ranked; estimates in Opus 5.5 agent-hours. Detail and the full beta-gate list in | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | Progress log: hidden-sheet navigation at workbook boundaries | | 2026-10-11 | minor | Password-encrypted workbooks open (#11); progress log row | | 2026-10-11 | minor | Forecast Sheet (#322); progress log row | | 2026-10-11 | minor | Worksheet functions 505 / 523 (ODD* bond functions); progress log row | diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index f05b77d91..590d129ad 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -890,17 +890,18 @@ fn activate(s: &mut Session, p: &Json) -> Result { Ok(json!({"sheet": i, "name": d.wb.active().map(|s| s.name.clone())})) } -fn step_sheet(s: &mut Session, dir: i64) -> Result { +fn step_sheet(s: &mut Session, dir: isize) -> Result { let d = s.doc()?; - let n = d.wb.sheets.len() as i64; - let mut i = d.wb.active_sheet as i64; - for _ in 0..n { - i = (i + dir).clamp(0, n - 1); - if d.wb.sheet(i as usize).is_some_and(|s| s.visibility == Visibility::Visible) { - break; + let mut i = d.wb.active_sheet; + while let Some(next) = i.checked_add_signed(dir) { + let Some(sheet) = d.wb.sheet(next) else { break }; + if sheet.visibility == Visibility::Visible { + s.doc_mut()?.wb_switch_sheet(next); + return ok(); } + i = next; } - s.doc_mut()?.wb_switch_sheet(i as usize); + // No visible sheet in this direction: keep the current sheet and its selection. ok() } diff --git a/crates/engine/tests/hidden_sheet_navigation.rs b/crates/engine/tests/hidden_sheet_navigation.rs new file mode 100644 index 000000000..f457f4bdb --- /dev/null +++ b/crates/engine/tests/hidden_sheet_navigation.rs @@ -0,0 +1,78 @@ +//! Sheet navigation must never activate hidden sheets when it reaches a workbook boundary. + +use std::sync::Arc; + +use gridcraft_engine::{Session, model::Visibility}; +use serde_json::json; + +fn workbook(hidden: &[usize], active: usize) -> Session { + let mut session = Session::new(); + session.new_workbook(); + for name in ["Second", "Third", "Fourth"] { + session.execute("home.insertSheet", json!({"name": name})).unwrap(); + } + for sheet in hidden { + session.execute("sheet.hide", json!({"sheet": sheet})).unwrap(); + } + session.execute("sheet.activate", json!({"sheet": active})).unwrap(); + session.execute("selection.set", json!({"cell": "C7"})).unwrap(); + session +} + +#[test] +fn next_stays_on_the_last_visible_sheet() { + let mut session = workbook(&[2, 3], 1); + for _ in 0..2 { + session.execute("sheet.next", json!({})).unwrap(); + let doc = session.active().unwrap(); + assert_eq!(doc.wb.active_sheet, 1); + assert_eq!(doc.selection.active.a1(), "C7"); + } +} + +#[test] +fn previous_stays_on_the_first_visible_sheet() { + let mut session = workbook(&[0, 1], 2); + for _ in 0..2 { + session.execute("sheet.previous", json!({})).unwrap(); + let doc = session.active().unwrap(); + assert_eq!(doc.wb.active_sheet, 2); + assert_eq!(doc.selection.active.a1(), "C7"); + } +} + +#[test] +fn navigation_skips_hidden_sheets_in_both_directions() { + let mut session = workbook(&[1, 2], 0); + session.execute("sheet.next", json!({})).unwrap(); + assert_eq!(session.active().unwrap().wb.active_sheet, 3); + session.execute("sheet.previous", json!({})).unwrap(); + let doc = session.active().unwrap(); + assert_eq!(doc.wb.active_sheet, 0); + assert_eq!(doc.selection.active.a1(), "C7"); +} + +#[test] +fn navigation_with_one_visible_sheet_is_a_noop() { + let mut session = workbook(&[0, 2, 3], 1); + for command in ["sheet.previous", "sheet.next"] { + session.execute(command, json!({})).unwrap(); + let doc = session.active().unwrap(); + assert_eq!(doc.wb.active_sheet, 1); + assert_eq!(doc.selection.active.a1(), "C7"); + } +} + +#[test] +fn very_hidden_sheets_are_skipped_at_interior_and_boundary_positions() { + let mut session = workbook(&[1, 3], 0); + // VeryHidden is an imported XLSX state; sheet.hide only creates Hidden sheets. + let wb = Arc::make_mut(&mut session.active_mut().unwrap().wb); + for index in [1, 3] { + Arc::make_mut(&mut wb.sheets[index]).visibility = Visibility::VeryHidden; + } + for (command, expected) in [("sheet.next", 2), ("sheet.next", 2), ("sheet.previous", 0)] { + session.execute(command, json!({})).unwrap(); + assert_eq!(session.active().unwrap().wb.active_sheet, expected); + } +} diff --git a/docs/gaps.md b/docs/gaps.md index 09870ba27..c8075f8e2 100644 --- a/docs/gaps.md +++ b/docs/gaps.md @@ -1,6 +1,6 @@ # Gaps -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-11 · **Change:** minor (§6: iterative calculation landed) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (§9: hidden-sheet navigation at workbook boundaries fixed) · **Target:** Microsoft Excel (Microsoft 365) Every known shortfall against Excel, one entry each, ranked by user impact (what blocks someone using GridCraft for real work first). This is the work list: pick from the top, check the open @@ -84,6 +84,8 @@ No alpha blockers: every core workflow passes the [alpha gate](roadmap.md#alpha- - **Estimate:** 20–30 h for beta (25–40 h full). ### 9. Keyboard-first users: key tips, shortcuts, context menus (U) +- Next/previous sheet navigation now skips Hidden and VeryHidden worksheets without activating a + hidden sheet at the workbook boundary. Other keyboard gaps below remain. - **Evidence:** #33, #49 (key tips: initial Home-tab support landed in #43 on 10-10, the rest of the ribbon not yet), #110, #28 (selection), 59 command shortcuts vs Excel's 200+. - **Estimate:** 12–20 h. [ui-parity.md](ui-parity.md) @@ -160,6 +162,7 @@ No alpha blockers: every core workflow passes the [alpha gate](roadmap.md#alpha- | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | §9: next/previous sheet commands retain the current visible sheet and selection when there is no visible destination | | 2026-10-11 | minor | §6: iterative calculation for circular references landed (Maximum Iterations, Maximum Change) | | 2026-10-11 | minor | §15: trim-reference operators (`A1:.E10`, `A1.:E10`, `A1.:.E10`) parse and evaluate | | 2026-10-11 | minor | §4: trendlines (six types, forecast, equation and R²) read, written and drawn | diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 10abcbccc..a8b92655e 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-11 · **Change:** minor (AutoFit reserves filter dropdown space, observed against Windows Excel) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-12 · **Change:** minor (next/previous sheet navigation stops at visible-sheet boundaries) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Interaction, controls and feel, against Excel. Part of [target-app-parity.md](target-app-parity.md). **UI/UX fidelity: ~55% ready, 50–80 h** (estimated). Ribbon command coverage is measured @@ -21,7 +21,7 @@ Sources: `crates/ui-egui/src` (ribbon, grid, editor, formula bar, dialogs, panes | Ribbon collapse, simplified ribbon, customize ribbon/QAT | yes | partial | customization missing | | Key tips (Alt / F10 then letters) | yes, every ribbon control | **initial** (#43, 10-10): Alt or F10 shows key tips on Home for alignment, Paste Special and AutoFit plus legacy Alt sequences; other tabs and controls not yet (#33, #49) | 3–5 h | | Formula bar with Name Box, expand, function hints | yes | yes | done | -| Sheet tabs: rename, reorder, colour, hide, scroll | yes | yes | rename focus bug on macOS (#145) | +| Sheet tabs: rename, reorder, colour, hide, scroll | yes | yes | next/previous skips Hidden and VeryHidden sheets and retains the current sheet and selection when no visible destination exists; rename focus bug on macOS (#145) | | Status bar: Sum/Count/Average and more, view buttons, zoom | yes | yes | customizable statistics partial | | Task panes (Format Chart, Comments, Watch, Selection, PivotTable fields) | yes | yes | element-level Format pane for shapes and pictures missing | | Backstage (File) view: Info, New with templates, Open recent, Export, Options | yes | partial | Options dialog and template gallery missing | @@ -160,6 +160,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-12 | minor | Next/previous sheet navigation no longer activates hidden worksheets at either workbook boundary; command tests cover Hidden, VeryHidden and selection retention | | 2026-10-11 | minor | AutoFit accounts for table and ordinary filter dropdowns, including low zoom; header text excludes the button area | | 2026-10-11 | minor | Outline band for grouped rows and columns, Show / Hide Detail on the Data tab, numpad navigation keys in shortcuts (#373) | | 2026-10-11 | minor | Shift+Space / Ctrl+Space no longer start an edit; Control+Space selects the column on Mac (#338) | From f644004cc66392a1c94854aa1d587758c95aa628 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 10:35:14 +0200 Subject: [PATCH 03/17] Sheet tabs: New sheet (+) right after the last tab, the strip capped at 60% (part of #943) --- crates/ui-egui/src/tabs.rs | 5 ++-- crates/ui-egui/tests/sheet_tabs.rs | 46 +++++++++++++++++++++++++++++- docs/ui-parity.md | 5 ++-- 3 files changed, 51 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/tabs.rs b/crates/ui-egui/src/tabs.rs index b77524bb9..a2b2ac818 100644 --- a/crates/ui-egui/src/tabs.rs +++ b/crates/ui-egui/src/tabs.rs @@ -69,7 +69,8 @@ pub fn sheet_tabs(app: &mut SheetApp, ui: &mut Ui) { .map(|(i, s)| (i, s.name.clone(), s.tab_color.and_then(|c| c.resolve(&d.wb.theme)), i == d.wb.active_sheet)) .collect(); // Keep worksheet tabs in their own horizontally scrollable viewport. As in Excel, the - // tabs get 60% of the bar and the grid's horizontal scrollbar the rest. + // tabs get up to 60% of the bar and the grid's horizontal scrollbar the rest; a strip + // that needs less shrinks to its tabs, so New sheet (+) sits right after the last one. let tabs_width = (ui.available_width() * 0.6).max(100.0); // Reveal the selected sheet after navigation or layout changes, but // allow browsing other tabs without snapping back on every frame. @@ -94,7 +95,7 @@ pub fn sheet_tabs(app: &mut SheetApp, ui: &mut Ui) { egui::ScrollArea::horizontal() .id_salt("worksheet_tabs_horizontal") - .auto_shrink([false, false]) + .auto_shrink([true, false]) .scroll_bar_visibility(egui::scroll_area::ScrollBarVisibility::AlwaysHidden) .show(ui, |ui| { if wheel_scroll.abs() > 0.0 { diff --git a/crates/ui-egui/tests/sheet_tabs.rs b/crates/ui-egui/tests/sheet_tabs.rs index 7b43cd12b..b36adf3dd 100644 --- a/crates/ui-egui/tests/sheet_tabs.rs +++ b/crates/ui-egui/tests/sheet_tabs.rs @@ -7,10 +7,14 @@ use serde_json::json; type Harness = egui_kittest::Harness<'static, SheetApp>; fn harness(width: f32, active: usize) -> Harness { + harness_with(width, 30, active) +} + +fn harness_with(width: f32, sheets: usize, active: usize) -> Harness { let mut s = Session::new(); s.new_workbook(); s.execute("sheet.rename", json!({"sheet":0,"name":"Tab00"})).unwrap(); - for i in 1..30 { + for i in 1..sheets { s.execute("home.insertSheet", json!({"name":format!("Tab{i:02}")})).unwrap(); } s.execute("sheet.activate", json!({"sheet":active})).unwrap(); @@ -162,3 +166,43 @@ fn overflow_leaves_the_worksheet_scrollbar_reachable() { assert!(track.left() >= 0.0 && track.right() <= width + 0.5, "{width}: the scrollbar is offscreen: {track:?}"); } } + +/// The New sheet button's left edge. The scrollbar track starts 8 px after the 26 px button. +fn new_sheet_left(h: &Harness) -> f32 { + h.state().grid.hscroll.expect("the tab bar has a horizontal scrollbar").left() - 8.0 - 26.0 +} + +/// The painted background of the active tab: the smallest fill enclosing its name (the bar's own fill encloses it too). +fn active_tab_rect(h: &Harness, name: &str) -> Rect { + let (text, _) = tab(h, name).unwrap(); + h.output() + .shapes + .iter() + .filter_map(|s| match &s.shape { + Shape::Rect(r) if r.rect.contains_rect(text) => Some(r.rect), + _ => None, + }) + .min_by(|a, b| a.area().total_cmp(&b.area())) + .unwrap_or_else(|| panic!("{name} has no painted tab background")) +} + +#[test] +fn new_sheet_button_follows_the_last_tab() { + for width in [700.0, 1200.0] { + let h = harness_with(width, 2, 1); + let last = active_tab_rect(&h, "Tab01"); + let plus = new_sheet_left(&h); + assert!(plus >= last.right() && plus - last.right() <= 12.0, "{width}: + at {plus}, last tab ends at {}", last.right()); + } +} + +#[test] +fn overflowing_tabs_keep_their_share_and_the_new_sheet_button_after_it() { + for width in [700.0, 1200.0] { + let h = harness(width, 0); + let (_, viewport) = tab(&h, "Tab00").unwrap(); + assert!(viewport.width() <= width * 0.6, "{width}: the tab strip grew past its share: {viewport:?}"); + let plus = new_sheet_left(&h); + assert!(plus >= viewport.right() && plus - viewport.right() <= 12.0, "{width}: + at {plus}, strip ends at {}", viewport.right()); + } +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 22e221e69..3d17238c5 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (horizontal scrollbar in the sheet tab bar, #840) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (New sheet (+) right after the last sheet tab, #943) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Interaction, controls and feel, against Excel. Part of [target-app-parity.md](target-app-parity.md). **UI/UX fidelity: ~55% ready, 50–80 h** (estimated). Ribbon command coverage is measured @@ -21,7 +21,7 @@ Sources: `crates/ui-egui/src` (ribbon, grid, editor, formula bar, dialogs, panes | Ribbon collapse, simplified ribbon, customize ribbon/QAT | yes | partial | customization missing | | Key tips (Alt / F10 then letters) | yes, every ribbon control | **initial** (#43, 10-10): Alt or F10 shows key tips on Home for alignment, Paste Special and AutoFit plus legacy Alt sequences; other tabs and controls not yet (#33, #49) | 3–5 h | | Formula bar with Name Box, expand, function hints | yes | yes | done | -| Sheet tabs: rename, reorder, colour, hide, scroll | yes | yes | active tab revealed on navigation and resize (#681); manual wheel/trackpad browsing preserved; rename focus bug on macOS (#145) | +| Sheet tabs: rename, reorder, colour, hide, scroll | yes | yes | active tab revealed on navigation and resize (#681); New sheet (+) right after the last tab, the strip capped at 60% of the bar (#943); manual wheel/trackpad browsing preserved; rename focus bug on macOS (#145) | | Status bar: Sum/Count/Average and more, view buttons, zoom | yes | yes | customizable statistics partial | | Task panes (Format Chart, Comments, Watch, Selection, PivotTable fields) | yes | yes | element-level Format pane for shapes and pictures missing | | Backstage (File) view: Info, New with templates, Open recent, Export, Options | yes | partial | Options dialog and template gallery missing | @@ -161,6 +161,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-13 | minor | Sheet tab bar: the tab strip shrinks to its tabs, up to 60% of the bar, so New sheet (+) sits right after the last tab and the horizontal scrollbar takes the rest (part of #943) | | 2026-10-12 | minor | The File view's recent list tells when each workbook was last changed, read off the UI thread | | 2026-10-13 | minor | Horizontal scrollbar in the sheet tab bar: drag, track click pages, frozen columns excluded from the page (#840) | | 2026-10-12 | minor | Ctrl+wheel zoom steps along a ladder of percentages about 20% apart that never skips 100%, gliding to each step with animations on, so a notch out undoes a notch in (it drifted: 100 → 122 → 101); Excel steps 15 points, see decisions.md | From 71437f23b1556f404ae9ae8975fb40080c712c98 Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Tue, 13 Oct 2026 18:03:08 +0200 Subject: [PATCH 04/17] Fix table without header: re-point references when the data moves down (#1030) --- crates/engine/src/cmd/insert.rs | 41 +++++++++++++++++++++++++++------ crates/engine/src/cmd/sheet.rs | 4 ++-- 2 files changed, 36 insertions(+), 9 deletions(-) diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index d878e4469..b195a4954 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -2,9 +2,11 @@ //! conditional formatting (Home › Styles) and symbols. use gridcraft_core::{CellRef, MAX_ROWS, RangeRef}; +use gridcraft_formula::adjust::{Axis, Edit}; use gridcraft_model::*; use serde_json::{Value as Json, json}; +use super::sheet::{rewrite, shift_cell_features}; use super::*; use crate::selection::current_region; @@ -276,13 +278,13 @@ fn insert_table(s: &mut Session, p: &Json) -> Result { return Err(EngineError::Other("To prevent possible loss of data, cells can't be pushed off the end of the worksheet.".into())); } sh.cells.shift_rows_in_cols(r.start.col, r.end.col, r.start.row, 1); - sh.cell_pictures = std::mem::take(&mut sh.cell_pictures) - .into_iter() - .filter_map(|(c, v)| { - let dest = if c.col >= r.start.col && c.col <= r.end.col && c.row >= r.start.row { c.offset(1, 0) } else { Some(c) }; - dest.map(|c| (c, v)) - }) - .collect(); + // Like Insert Cells > Shift down: move merges, conditional formats, notes, links and pictures, and re-point every formula + // reference (on any sheet) at the cells that moved. + let e = Edit::InsertCells { axis: Axis::Rows, range: RangeRef::new(r.start, CellRef::new(r.start.row, r.end.col)) }; + shift_cell_features(sh, &e); + let name = sh.name.clone(); + rewrite(&mut cx.wb, &name, &e); + let sh = cx.sheet_mut(sheet)?; for (i, c) in (r.start.col..=r.end.col).enumerate() { sh.set_value(CellRef::new(r.start.row, c), gridcraft_core::Value::text(format!("Column{}", i + 1))); } @@ -1553,4 +1555,29 @@ mod tests { let rules = s.execute("home.manageRules", json!({})).unwrap(); assert_eq!(rules["rules"].as_array().map(Vec::len), Some(0)); } + + #[test] + fn table_without_header_keeps_outside_references() { + // #1030: the data moved down for the new header row, but formulas outside kept the old addresses. + let mut s = Session::new(); + s.new_workbook(); + s.execute("home.insertSheet", json!({"name": "Two"})).unwrap(); + s.execute("sheet.activate", json!({"sheet": 0})).unwrap(); + s.execute("cell.set", json!({"cell": "A1", "input": "10"})).unwrap(); + s.execute("cell.set", json!({"cell": "A2", "input": "20"})).unwrap(); + s.execute("cell.set", json!({"cell": "B1", "input": "=SUM(A1:A2)"})).unwrap(); + s.execute("cell.set", json!({"cell": "C1", "input": "=A2"})).unwrap(); + s.execute("cell.set", json!({"sheet": "Two", "cell": "A1", "input": "=Sheet1!A2"})).unwrap(); + s.execute("insert.table", json!({"range": "A1:A2", "header": false})).unwrap(); + let get = |s: &mut Session, a: &str| s.execute("cell.get", json!({"cell": a})).unwrap(); + assert_eq!(get(&mut s, "A1")["value"], "Column1"); + assert_eq!(get(&mut s, "A2")["value"], json!(10.0)); + assert_eq!(get(&mut s, "B1")["formula"], "=SUM(A2:A3)"); + assert_eq!(get(&mut s, "B1")["value"], json!(30.0)); + assert_eq!(get(&mut s, "C1")["formula"], "=A3"); + assert_eq!(get(&mut s, "C1")["value"], json!(20.0)); + let other = s.execute("cell.get", json!({"sheet": "Two", "cell": "A1"})).unwrap(); + assert_eq!(other["formula"], "=Sheet1!A3"); + assert_eq!(other["value"], json!(20.0)); + } } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 7870dee4e..d5d817f41 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -214,7 +214,7 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: } /// Rewrites every formula reference (and chart series) for a structural edit on `target`. -fn rewrite(wb: &mut gridcraft_model::Workbook, target: &str, e: &Edit) { +pub(super) fn rewrite(wb: &mut gridcraft_model::Workbook, target: &str, e: &Edit) { super::edit::rewrite_all_formulas(wb, target, e); for si in 0..wb.sheets.len() { let host = wb.sheets.get(si).map(|s| s.name.clone()).unwrap_or_default(); @@ -963,7 +963,7 @@ pub(crate) fn map_range(r: RangeRef, e: &Edit) -> Option { /// Shifts what Insert/Delete Cells moves along with the cells: merged areas, conditional /// format, validation, table, filter and print-area ranges, notes, links and sparklines. -fn shift_cell_features(sh: &mut Sheet, e: &Edit) { +pub(super) fn shift_cell_features(sh: &mut Sheet, e: &Edit) { let map_cell = |c: CellRef| map_range(RangeRef::cell(c), e).map(|r| r.start); sh.merges = sh.merges.iter().filter_map(|m| map_range(*m, e)).filter(|m| !m.is_single()).collect(); for cf in sh.cond_formats.iter_mut() { From 2cae3a22b16b65ef0b1c4ad68d9c254ad9a737fe Mon Sep 17 00:00:00 2001 From: dev-exe <29832920+dev-exe@users.noreply.github.com> Date: Wed, 14 Oct 2026 20:28:46 +0530 Subject: [PATCH 05/17] Slicers: a table named in `table` is found on any sheet insert.slicer and table.insertSlicer looked for the table named by `table` only on the active sheet. With another sheet active, the call failed with "Select a cell inside a table first, or pass `table`" although `table` was passed. Other table commands find a table by name in the whole workbook (Workbook::table). The named table is now found on any sheet, and the slicer goes on that table's sheet. `at` is read on that sheet, so "Sheet1!G2" names the table's own sheet. A name that matches no table gives "no table named `X`". Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/slicer.rs | 18 ++++++++++++++---- crates/engine/src/tests.rs | 29 +++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 4 deletions(-) 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 ae499a189..2e46b56ff 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -1206,6 +1206,35 @@ fn table_rename_keeps_a_defined_name_with_the_same_name() { assert_eq!((v(&s, "E1"), v(&s, "F1")), (Value::Number(100.0), Value::Number(7.0))); } +#[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(); From 1b4df8acc2ae7c0f4ebc6f98baa3570ea747a25c Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Thu, 15 Oct 2026 06:22:02 +0200 Subject: [PATCH 06/17] Refuse table names that clash with defined names or R1C1 references Found while testing crates/engine/src/cmd/insert.rs:477 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/formulas.rs | 2 +- crates/engine/src/cmd/insert.rs | 25 ++++++++++++++++++++++++- 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/formulas.rs b/crates/engine/src/cmd/formulas.rs index 40df72df0..eca08a093 100644 --- a/crates/engine/src/cmd/formulas.rs +++ b/crates/engine/src/cmd/formulas.rs @@ -237,7 +237,7 @@ fn valid_name(n: &str) -> bool { /// Looks like an R1C1 reference (`R`, `C`, `RC`, `R2`, `C3`, `R1C1`), which Excel doesn't /// accept as a name. -fn is_r1c1(n: &str) -> bool { +pub(crate) fn is_r1c1(n: &str) -> bool { let digits = |s: &str| s.bytes().take_while(u8::is_ascii_digit).count(); let mut rest = n; if let Some(r) = rest.strip_prefix(['R', 'r']) { diff --git a/crates/engine/src/cmd/insert.rs b/crates/engine/src/cmd/insert.rs index d878e4469..5e84b292b 100644 --- a/crates/engine/src/cmd/insert.rs +++ b/crates/engine/src/cmd/insert.rs @@ -471,6 +471,7 @@ fn table_style(s: &mut Session, p: &Json) -> Result { fn valid_table_name(name: &str) -> bool { name.starts_with(|c: char| c.is_alphabetic() || c == '_' || c == '\\') && CellRef::parse(name).is_none() + && !super::formulas::is_r1c1(name) && name.chars().enumerate().all(|(i, c)| c.is_alphanumeric() || c == '_' || c == '.' || (i == 0 && c == '\\')) } @@ -480,7 +481,8 @@ fn table_rename(s: &mut Session, p: &Json) -> Result { if !valid_table_name(&name) { return Err(EngineError::Other("The name that you entered is not valid.".into())); } - if s.doc()?.wb.table(&name).is_some_and(|x| x != (si, ti)) { + let wb = &s.doc()?.wb; + if wb.table(&name).is_some_and(|x| x != (si, ti)) || wb.names.iter().any(|n| n.name.eq_ignore_ascii_case(&name)) { return Err(EngineError::Other("The name entered already exists. Enter a unique name.".into())); } edit(s, |cx| { @@ -1553,4 +1555,25 @@ mod tests { let rules = s.execute("home.manageRules", json!({})).unwrap(); assert_eq!(rules["rules"].as_array().map(Vec::len), Some(0)); } + + #[test] + fn table_names_refuse_defined_names_and_r1c1() { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1:A2", "values": [["h"], ["1"]]})).unwrap(); + s.execute("formulas.defineName", json!({"name": "Rate", "refersTo": "=0.2"})).unwrap(); + s.execute("insert.table", json!({"range": "A1:A2"})).unwrap(); + let e = s.execute("table.rename", json!({"table": "Table1", "name": "Rate"})).unwrap_err(); + assert!(e.to_string().contains("already exists"), "{e}"); + let e = s.execute("table.rename", json!({"table": "Table1", "name": "rate"})).unwrap_err(); + assert!(e.to_string().contains("already exists"), "{e}"); + for n in ["R2C3", "c", "r", "RC", "C5"] { + let e = s.execute("table.rename", json!({"table": "Table1", "name": n})).unwrap_err(); + assert!(e.to_string().contains("not valid"), "{n}: {e}"); + let e = s.execute("insert.table", json!({"range": "C1:C2", "name": n})).unwrap_err(); + assert!(e.to_string().contains("not valid"), "{n}: {e}"); + } + s.execute("table.rename", json!({"table": "Table1", "name": "Table1"})).unwrap(); + s.execute("table.rename", json!({"table": "Table1", "name": "Rates"})).unwrap(); + } } From 7deb382f3d6e3828cebb7224f8ead4fad4f7e3d5 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:22:31 +0300 Subject: [PATCH 07/17] xlsx: keep a table's filter criteria when the sheet AutoFilter coincides with it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The engine's live filter state is always sheet.autofilter — apply_filter, filter_values, clear_filter, reapply and the slicer command all read and write it, and a table's header dropdown installs one over the table's own range — while Table::filter is only ever filled by the reader. Writing suppressed the sheet whenever any table merely intersected it and wrote the table part from Table::filter, which was empty: a filtered table saved with its rows still hidden and no criteria anywhere, so on reopen there was no filter to clear and Excel showed an inactive filter over mysteriously hidden rows. A slicer's selection was lost the same way. Fold the coinciding sheet filter's criteria into the table part (colId offsets are relative to the range's first column, and both ranges start at the same column, so they carry over verbatim); a filter that only partially covers a table still has nowhere valid to go and keeps being suppressed. Regression test: a_table_filter_criteria_survive_a_round_trip — on main the table part carries no criteria after a save; with the fold they round trip, the hidden row stays, and the sheet keeps no duplicate autoFilter. --- crates/xlsx/src/sheet_write.rs | 25 +++++++++++-- crates/xlsx/src/tests/roundtrip.rs | 57 ++++++++++++++++++++++++++++++ 2 files changed, 79 insertions(+), 3 deletions(-) diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 78e1afaae..98946f6ac 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -76,10 +76,29 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( let nstyles = wb.styles.len() as u32; let sid = |s: gridcraft_model::StyleId| if s.0 < nstyles { s.0 } else { 0 }; + // A table's dropdown filter is kept in `sheet.autofilter`, not in `Table::filter` (the + // engine's only live filter state, which the reader also fills). Where the two coincide, + // fold the sheet filter's criteria into the table so they reach the file: the stored + // `colId` offsets are relative to the range's first column, and both ranges start at the + // same column, so they carry over verbatim. A filter that only partially covers a table + // has nowhere valid to go, so it keeps being suppressed below. + let folded: Vec = match &sheet.autofilter { + Some(af) if !af.criteria.is_empty() => sheet + .tables + .iter() + .cloned() + .map(|mut t| { + if t.header_row && t.filter_button && t.filter.is_empty() && t.range.contains_range(&af.range) { + t.filter = af.criteria.clone(); + } + t + }) + .collect(), + _ => sheet.tables.clone(), + }; // Table header cells must hold the column names. let mut overrides: BTreeMap = BTreeMap::new(); - let tables: Vec<(&Table, Vec)> = sheet - .tables + let tables: Vec<(&Table, Vec)> = folded .iter() .filter(|t| t.range.height() > t.header_row as u32 + t.totals_row as u32 || (t.header_row && t.range.height() > t.totals_row as u32)) .map(|t| (t, table_columns(t))) @@ -341,7 +360,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( s.push_str(&protected_ranges_xml(sheet)); // autoFilter (a sheet filter overlapping a table would be invalid). if let Some(af) = &sheet.autofilter - && !sheet.tables.iter().any(|t| t.range.intersects(&af.range)) + && !folded.iter().any(|t| t.range.contains_range(&af.range) && t.header_row && t.filter_button) { s.push_str(&autofilter_xml(af.range, &af.criteria, out)); } diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 901802905..b2789a3dd 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -1164,3 +1164,60 @@ fn doc_props_keep_boundary_whitespace() { let (back, _) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); assert_eq!(back.props, wb.props); } + +/// A table's dropdown filter lives in `sheet.autofilter` (the engine's only live filter state), +/// not in `Table::filter`. Writing used to suppress the sheet `` because a table +/// intersects it and write the table part from `Table::filter`, which was empty: the criteria +/// were lost on every save, leaving the rows hidden with no filter to clear on reopen. +#[test] +fn a_table_filter_criteria_survive_a_round_trip() { + let mut wb = Workbook::new(); + let s = wb.sheet_mut(0).unwrap(); + for (r, v) in [(0u32, "Region"), (1, "East"), (2, "West"), (3, "East")] { + s.set_value(at(&format!("A{}", r + 1)), Value::text(v)); + } + s.tables.push(Table { + id: 1, + name: "Sales".into(), + range: rr("A1:A4"), + header_row: true, + totals_row: false, + columns: vec![TableColumn { + name: "Region".into(), + totals: TotalsFn::None, + totals_label: None, + formula: None, + unique_name: None, + xml: None, + }], + style: "TableStyleMedium2".into(), + banded_rows: true, + banded_cols: false, + first_col: false, + last_col: false, + filter_button: true, + filter: vec![], + table_type: None, + connection_id: None, + }); + // The dropdown of a table's header cell installs the sheet AutoFilter over the table's + // range, and `data.filterBy` stores its criteria there. + s.autofilter = Some(AutoFilter { + range: rr("A1:A4"), + criteria: vec![(0, FilterCriterion::Values { values: vec!["East".into()], blanks: false })], + }); + s.rows.entry(1).or_default().hidden = true; + + let (back, rep) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); + assert!(rep.warnings.is_empty(), "{:?}", rep.warnings); + let b = back.sheet(0).unwrap(); + let t = b.tables.first().expect("the table survived"); + let (_, crit) = t.filter.first().expect("the table's filter criteria were kept on save"); + assert!( + matches!(crit, FilterCriterion::Values { values, .. } if values == &["East".to_string()]), + "the criteria were dropped: {:?}", + t.filter + ); + assert!(b.is_row_hidden(1), "the row the filter hid is still hidden"); + assert!(b.autofilter.is_none(), "a table's filter belongs to the table part, not the sheet"); +} From 36e45a5c5834075549a2e659e0f6553e04e8dce7 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Fri, 16 Oct 2026 23:35:20 +0200 Subject: [PATCH 08/17] Reject table.resize when the new range overlaps another table Found while testing crates/engine/src/cmd/extra.rs:818-855 (table_resize, overlap check at ~832) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 --- crates/engine/src/cmd/extra.rs | 3 +++ crates/engine/tests/table_resize_overlap.rs | 27 +++++++++++++++++++++ 2 files changed, 30 insertions(+) diff --git a/crates/engine/src/cmd/extra.rs b/crates/engine/src/cmd/extra.rs index 01e2ac6e6..95f201788 100644 --- a/crates/engine/src/cmd/extra.rs +++ b/crates/engine/src/cmd/extra.rs @@ -828,6 +828,9 @@ fn table_resize(s: &mut Session, p: &Json) -> Result { edit(s, |cx| { let sheet = cx.sheet_mut(si)?; let headers: Vec = (r.start.col..=r.end.col).map(|c| sheet.value(CellRef::new(r.start.row, c)).display()).collect(); + if sheet.tables.iter().enumerate().any(|(i, o)| i != ti && o.range.intersects(&r)) { + return Err(EngineError::Other("A table can't overlap another table.".into())); + } let t = sheet.tables.get_mut(ti).ok_or_else(|| bad("table", "gone"))?; if r.start.row != t.range.start.row || !r.intersects(&t.range) { return Err(EngineError::Other( diff --git a/crates/engine/tests/table_resize_overlap.rs b/crates/engine/tests/table_resize_overlap.rs index f8442cc01..436c941a8 100644 --- a/crates/engine/tests/table_resize_overlap.rs +++ b/crates/engine/tests/table_resize_overlap.rs @@ -27,3 +27,30 @@ fn resize_to_overlapping_range_still_works() { s.execute("table.resize", json!({"table":"T", "range":"B1:C2"})).unwrap(); assert_eq!(table_range(&s), "B1:C2"); } + +fn two_tables() -> Session { + let mut s = table(); + s.execute("range.setValues", json!({"range":"D1", "values":[["P", "Q"], ["y", "8"]]})).unwrap(); + s.execute("insert.table", json!({"range":"D1:E2", "header":true, "name":"U"})).unwrap(); + s +} + +fn ranges(s: &Session) -> Vec { + s.doc().unwrap().wb.active().unwrap().tables.iter().map(|t| t.range.a1()).collect() +} + +#[test] +fn resize_over_another_table_is_rejected() { + let mut s = two_tables(); + assert!(s.execute("table.resize", json!({"table":"T", "range":"A1:E2"})).is_err()); + assert_eq!(ranges(&s), ["A1:B2", "D1:E2"]); + assert!(s.execute("table.resize", json!({"table":"U", "range":"A1:E2"})).is_err()); + assert_eq!(ranges(&s), ["A1:B2", "D1:E2"]); +} + +#[test] +fn resize_next_to_another_table_still_works() { + let mut s = two_tables(); + s.execute("table.resize", json!({"table":"T", "range":"A1:C3"})).unwrap(); + assert_eq!(ranges(&s), ["A1:C3", "D1:E2"]); +} From 9e5cdf9e434a141a82d582309934cbb1c276fe9e Mon Sep 17 00:00:00 2001 From: Thiago Date: Fri, 16 Oct 2026 22:22:33 -0300 Subject: [PATCH 09/17] Tables: add the missing Resize Table button (#935) table.resize has existed in the engine since #589/#1706 (overlap checks, tests), and the command catalog already promised it under Table Design > Properties, but no ribbon button ever called it -- the only way to resize a table was scripting the command directly (control channel, CLI, MCP). #935 (no drag handle) assumed this menu path worked; it doesn't. Adds a "Resize Table" button next to the Table Name field (same Properties group the catalog already declares), opening a small dialog pre-filled with the table's current range. Confirming runs table.resize with the typed range, same validation as #589/#1706 (rejects overlap with another table, matching Excel's "A table can't overlap another table."). The drag-handle part of #935 is unaddressed; this only restores the menu/dialog path. Verified manually: created two tables, used the new button to grow one onto the other (rejected), then to an adjacent non-overlapping range (accepted, table intact). --- crates/ui-egui/src/dialogs.rs | 18 ++++++++++++++++++ crates/ui-egui/src/ribbon.rs | 5 +++++ 2 files changed, 23 insertions(+) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..14d9e0a64 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -562,6 +562,24 @@ impl Dialog { defaults, ) } + "tableResize" => { + let (table, range) = app + .session + .active() + .and_then(|d| { + let sh = d.wb.active()?; + let t = sh.table_at(d.selection.active)?; + Some((t.name.clone(), t.range.a1())) + }) + .unwrap_or_default(); + Dialog::form( + "tableResize", + "Resize Table", + "table.resize", + vec![Text { key: "range", label: "New range for the table:" }], + json!({"table": table, "range": range}), + ) + } "dataValidation" => Dialog::form( "dataValidation", "Data Validation", diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 9c6d89eeb..c84a88dbe 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -2282,6 +2282,11 @@ fn table_design(app: &mut SheetApp, ui: &mut Ui) { act(app, "table.rename", json!({"table": tname, "name": name})); } }); + let __k = big_button(ui, Icon::Table, "Resize\nTable", "Resize Table", false); + control_keytip(app, ui, &__k, "dialog:tableResize"); + if __k.clicked() { + app.open_dialog("tableResize", json!({})); + } sep(ui); let __k = big_button(ui, Icon::Duplicates, "Remove\nDuplicates", "Remove Duplicates", false); control_keytip(app, ui, &__k, "data.removeDuplicates"); From 8ca66c5c90a76049ab62861ca8b838a83a75e8f7 Mon Sep 17 00:00:00 2001 From: TheRenegadeDev Date: Fri, 16 Oct 2026 20:42:20 -0500 Subject: [PATCH 10/17] Use the platform clock for UI timing on the web (#1026) --- crates/ui-egui/src/lib.rs | 11 +---------- crates/ui-egui/tests/ui_clock.rs | 10 ++++++++++ docs/ui-parity.md | 3 ++- 3 files changed, 13 insertions(+), 11 deletions(-) create mode 100644 crates/ui-egui/tests/ui_clock.rs diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index f96cdac3c..72a2a5d8f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1243,17 +1243,8 @@ pub fn clean_error(e: &str) -> String { e.to_string() } -#[allow(clippy::disallowed_methods)] // the clock is read only off wasm pub fn now_ms() -> f64 { - #[cfg(not(target_arch = "wasm32"))] - { - use std::time::{SystemTime, UNIX_EPOCH}; - SystemTime::now().duration_since(UNIX_EPOCH).map(|d| d.as_secs_f64() * 1000.0).unwrap_or(0.0) - } - #[cfg(target_arch = "wasm32")] - { - 0.0 - } + gridcraft_engine::core::date::unix_now_ms().unwrap_or(0.0) } pub fn cell(r: u32, c: u32) -> CellRef { diff --git a/crates/ui-egui/tests/ui_clock.rs b/crates/ui-egui/tests/ui_clock.rs new file mode 100644 index 000000000..6eb0dcc00 --- /dev/null +++ b/crates/ui-egui/tests/ui_clock.rs @@ -0,0 +1,10 @@ +use gridcraft_engine::core::date::unix_now_ms; + +#[test] +fn ui_clock_returns_current_unix_milliseconds() { + let before = unix_now_ms().unwrap(); + let now = gridcraft_ui_egui::now_ms(); + let after = unix_now_ms().unwrap(); + assert!(now.is_finite() && now > 0.0); + assert!((before.min(after)..=before.max(after)).contains(&now)); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 22e221e69..cb139e3d2 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (horizontal scrollbar in the sheet tab bar, #840) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-16 · **Change:** minor (shared UI clock uses the browser-aware platform clock, #1026) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Interaction, controls and feel, against Excel. Part of [target-app-parity.md](target-app-parity.md). **UI/UX fidelity: ~55% ready, 50–80 h** (estimated). Ribbon command coverage is measured @@ -161,6 +161,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | The shared UI clock delegates to the core Unix-millisecond clock, which reads Date.now() on wasm, instead of returning zero there (#1026). Native clock contract tested; browser timing remains unverified | | 2026-10-12 | minor | The File view's recent list tells when each workbook was last changed, read off the UI thread | | 2026-10-13 | minor | Horizontal scrollbar in the sheet tab bar: drag, track click pages, frozen columns excluded from the page (#840) | | 2026-10-12 | minor | Ctrl+wheel zoom steps along a ladder of percentages about 20% apart that never skips 100%, gliding to each step with animations on, so a notch out undoes a notch in (it drifted: 100 → 122 → 101); Excel steps 15 points, see decisions.md | From a6aa53cbff43334668c90786e20c1daa79ac3c27 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Sat, 17 Oct 2026 04:04:00 +0200 Subject: [PATCH 11/17] Move PivotTables with their cells on row/column insert and delete Found while testing crates/engine/src/cmd/sheet.rs:115 (shift_sheet_features; it never touches sh.pivots) Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Sonnet 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/pivot.rs | 49 ++++++++++++++++++++++++++++++++++ crates/engine/src/cmd/sheet.rs | 21 +++++++++++++++ 2 files changed, 70 insertions(+) diff --git a/crates/engine/src/cmd/pivot.rs b/crates/engine/src/cmd/pivot.rs index 8a441fa1b..c20bb5a84 100644 --- a/crates/engine/src/cmd/pivot.rs +++ b/crates/engine/src/cmd/pivot.rs @@ -2308,4 +2308,53 @@ mod tests { // 64 Sub items under the collapsed "g" show as one column group: (1 + grand total) x 256 columns fit. s.execute("pivot.addField", json!({"field": "Sub", "area": "columns"})).unwrap(); } + + #[test] + fn inserting_or_deleting_lines_moves_the_pivot_with_its_cells() { + let mut s = sales(); + s.execute("insert.pivotTable", json!({"source": "Sheet1!A1:E13", "destination": "Sheet1!H10", "rows": ["Region"], "values": ["Sales"]})) + .unwrap(); + assert_eq!(pt(&s).last_range.unwrap().a1(), "H10:I14"); + // Rows above the report (and below the source) move the report only. + s.execute("home.insertRows", json!({"rows": "16:17"})).unwrap(); + s.execute("home.insertRows", json!({"rows": "6:7"})).unwrap(); + assert_eq!(text(&s, "H12"), "Row Labels"); + let l = s.execute("pivot.list", json!({})).unwrap(); + assert_eq!(l[0]["anchor"], "H12"); + assert_eq!(l[0]["range"], "H12:I16"); + assert_eq!(l[0]["source"], "Sheet1!A1:E15"); + // A row above the source moves both. + s.execute("home.insertRows", json!({"rows": "1:1"})).unwrap(); + let p = pt(&s); + assert_eq!(p.source, "Sheet1!A2:E16"); + assert_eq!(p.anchor.a1(), "H13"); + s.execute("pivot.refresh", json!({})).unwrap(); + assert_eq!(pt(&s).last_range.unwrap().a1(), "H13:I17"); + assert_eq!(text(&s, "H13"), "Row Labels"); + assert_eq!(val(&s, "I17"), Value::Number(TOTAL)); + // Deleting a column left of the report moves it left. + s.execute("home.deleteColumns", json!({"cols": "F:F"})).unwrap(); + let p = pt(&s); + assert_eq!(p.anchor.a1(), "G13"); + assert_eq!(p.last_range.unwrap().a1(), "G13:H17"); + // Deleting rows above moves it up. + s.execute("home.deleteRows", json!({"rows": "1:1"})).unwrap(); + assert_eq!(pt(&s).source, "Sheet1!A1:E15"); + assert_eq!(pt(&s).anchor.a1(), "G12"); + s.execute("pivot.refresh", json!({})).unwrap(); + assert_eq!(text(&s, "G12"), "Row Labels"); + assert_eq!(val(&s, "H16"), Value::Number(TOTAL)); + } + + #[test] + fn inserting_rows_on_the_source_sheet_moves_another_sheets_pivot_source() { + let mut s = sales(); + s.execute("insert.pivotTable", json!({"source": "Sheet1!A1:E13", "rows": ["Region"], "values": ["Sales"]})).unwrap(); + s.execute("sheet.activate", json!({"sheet": "Sheet1"})).unwrap(); + s.execute("home.insertRows", json!({"rows": "1:2"})).unwrap(); + let l = s.execute("pivot.list", json!({})).unwrap(); + assert_eq!(l[0]["source"], "Sheet1!A3:E15"); + assert_eq!(l[0]["anchor"], "A3"); + s.execute("pivot.refresh", json!({"pivot": "PivotTable1"})).unwrap(); + } } diff --git a/crates/engine/src/cmd/sheet.rs b/crates/engine/src/cmd/sheet.rs index 7870dee4e..7cb82ccf4 100644 --- a/crates/engine/src/cmd/sheet.rs +++ b/crates/engine/src/cmd/sheet.rs @@ -211,6 +211,15 @@ fn shift_sheet_features(sh: &mut Sheet, axis: Axis, at: u32, count: u32, insert: if let Some(pa) = sh.print.print_area { sh.print.print_area = map_range(pa); } + // PivotTables follow their report cells. A deleted anchor line collapses onto the line that + // takes its place; a report deleted entirely has no last area until the next refresh. + for pt in sh.pivots.iter_mut() { + pt.anchor = map_cell(pt.anchor).unwrap_or_else(|| match axis { + Axis::Rows => CellRef::new(at, pt.anchor.col), + Axis::Cols => CellRef::new(pt.anchor.row, at), + }); + pt.last_range = pt.last_range.and_then(map_range); + } } /// Rewrites every formula reference (and chart series) for a structural edit on `target`. @@ -235,6 +244,18 @@ fn rewrite(wb: &mut gridcraft_model::Workbook, target: &str, e: &Edit) { for sp in sh.sparklines.iter_mut() { sp.source = fix(&sp.source); } + // PivotTable sources (a range, table or defined name) are only rewritten when the edit + // moves them, so table and name sources keep their text. + for pt in sh.pivots.iter_mut() { + let body = pt.source.trim().trim_start_matches('='); + if let Ok(expr) = gridcraft_formula::parse(body) { + let before = gridcraft_formula::print(&expr); + let after = gridcraft_formula::print(&gridcraft_formula::adjust::adjust(expr, &host, target, e)); + if after != before { + pt.source = after; + } + } + } } } From 71c1097d5ab8d9da1af2d7e51b07b6542b05ace3 Mon Sep 17 00:00:00 2001 From: TheRenegadeDev Date: Fri, 16 Oct 2026 21:20:34 -0500 Subject: [PATCH 12/17] #1054: clean up web print iframe and blob resources --- apps/gridcraft-web/Cargo.toml | 2 +- apps/gridcraft-web/src/web.rs | 54 ++++++++++++++++++++++++++--------- docs/ui-parity.md | 3 +- 3 files changed, 43 insertions(+), 16 deletions(-) diff --git a/apps/gridcraft-web/Cargo.toml b/apps/gridcraft-web/Cargo.toml index eac6c1354..4856b565d 100644 --- a/apps/gridcraft-web/Cargo.toml +++ b/apps/gridcraft-web/Cargo.toml @@ -26,4 +26,4 @@ rfd = { workspace = true } wasm-bindgen = "0.2" wasm-bindgen-futures = "0.4" js-sys = "0.3" -web-sys = { version = "0.3", features = ["Window", "Document", "Element", "HtmlElement", "HtmlCanvasElement", "HtmlAnchorElement", "HtmlIFrameElement", "Blob", "BlobPropertyBag", "Url", "Location", "CssStyleDeclaration", "Clipboard", "ClipboardItem", "Navigator", "MessageEvent", "UrlSearchParams"] } +web-sys = { version = "0.3", features = ["Window", "Document", "Element", "HtmlElement", "HtmlCanvasElement", "HtmlAnchorElement", "HtmlIFrameElement", "Blob", "BlobPropertyBag", "Url", "Location", "CssStyleDeclaration", "Clipboard", "ClipboardItem", "Navigator", "MessageEvent", "UrlSearchParams", "AddEventListenerOptions"] } diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index c0fcb22da..6efbbaedb 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -224,15 +224,20 @@ fn print_pdf(bytes: &[u8]) -> Result<(), String> { let window = web_sys::window().ok_or("no window")?; let document = window.document().ok_or("no document")?; + let iframe: web_sys::HtmlIFrameElement = document.create_element("iframe").map_err(js)?.dyn_into().map_err(|_| "not an iframe")?; + iframe.style().set_property("display", "none").map_err(js)?; + let body = document.body().ok_or("no body")?; + let parts = js_sys::Array::of1(&js_sys::Uint8Array::from(bytes)); let opts = web_sys::BlobPropertyBag::new(); opts.set_type("application/pdf"); let blob = web_sys::Blob::new_with_u8_array_sequence_and_options(&parts, &opts).map_err(js)?; let url = web_sys::Url::create_object_url_with_blob(&blob).map_err(js)?; - let iframe: web_sys::HtmlIFrameElement = document.create_element("iframe").map_err(js)?.dyn_into().map_err(|_| "not an iframe")?; - iframe.style().set_property("display", "none").map_err(js)?; - document.body().ok_or("no body")?.append_child(&iframe).map_err(js)?; + if let Err(e) = body.append_child(&iframe) { + cleanup_print(&iframe, &url); + return Err(js(e)); + } // `onload` fires once the PDF has actually rendered inside the iframe; // printing before that would show a blank page. Set the handler BEFORE @@ -240,22 +245,44 @@ fn print_pdf(bytes: &[u8]) -> Result<(), String> { let iframe_for_load = iframe.clone(); let url_for_cleanup = url.clone(); let on_load = wasm_bindgen::closure::Closure::once_into_js(move || { - let Some(content_window) = iframe_for_load.content_window() else { return }; - let _ = content_window.print(); - // `afterprint` fires once the print dialog (or print preview) closes, whether the user - // printed or cancelled — the right moment to release the blob URL and drop the iframe. - let iframe_for_cleanup = iframe_for_load.clone(); - let on_after_print = wasm_bindgen::closure::Closure::once_into_js(move || { - web_sys::Url::revoke_object_url(&url_for_cleanup).ok(); - iframe_for_cleanup.remove(); - }); - let _ = content_window.add_event_listener_with_callback("afterprint", on_after_print.unchecked_ref()); + iframe_for_load.set_onload(None); + if let Err(e) = print_loaded_pdf(&iframe_for_load, &url_for_cleanup) { + cleanup_print(&iframe_for_load, &url_for_cleanup); + log::error!("print failed: {e:?}"); + } }); iframe.set_onload(Some(on_load.unchecked_ref())); iframe.set_src(&url); Ok(()) } +fn cleanup_print(iframe: &web_sys::HtmlIFrameElement, url: &str) { + // Both operations are idempotent: print can fail after dispatching `afterprint`. + web_sys::Url::revoke_object_url(url).ok(); + iframe.remove(); +} + +fn print_loaded_pdf(iframe: &web_sys::HtmlIFrameElement, url: &str) -> Result<(), JsValue> { + let content_window = iframe.content_window().ok_or_else(|| JsValue::from_str("no print content window"))?; + let iframe_for_cleanup = iframe.clone(); + let url_for_cleanup = url.to_owned(); + let on_after_print = wasm_bindgen::closure::Closure::once(move || { + cleanup_print(&iframe_for_cleanup, &url_for_cleanup); + }); + // Listen before `print()`, which can dispatch `afterprint` before it returns. + // A one-shot listener also releases the callback when the dialog closes or is cancelled. + let options = web_sys::AddEventListenerOptions::new(); + options.set_once(true); + let callback = on_after_print.as_ref().unchecked_ref(); + content_window.add_event_listener_with_callback_and_add_event_listener_options("afterprint", callback, &options)?; + if let Err(e) = content_window.print() { + let _ = content_window.remove_event_listener_with_callback("afterprint", callback); + return Err(e); + } + let _ = on_after_print.into_js_value(); + Ok(()) +} + fn mime_for(name: &str) -> &'static str { match name.rsplit('.').next().map(str::to_ascii_lowercase).as_deref() { Some("xlsx") => "application/vnd.openxmlformats-officedocument.spreadsheetml.sheet", @@ -274,7 +301,6 @@ fn mime_for(name: &str) -> &'static str { } /// Trigger a browser save dialog or download of `bytes` named after `name`. - fn download(name: &str, bytes: &[u8]) -> Result<(), String> { let js = |e: wasm_bindgen::JsValue| format!("{e:?}"); let window = web_sys::window().ok_or("no window")?; diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 22e221e69..19b026adc 100644 --- a/docs/ui-parity.md +++ b/docs/ui-parity.md @@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-13 · **Change:** minor (horizontal scrollbar in the sheet tab bar, #840) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-16 · **Change:** minor (web print resource cleanup, #1054) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Interaction, controls and feel, against Excel. Part of [target-app-parity.md](target-app-parity.md). **UI/UX fidelity: ~55% ready, 50–80 h** (estimated). Ribbon command coverage is measured @@ -161,6 +161,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | Web printing registers one-shot `afterprint` cleanup before opening the dialog; missing content windows and setup, listener or print failures also release the hidden iframe and PDF blob URL (#1054) | | 2026-10-12 | minor | The File view's recent list tells when each workbook was last changed, read off the UI thread | | 2026-10-13 | minor | Horizontal scrollbar in the sheet tab bar: drag, track click pages, frozen columns excluded from the page (#840) | | 2026-10-12 | minor | Ctrl+wheel zoom steps along a ladder of percentages about 20% apart that never skips 100%, gliding to each step with animations on, so a notch out undoes a notch in (it drifted: 100 → 122 → 101); Excel steps 15 points, see decisions.md | From 2fb880a6d54b2bc980bc40603cfda33f43bdbf4e Mon Sep 17 00:00:00 2001 From: Mario Date: Tue, 20 Oct 2026 09:30:04 +0200 Subject: [PATCH 13/17] Fix WebGL startup on devices with lower GPU limits --- apps/gridcraft-web/src/web.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/apps/gridcraft-web/src/web.rs b/apps/gridcraft-web/src/web.rs index c0fcb22da..971b3b8d2 100644 --- a/apps/gridcraft-web/src/web.rs +++ b/apps/gridcraft-web/src/web.rs @@ -20,6 +20,17 @@ pub fn start() { return; }; let mut options = eframe::WebOptions::default(); + if let eframe::egui_wgpu::WgpuSetup::CreateNew(setup) = &mut options.wgpu_options.wgpu_setup { + let default_descriptor = std::sync::Arc::clone(&setup.device_descriptor); + + setup.device_descriptor = std::sync::Arc::new(move |adapter| { + let mut descriptor = default_descriptor(adapter); + + descriptor.required_limits = descriptor.required_limits.or_worse_values_from(&adapter.limits()); + + descriptor + }); + } if query().contains("webgl") && let eframe::egui_wgpu::WgpuSetup::CreateNew(create) = &mut options.wgpu_options.wgpu_setup { From a9223eabf8fd0e9501ed7d96cc1c9581dc66168e Mon Sep 17 00:00:00 2001 From: Francisco Herrera <14956584+alosslessdev@users.noreply.github.com> Date: Wed, 21 Oct 2026 14:44:47 -0500 Subject: [PATCH 14/17] Table Design: draw the Table Styles gallery in the ribbon (#2148) --- crates/ui-egui/src/ribbon.rs | 39 +++++++++++++++++++++++++++++++----- 1 file changed, 34 insertions(+), 5 deletions(-) diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index a0ab5a339..742ee80cb 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -2268,10 +2268,10 @@ fn automate(app: &mut SheetApp, ui: &mut Ui) { } fn table_design(app: &mut SheetApp, ui: &mut Ui) { - let Some((tname, header, totals, banded, banded_c, first, last, filter)) = app.session.active().and_then(|d| { + let Some((tname, header, totals, banded, banded_c, first, last, filter, style)) = app.session.active().and_then(|d| { let sh = d.wb.active()?; let t = sh.table_at(d.selection.active)?; - Some((t.name.clone(), t.header_row, t.totals_row, t.banded_rows, t.banded_cols, t.first_col, t.last_col, t.filter_button)) + Some((t.name.clone(), t.header_row, t.totals_row, t.banded_rows, t.banded_cols, t.first_col, t.last_col, t.filter_button, t.style.clone())) }) else { return; }; @@ -2325,9 +2325,38 @@ fn table_design(app: &mut SheetApp, ui: &mut Ui) { } }); sep(ui); - let st = big_button(ui, Icon::FormatTable, "Table\nStyles", "Table Styles", true); - anchor_keytip(app, ui, &st, &["table.style"]); - egui::Popup::menu(&st).show(|ui| table_gallery(app, ui, "table.style")); + table_style_strip(app, ui, &tname, &style); +} + +/// The first page of Excel's Table Styles gallery, drawn in the ribbon. The More arrow opens the +/// full gallery (Light, Medium, Dark) as a popup. +const RIBBON_TABLE_STYLES: [&str; 7] = + ["TableStyleLight1", "TableStyleLight2", "TableStyleLight3", "TableStyleLight4", "TableStyleLight5", "TableStyleLight6", "TableStyleLight7"]; + +fn table_style_strip(app: &mut SheetApp, ui: &mut Ui, tname: &str, current: &str) { + let Some(wb) = app.session.active().map(|d| d.wb.clone()) else { return }; + let t = Tokens::get(ui.ctx()); + ui.add_space(12.0); + ui.horizontal(|ui| { + ui.spacing_mut().item_spacing.x = 3.0; + for name in RIBBON_TABLE_STYLES { + let (r, resp) = ui.allocate_exact_size(vec2(52.0, 40.0), Sense::click()); + paint_table_swatch(ui, r, &wb, name); + if resp.hovered() { + ui.painter().rect_stroke(r.expand(1.5), 2.0, Stroke::new(2.0, Color32::from_rgb(0xE8, 0x7D, 0x2B)), StrokeKind::Outside); + } + if name == current { + ui.painter().rect_stroke(r.expand(1.5), 2.0, Stroke::new(2.0, t.accent), StrokeKind::Outside); + } + let resp = resp.on_hover_text(name); + if resp.clicked() { + act(app, "table.style", json!({"table": tname, "style": name})); + } + } + let more = icon_button(ui, Icon::Chevron, t.text_dim, "More Table Styles", vec2(18.0, 40.0)); + anchor_keytip(app, ui, &more, &["table.style"]); + egui::Popup::menu(&more).show(|ui| table_gallery(app, ui, "table.style")); + }); } fn chart_design(app: &mut SheetApp, ui: &mut Ui) { From c3184b74f94cc6299418830c5f4fca67b1eacb75 Mon Sep 17 00:00:00 2001 From: Francisco Herrera <14956584+alosslessdev@users.noreply.github.com> Date: Wed, 21 Oct 2026 15:16:30 -0500 Subject: [PATCH 15/17] Address CodeRabbit review: borrow the workbook and keep both swatch outlines (#2148) --- crates/ui-egui/src/ribbon.rs | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 742ee80cb..a7ca0ad6a 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -2334,24 +2334,32 @@ const RIBBON_TABLE_STYLES: [&str; 7] = ["TableStyleLight1", "TableStyleLight2", "TableStyleLight3", "TableStyleLight4", "TableStyleLight5", "TableStyleLight6", "TableStyleLight7"]; fn table_style_strip(app: &mut SheetApp, ui: &mut Ui, tname: &str, current: &str) { - let Some(wb) = app.session.active().map(|d| d.wb.clone()) else { return }; let t = Tokens::get(ui.ctx()); ui.add_space(12.0); ui.horizontal(|ui| { ui.spacing_mut().item_spacing.x = 3.0; - for name in RIBBON_TABLE_STYLES { - let (r, resp) = ui.allocate_exact_size(vec2(52.0, 40.0), Sense::click()); - paint_table_swatch(ui, r, &wb, name); - if resp.hovered() { - ui.painter().rect_stroke(r.expand(1.5), 2.0, Stroke::new(2.0, Color32::from_rgb(0xE8, 0x7D, 0x2B)), StrokeKind::Outside); - } - if name == current { - ui.painter().rect_stroke(r.expand(1.5), 2.0, Stroke::new(2.0, t.accent), StrokeKind::Outside); - } - let resp = resp.on_hover_text(name); - if resp.clicked() { - act(app, "table.style", json!({"table": tname, "style": name})); + // Borrow the workbook only while painting; the session must be free for `act` afterwards. + let clicked = { + let Some(d) = app.session.active() else { return }; + let wb = &d.wb; + let mut clicked = None; + for name in RIBBON_TABLE_STYLES { + let (r, resp) = ui.allocate_exact_size(vec2(52.0, 40.0), Sense::click()); + paint_table_swatch(ui, r, wb, name); + if name == current { + ui.painter().rect_stroke(r.expand(2.5), 2.0, Stroke::new(2.0, t.accent), StrokeKind::Outside); + } + if resp.hovered() { + ui.painter().rect_stroke(r.expand(1.0), 2.0, Stroke::new(2.0, Color32::from_rgb(0xE8, 0x7D, 0x2B)), StrokeKind::Outside); + } + if resp.on_hover_text(name).clicked() { + clicked = Some(name); + } } + clicked + }; + if let Some(name) = clicked { + act(app, "table.style", json!({"table": tname, "style": name})); } let more = icon_button(ui, Icon::Chevron, t.text_dim, "More Table Styles", vec2(18.0, 40.0)); anchor_keytip(app, ui, &more, &["table.style"]); From 7a016c2de968824d091dfe7a39f5d89f292f7bdb Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 02:35:41 +0000 Subject: [PATCH 16/17] Integration: #1531 fold the sheet filter into a table only when both start at the same column A sheet AutoFilter that starts right of the table's first column is not folded (its colId offsets would hit the wrong column); the sheet autoFilter stays suppressed for any table it intersects, as before. Test added. --- crates/xlsx/src/sheet_write.rs | 9 ++++-- crates/xlsx/src/tests/roundtrip.rs | 48 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 2 deletions(-) diff --git a/crates/xlsx/src/sheet_write.rs b/crates/xlsx/src/sheet_write.rs index 98946f6ac..dad16857b 100644 --- a/crates/xlsx/src/sheet_write.rs +++ b/crates/xlsx/src/sheet_write.rs @@ -88,7 +88,12 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( .iter() .cloned() .map(|mut t| { - if t.header_row && t.filter_button && t.filter.is_empty() && t.range.contains_range(&af.range) { + if t.header_row + && t.filter_button + && t.filter.is_empty() + && t.range.contains_range(&af.range) + && t.range.start.col == af.range.start.col + { t.filter = af.criteria.clone(); } t @@ -360,7 +365,7 @@ pub fn write_sheet(wb: &Workbook, si: usize, selected: bool, out: &mut Out) -> ( s.push_str(&protected_ranges_xml(sheet)); // autoFilter (a sheet filter overlapping a table would be invalid). if let Some(af) = &sheet.autofilter - && !folded.iter().any(|t| t.range.contains_range(&af.range) && t.header_row && t.filter_button) + && !sheet.tables.iter().any(|t| t.range.intersects(&af.range)) { s.push_str(&autofilter_xml(af.range, &af.criteria, out)); } diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index e1105bf1b..81c4240b9 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -1222,3 +1222,51 @@ fn a_table_filter_criteria_survive_a_round_trip() { assert!(b.is_row_hidden(1), "the row the filter hid is still hidden"); assert!(b.autofilter.is_none(), "a table's filter belongs to the table part, not the sheet"); } + +/// Stored `colId` offsets are relative to the filter range's first column, so criteria of a filter +/// that starts right of the table's first column must not be folded into the table part, where +/// they would apply to the wrong column. +#[test] +fn a_partial_sheet_filter_is_not_folded_into_a_table() { + let mut wb = Workbook::new(); + let s = wb.sheet_mut(0).unwrap(); + for (a, v) in [("A1", "Id"), ("B1", "Region"), ("A2", "1"), ("B2", "East"), ("A3", "2"), ("B3", "West")] { + s.set_value(at(a), Value::text(v)); + } + let column = |name: &str| TableColumn { + name: name.into(), + totals: TotalsFn::None, + totals_label: None, + formula: None, + unique_name: None, + xml: None, + }; + s.tables.push(Table { + id: 1, + name: "Sales".into(), + range: rr("A1:B3"), + header_row: true, + totals_row: false, + columns: vec![column("Id"), column("Region")], + style: "TableStyleMedium2".into(), + banded_rows: true, + banded_cols: false, + first_col: false, + last_col: false, + filter_button: true, + filter: vec![], + table_type: None, + connection_id: None, + }); + s.autofilter = Some(AutoFilter { + range: rr("B1:B3"), + criteria: vec![(0, FilterCriterion::Values { values: vec!["East".into()], blanks: false })], + }); + + let (back, rep) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); + assert!(rep.warnings.is_empty(), "{:?}", rep.warnings); + let b = back.sheet(0).unwrap(); + let t = b.tables.first().expect("the table survived"); + assert!(t.filter.is_empty(), "the criteria landed on the wrong table column: {:?}", t.filter); + assert!(b.autofilter.is_none(), "a sheet filter inside a table would be invalid"); +} From 4ac3f2fa64abc4ca4cc74e52bcbd04008f61a345 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 02:45:35 +0000 Subject: [PATCH 17/17] Integration: cargo fmt --- crates/xlsx/src/tests/roundtrip.rs | 37 +++++++----------------------- 1 file changed, 8 insertions(+), 29 deletions(-) diff --git a/crates/xlsx/src/tests/roundtrip.rs b/crates/xlsx/src/tests/roundtrip.rs index 81c4240b9..40f1189b4 100644 --- a/crates/xlsx/src/tests/roundtrip.rs +++ b/crates/xlsx/src/tests/roundtrip.rs @@ -1183,14 +1183,7 @@ fn a_table_filter_criteria_survive_a_round_trip() { range: rr("A1:A4"), header_row: true, totals_row: false, - columns: vec![TableColumn { - name: "Region".into(), - totals: TotalsFn::None, - totals_label: None, - formula: None, - unique_name: None, - xml: None, - }], + columns: vec![TableColumn { name: "Region".into(), totals: TotalsFn::None, totals_label: None, formula: None, unique_name: None, xml: None }], style: "TableStyleMedium2".into(), banded_rows: true, banded_cols: false, @@ -1203,10 +1196,8 @@ fn a_table_filter_criteria_survive_a_round_trip() { }); // The dropdown of a table's header cell installs the sheet AutoFilter over the table's // range, and `data.filterBy` stores its criteria there. - s.autofilter = Some(AutoFilter { - range: rr("A1:A4"), - criteria: vec![(0, FilterCriterion::Values { values: vec!["East".into()], blanks: false })], - }); + s.autofilter = + Some(AutoFilter { range: rr("A1:A4"), criteria: vec![(0, FilterCriterion::Values { values: vec!["East".into()], blanks: false })] }); s.rows.entry(1).or_default().hidden = true; let (back, rep) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); @@ -1214,11 +1205,7 @@ fn a_table_filter_criteria_survive_a_round_trip() { let b = back.sheet(0).unwrap(); let t = b.tables.first().expect("the table survived"); let (_, crit) = t.filter.first().expect("the table's filter criteria were kept on save"); - assert!( - matches!(crit, FilterCriterion::Values { values, .. } if values == &["East".to_string()]), - "the criteria were dropped: {:?}", - t.filter - ); + assert!(matches!(crit, FilterCriterion::Values { values, .. } if values == &["East".to_string()]), "the criteria were dropped: {:?}", t.filter); assert!(b.is_row_hidden(1), "the row the filter hid is still hidden"); assert!(b.autofilter.is_none(), "a table's filter belongs to the table part, not the sheet"); } @@ -1233,14 +1220,8 @@ fn a_partial_sheet_filter_is_not_folded_into_a_table() { for (a, v) in [("A1", "Id"), ("B1", "Region"), ("A2", "1"), ("B2", "East"), ("A3", "2"), ("B3", "West")] { s.set_value(at(a), Value::text(v)); } - let column = |name: &str| TableColumn { - name: name.into(), - totals: TotalsFn::None, - totals_label: None, - formula: None, - unique_name: None, - xml: None, - }; + let column = + |name: &str| TableColumn { name: name.into(), totals: TotalsFn::None, totals_label: None, formula: None, unique_name: None, xml: None }; s.tables.push(Table { id: 1, name: "Sales".into(), @@ -1258,10 +1239,8 @@ fn a_partial_sheet_filter_is_not_folded_into_a_table() { table_type: None, connection_id: None, }); - s.autofilter = Some(AutoFilter { - range: rr("B1:B3"), - criteria: vec![(0, FilterCriterion::Values { values: vec!["East".into()], blanks: false })], - }); + s.autofilter = + Some(AutoFilter { range: rr("B1:B3"), criteria: vec![(0, FilterCriterion::Values { values: vec!["East".into()], blanks: false })] }); let (back, rep) = read_xlsx(&write_xlsx(&wb).unwrap()).unwrap(); assert!(rep.warnings.is_empty(), "{:?}", rep.warnings);