From 76c9f4f2afd5326d321b06268444448a218ac212 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sat, 10 Oct 2026 01:09:43 -0400 Subject: [PATCH 01/20] Build: the ui-egui build script reads CARGO_MANIFEST_DIR when it runs env! fixes the path to contributors/ when the build script is compiled. After a checkout is moved or copied, the compiled script keeps looking in the old location: the About window loses its contributor list, and because the rerun-if-changed path no longer exists, Cargo reruns the script and rebuilds the UI crate on every build. Reading the variable at run time follows the checkout. --- crates/ui-egui/build.rs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/build.rs b/crates/ui-egui/build.rs index f40007fb2..8a06c5932 100644 --- a/crates/ui-egui/build.rs +++ b/crates/ui-egui/build.rs @@ -13,7 +13,10 @@ use std::fmt::Write as _; use serde_json::Value; fn main() { - let dir = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../../contributors"); + // Read when the script runs: `env!` would keep the path from when the script was compiled, + // which goes stale if the checkout moves (and a missing file reruns the script every build). + let manifest = std::env::var_os("CARGO_MANIFEST_DIR").map(std::path::PathBuf::from).unwrap_or_default(); + let dir = manifest.join("../../contributors"); let json = read(&dir.join("contributors.json"), "the About window shows no contributors", |t| { serde_json::from_str::(t).map_err(|e| e.to_string()) }) From 194a1204ab9575a09d8c428571bd35ead28477ed Mon Sep 17 00:00:00 2001 From: MarekSpodBiedry <73122968+MarekSpodBiedry@users.noreply.github.com> Date: Mon, 12 Oct 2026 08:31:43 +0200 Subject: [PATCH 02/20] Open the Header & Footer dialog on the sheet's saved header and footer (#523) --- crates/ui-egui/src/dialogs.rs | 6 +++++- crates/ui-egui/tests/interaction.rs | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 5e859c68e..0560ad8f4 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -368,7 +368,11 @@ impl Dialog { Text { key: "header", label: "Header (&L left, &C center, &R right; &P page, &N pages, &D date, &A sheet):" }, Text { key: "footer", label: "Footer:" }, ], - json!({"header": "", "footer": "&CPage &P of &N"}), + { + let sheet = app.session.active().and_then(|d| d.wb.active()); + let (header, footer) = sheet.map(|s| (s.print.header.clone(), s.print.footer.clone())).unwrap_or_default(); + json!({"header": header, "footer": footer}) + }, ), "pageSetup" => Dialog::form( "pageSetup", diff --git a/crates/ui-egui/tests/interaction.rs b/crates/ui-egui/tests/interaction.rs index 709393389..ccbb30e95 100644 --- a/crates/ui-egui/tests/interaction.rs +++ b/crates/ui-egui/tests/interaction.rs @@ -630,3 +630,21 @@ fn rotated_wrapped_text_is_drawn_rotated_inside_its_cell() { assert!(clip.width() < 70.0, "clip spans one 6-character column, got {clip:?}"); } } + +#[test] +fn header_footer_dialog_opens_on_the_saved_header_and_footer() { + // #523: the dialog started from a fixed empty header and `&CPage &P of &N`, so OK replaced the sheet's text. + let mut s = blank(); + s.execute("pageLayout.headerFooter", json!({"header": "&LLeft", "footer": "&CConfidential"})).unwrap(); + let mut h = harness(s); + h.state_mut().open_dialog("headerFooter", json!({})); + h.run_steps(2); + let d = h.state_mut().dialog.as_mut().unwrap(); + assert_eq!(d.values["header"], json!("&LLeft")); + assert_eq!(d.values["footer"], json!("&CConfidential")); + d.values.insert("header".into(), json!("&CNew")); + let vals = json!(d.values.clone()); + h.state_mut().run("pageLayout.headerFooter", vals).unwrap(); + let pr = &h.state().session.active().unwrap().wb.active().unwrap().print; + assert_eq!((pr.header.as_str(), pr.footer.as_str()), ("&CNew", "&CConfidential")); +} From 89439636d88fce4115799eb3ec7a067b3476e104 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Wed, 14 Oct 2026 08:08:41 +0200 Subject: [PATCH 03/20] Dialogs move by their title bar instead of staying pinned to the centre (fixes #1250) --- crates/ui-egui/src/dialogs.rs | 4 +- crates/ui-egui/tests/dialog_move.rs | 70 +++++++++++++++++++++++++++++ docs/ui-parity.md | 6 ++- 3 files changed, 77 insertions(+), 3 deletions(-) create mode 100644 crates/ui-egui/tests/dialog_move.rs diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..d00fbe807 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1040,7 +1040,9 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { .collapsible(false) .resizable(false) .default_width(width) - .anchor(egui::Align2::CENTER_CENTER, vec2(0.0, -40.0)) + // A default position, not an anchor: egui anchors can't be dragged, and dialogs move by their title bar. + .pivot(egui::Align2::CENTER_CENTER) + .default_pos(ctx.content_rect().center() + vec2(0.0, -40.0)) .open(&mut win_open); let window = if d.name == "insertPicture" { window.default_height(240.0) } else { window }; window.show(ctx, |ui| { diff --git a/crates/ui-egui/tests/dialog_move.rs b/crates/ui-egui/tests/dialog_move.rs new file mode 100644 index 000000000..7548342c7 --- /dev/null +++ b/crates/ui-egui/tests/dialog_move.rs @@ -0,0 +1,70 @@ +//! Dialogs can be dragged by their title bar, like Excel's, and stay where they are dropped. + +use egui::{Event, Modifiers, PointerButton, vec2}; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness() -> Harness { + let mut session = Session::new(); + session.new_workbook(); + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(6); + h +} + +fn dialog_rect(h: &Harness, name: &str) -> egui::Rect { + h.ctx.memory(|m| m.area_rect(egui::Id::new("dialog").with(name))).unwrap() +} + +fn button(h: &mut Harness, pos: egui::Pos2, pressed: bool) { + h.input_mut().events.push(Event::PointerButton { pos, button: PointerButton::Primary, pressed, modifiers: Modifiers::NONE }); + h.run_steps(2); +} + +fn drag_title_bar(h: &mut Harness, name: &str, by: egui::Vec2) { + let start = dialog_rect(h, name).center_top() + vec2(0.0, 10.0); + h.input_mut().events.push(Event::PointerMoved(start)); + h.run_steps(2); + button(h, start, true); + for step in 1..=4 { + h.input_mut().events.push(Event::PointerMoved(start + by * (step as f32 / 4.0))); + h.run_steps(1); + } + button(h, start + by, false); +} + +#[test] +fn dragging_the_find_and_replace_title_bar_moves_the_dialog_and_it_stays_there() { + let mut h = harness(); + h.state_mut().open_dialog("find", json!({})); + h.run_steps(6); + let before = dialog_rect(&h, "find"); + drag_title_bar(&mut h, "find", vec2(120.0, 80.0)); + h.run_steps(10); + let after = dialog_rect(&h, "find"); + assert_eq!(after.min - before.min, vec2(120.0, 80.0), "dialog went from {before:?} to {after:?}"); + assert!(h.state().dialog.is_some(), "the drag does not close the dialog"); +} + +#[test] +fn go_to_format_cells_and_sort_can_be_dragged_too() { + for name in ["goTo", "formatCells", "sort"] { + let mut h = harness(); + h.state_mut().open_dialog(name, json!({})); + h.run_steps(6); + let before = dialog_rect(&h, name); + drag_title_bar(&mut h, name, vec2(-100.0, 60.0)); + h.run_steps(10); + let after = dialog_rect(&h, name); + assert_eq!(after.min - before.min, vec2(-100.0, 60.0), "{name} went from {before:?} to {after:?}"); + } +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 22e221e69..2707c69e4 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-14 · **Change:** minor (dialogs move by their title bar, #1250) · **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 @@ -14,6 +14,7 @@ Sources: `crates/ui-egui/src` (ribbon, grid, editor, formula bar, dialogs, panes | Element | Excel | GridCraft | Status | |---|---|---|---| +| 2026-10-14 | minor | Dialogs (Find and Replace, Go To, Format Cells, Sort and the rest) move by their title bar instead of staying pinned to the centre (#1250) | | Title bar, Quick Access Toolbar, AutoSave toggle | yes | yes | done; macOS window controls misaligned (#170), Dock icon size (#169) | | Ribbon: tabs, groups, split buttons, galleries, contextual tabs | yes | yes (Table Design, Chart Design, Format, PivotTable, Header & Footer tabs) | done; missing contextual tabs for pictures, shapes, sparklines, slicers | | Ribbon collapse, simplified ribbon, customize ribbon/QAT | yes | partial | customization missing (File ▸ Options is the one missing File command) | @@ -137,7 +138,8 @@ Format Cells, Insert Function/Formula Builder, Find and Replace, Go To and Go To Name Manager, Data Validation, Paste Special, Conditional Formatting manager, Page Setup, Goal Seek, Scenario Manager, Data Analysis, Forecast Sheet and others exist. Missing or partial: Options, Solver, Power Query editor, Insert Object, Equation editor, Protect/Allow Edit Ranges, -Workbook Links, Accessibility checker. +Workbook Links, Accessibility checker. Like Excel's, every dialog opens centred and moves by its +title bar, keeping its place while it is open (#1250). ## Accessibility From 34aa98870d2e388c353a1aa718324609a549c54b Mon Sep 17 00:00:00 2001 From: Richard Date: Wed, 14 Oct 2026 20:20:10 +1100 Subject: [PATCH 04/20] Zoom dialog: Excel's layout, current zoom pre-selected, Fit selection (#930) View > Zoom is now one Magnification column as in Excel: 200%, 100%, 75%, 50%, 25%, Fit selection and Custom with its percentage box. The current zoom is pre-selected (Custom when it matches no preset), the box shows whole percentages clamped to 10..400 and typing in it selects Custom. Fit selection runs view.zoomToSelection with the grid's size, the same answer as the ribbon's Zoom to Selection. The window is 220 px wide instead of 380; its height is left to the shared dialog sizing work (#978). Magnification and Fit selection are translated in every catalog. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_01A2bJ3LxrZ6YnmppgrfwHwr --- crates/ui-egui/src/dialogs.rs | 100 ++++++++++++++++++++-------- crates/ui-egui/src/i18n.rs | 2 + crates/ui-egui/src/i18n/cs.rs | 2 + crates/ui-egui/src/i18n/de.rs | 2 + crates/ui-egui/src/i18n/es.rs | 2 + crates/ui-egui/src/i18n/fr.rs | 2 + crates/ui-egui/src/i18n/ko.rs | 2 + crates/ui-egui/src/i18n/pt.rs | 1 + crates/ui-egui/src/i18n/ru.rs | 2 + crates/ui-egui/src/i18n/sr.rs | 2 + crates/ui-egui/src/i18n/zh.rs | 2 + crates/ui-egui/tests/zoom_dialog.rs | 62 +++++++++++++++++ 12 files changed, 154 insertions(+), 27 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..b7b9488c4 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -47,13 +47,28 @@ impl Dialog { &self.name } - /// Sets a form value. In Zoom, a Magnification choice is also the percentage `view.zoom` stores. + /// Sets a form value. In Zoom, a Magnification choice is also the percentage `view.zoom` + /// stores, a typed percentage selects Custom, and Fit selection makes OK run + /// `view.zoomToSelection` (its view size was captured when the dialog opened). pub(crate) fn set_value(&mut self, key: &str, value: Json) { - if self.name == "zoom" - && key == "preset" - && let Some(p) = value.as_str().and_then(|s| s.parse::().ok()) - { - self.values.insert("percent".into(), json!(p)); + if self.name == "zoom" { + match key { + "preset" => match value.as_str() { + Some("fit") => self.command = Some("view.zoomToSelection"), + Some(s) => { + self.command = Some("view.zoom"); + if let Ok(p) = s.parse::() { + self.values.insert("percent".into(), json!(p)); + } + } + None => {} + }, + "percent" => { + self.command = Some("view.zoom"); + self.values.insert("preset".into(), json!(zoom_preset(value.as_f64().unwrap_or(0.0)))); + } + _ => {} + } } self.values.insert(key.into(), value); } @@ -522,20 +537,19 @@ impl Dialog { ], json!({"groupBy": "A", "function": "sum"}), ), - "zoom" => Dialog::form( - "zoom", - "Zoom", - "view.zoom", - vec![ - Choice { - key: "preset", - label: "Magnification", - options: vec![("200", "200%"), ("100", "100%"), ("75", "75%"), ("50", "50%"), ("25", "25%")], - }, - Number { key: "percent", label: "Custom (%):" }, - ], - json!({"percent": app.session.active().and_then(|d| d.wb.active().map(|s| s.zoom)).unwrap_or(100)}), - ), + "zoom" => { + // The current zoom is pre-selected (Custom when it matches no preset). Fit + // selection needs the grid's size, so it is captured here for `view.zoomToSelection`. + let zoom = app.session.active().and_then(|d| d.wb.active().map(|s| s.zoom)).unwrap_or(100); + let view = app.grid.cells_rect.unwrap_or(egui::Rect::from_min_size(egui::pos2(0.0, 0.0), vec2(1200.0, 700.0))); + Dialog::form( + "zoom", + "Zoom", + "view.zoom", + vec![], + json!({"percent": zoom, "preset": zoom_preset(zoom as f64), "viewWidth": view.width(), "viewHeight": view.height()}), + ) + } "defineName" => { // Name Manager › Edit… passes the name being edited (`oldName`, `oldScope`) and // its current fields; New… and Define Name start from the selection. @@ -983,13 +997,6 @@ fn build_params(d: &Dialog) -> Json { } } } - "zoom" => { - if let Some(p) = d.values.get("preset").and_then(Json::as_str) - && d.values.get("percent").is_none() - { - v["percent"] = json!(p.parse::().unwrap_or(100.0)); - } - } "cfQuick" => { let mut rule = Map::new(); for (k, val) in &d.values { @@ -1033,6 +1040,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { "recommendedCharts" => 600.0, "analysisToolPak" => 420.0, "solver" => 460.0, + "zoom" => 220.0, _ => 380.0, }; let window = egui::Window::new(app.ui.language.tr(&d.title)) @@ -1051,6 +1059,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { "insertFunction" => insert_function(app, ui, &mut d, &mut confirm), "find" => find(app, ui, &mut d), "goTo" => go_to(app, ui, &mut d, &mut confirm), + "zoom" => zoom(ui, &mut d, &mut confirm, &mut open), "sort" => sort(app, ui, &mut d, &mut confirm), "nameManager" => name_manager(app, ui, &mut d, &mut open), "manageRules" => manage_rules(app, ui), @@ -1890,6 +1899,43 @@ fn find(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog) { } } +/// Zoom's Magnification presets, as `preset` values and labels, in Excel's order. +const ZOOM_PRESETS: [(&str, &str); 5] = [("200", "200%"), ("100", "100%"), ("75", "75%"), ("50", "50%"), ("25", "25%")]; + +/// The `preset` that shows a zoom percentage: its Magnification entry, or Custom. +fn zoom_preset(percent: f64) -> &'static str { + ZOOM_PRESETS.iter().map(|(val, _)| *val).find(|val| val.parse::().ok() == Some(percent)).unwrap_or("custom") +} + +/// View › Zoom, laid out as Excel's: one Magnification column of 200% to 25%, Fit selection and +/// Custom with its percentage box. The current zoom is pre-selected; typing a percentage +/// selects Custom; Fit selection zooms so the selected cells fill the grid. +fn zoom(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, open: &mut bool) { + let lang = crate::i18n::current(ui.ctx()); + let cur = d.values.get("preset").and_then(Json::as_str).unwrap_or("custom").to_string(); + ui.label(egui::RichText::new(lang.tr("Magnification")).strong()); + for (val, text) in ZOOM_PRESETS { + if ui.radio(cur == val, text).clicked() { + d.set_value("preset", json!(val)); + } + } + if ui.radio(cur == "fit", lang.tr("Fit selection")).clicked() { + d.set_value("preset", json!("fit")); + } + ui.horizontal(|ui| { + if ui.radio(cur == "custom", format!("{}:", lang.tr("Custom"))).clicked() { + d.set_value("preset", json!("custom")); + } + // Whole percentages, as Excel shows them; the engine clamps to 10..400 as well. + let mut percent = d.values.get("percent").and_then(Json::as_f64).unwrap_or(100.0).round() as i64; + if ui.add(egui::DragValue::new(&mut percent).range(10..=400).speed(1.0)).changed() { + d.set_value("percent", json!(percent)); + } + ui.label("%"); + }); + ok_cancel(ui, confirm, open); +} + fn go_to(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool) { let names: Vec = app.session.active().map(|doc| doc.wb.names.iter().map(|n| n.name.clone()).collect()).unwrap_or_default(); for n in names { diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 061698df9..1833335f4 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -386,6 +386,8 @@ const JAPANESE: &[(&str, &str)] = &[ ("Gridlines", "目盛線"), ("Headings", "見出し"), ("Zoom", "ズーム"), + ("Magnification", "倍率"), + ("Fit selection", "選択範囲に合わせる"), ("100%", "100%"), ("Zoom to\nSelection", "選択範囲を拡大"), ("Freeze\nPanes", "ウィンドウ枠の固定"), diff --git a/crates/ui-egui/src/i18n/cs.rs b/crates/ui-egui/src/i18n/cs.rs index 7ed8372c7..5c7c894dd 100644 --- a/crates/ui-egui/src/i18n/cs.rs +++ b/crates/ui-egui/src/i18n/cs.rs @@ -192,6 +192,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "Mřížka", "Headings" => "Záhlaví", "Zoom" => "Lupa", + "Magnification" => "Měřítko", + "Fit selection" => "Přizpůsobit výběru", "Forecast\nSheet" => "List\nprognózy", "Add footer" => "Přidat zápatí", "Add header" => "Přidat záhlaví", diff --git a/crates/ui-egui/src/i18n/de.rs b/crates/ui-egui/src/i18n/de.rs index c20c7d1b3..8ad57abf2 100644 --- a/crates/ui-egui/src/i18n/de.rs +++ b/crates/ui-egui/src/i18n/de.rs @@ -192,6 +192,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "Gitternetzlinien", "Headings" => "Überschriften", "Zoom" => "Zoom", + "Magnification" => "Vergrößerung", + "Fit selection" => "An Markierung anpassen", "Forecast\nSheet" => "Prognose-\nblatt", "Add footer" => "Fußzeile hinzufügen", "Add header" => "Kopfzeile hinzufügen", diff --git a/crates/ui-egui/src/i18n/es.rs b/crates/ui-egui/src/i18n/es.rs index 4ce231bca..a2ceaef9b 100644 --- a/crates/ui-egui/src/i18n/es.rs +++ b/crates/ui-egui/src/i18n/es.rs @@ -193,6 +193,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "Líneas de cuadrícula", "Headings" => "Encabezados", "Zoom" => "Zoom", + "Magnification" => "Ampliación", + "Fit selection" => "Ajustar la selección", "Forecast\nSheet" => "Hoja de\npronóstico", "Add footer" => "Agregar pie de página", "Add header" => "Agregar encabezado", diff --git a/crates/ui-egui/src/i18n/fr.rs b/crates/ui-egui/src/i18n/fr.rs index 5d20ab0ca..3fb708871 100644 --- a/crates/ui-egui/src/i18n/fr.rs +++ b/crates/ui-egui/src/i18n/fr.rs @@ -214,6 +214,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "Quadrillage", "Headings" => "En-têtes", "Zoom" => "Zoom", + "Magnification" => "Zoom", + "Fit selection" => "Ajuster la sélection", "100%" => "100 %", "Zoom to\nSelection" => "Zoom sur la\nsélection", "Freeze\nPanes" => "Figer les\nvolets", diff --git a/crates/ui-egui/src/i18n/ko.rs b/crates/ui-egui/src/i18n/ko.rs index 1902ad301..77052d208 100644 --- a/crates/ui-egui/src/i18n/ko.rs +++ b/crates/ui-egui/src/i18n/ko.rs @@ -211,6 +211,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "눈금선", "Headings" => "머리글", "Zoom" => "확대/축소", + "Magnification" => "배율", + "Fit selection" => "선택 영역에 맞춤", "100%" => "100%", "Zoom to\nSelection" => "선택 영역에\n맞춤", "Freeze\nPanes" => "틀\n고정", diff --git a/crates/ui-egui/src/i18n/pt.rs b/crates/ui-egui/src/i18n/pt.rs index fb9236cdb..e1751cff7 100644 --- a/crates/ui-egui/src/i18n/pt.rs +++ b/crates/ui-egui/src/i18n/pt.rs @@ -212,6 +212,7 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "Linhas de Grade", "Headings" => "Títulos", "Zoom" => "Zoom", + "Fit selection" => "Ajustar seleção", "100%" => "100%", "Zoom to\nSelection" => "Zoom na\nSeleção", "Freeze\nPanes" => "Congelar\nPainéis", diff --git a/crates/ui-egui/src/i18n/ru.rs b/crates/ui-egui/src/i18n/ru.rs index 765f6bb32..de0b485a2 100644 --- a/crates/ui-egui/src/i18n/ru.rs +++ b/crates/ui-egui/src/i18n/ru.rs @@ -211,6 +211,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "Сетка", "Headings" => "Заголовки", "Zoom" => "Масштаб", + "Magnification" => "Масштаб", + "Fit selection" => "По выделению", "100%" => "100%", "Zoom to\nSelection" => "Масштаб по\nвыделению", "Freeze\nPanes" => "Закрепить\nобласти", diff --git a/crates/ui-egui/src/i18n/sr.rs b/crates/ui-egui/src/i18n/sr.rs index cbf18d4c4..1209ecfa3 100644 --- a/crates/ui-egui/src/i18n/sr.rs +++ b/crates/ui-egui/src/i18n/sr.rs @@ -206,6 +206,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "Linije mreže", "Headings" => "Zaglavlja", "Zoom" => "Uvećanje", + "Magnification" => "Uvećanje", + "Fit selection" => "Uklopi izbor", "100%" => "100%", "Zoom to\nSelection" => "Uvećaj na\nselekciju", "Freeze\nPanes" => "Zamrzni\nokna", diff --git a/crates/ui-egui/src/i18n/zh.rs b/crates/ui-egui/src/i18n/zh.rs index 00175925e..df9b7acbf 100644 --- a/crates/ui-egui/src/i18n/zh.rs +++ b/crates/ui-egui/src/i18n/zh.rs @@ -211,6 +211,8 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Gridlines" => "网格线", "Headings" => "标题", "Zoom" => "缩放", + "Magnification" => "缩放比例", + "Fit selection" => "恰好容纳选定区域", "100%" => "100%", "Zoom to\nSelection" => "缩放到\n所选内容", "Freeze\nPanes" => "冻结\n窗格", diff --git a/crates/ui-egui/tests/zoom_dialog.rs b/crates/ui-egui/tests/zoom_dialog.rs index f5be90833..f300d68f0 100644 --- a/crates/ui-egui/tests/zoom_dialog.rs +++ b/crates/ui-egui/tests/zoom_dialog.rs @@ -83,3 +83,65 @@ fn a_custom_percentage_after_a_magnification_wins_and_an_unedited_ok_keeps_the_z control(&mut h, "ui.dialog.confirm", json!({})); assert_eq!(zoom(&h), 225); } + +#[test] +fn the_current_zoom_is_preselected_as_its_magnification_or_as_custom() { + let mut h = harness(); + h.state_mut().open_dialog("zoom", json!({})); + h.run_steps(2); + let d = h.state().dialog.clone().unwrap(); + assert_eq!(d.values["preset"], json!("100")); + assert_eq!(d.values["percent"], json!(100)); + control(&mut h, "ui.dialog.cancel", json!({})); + h.state_mut().run("view.zoom", json!({"percent": 130})).unwrap(); + h.state_mut().open_dialog("zoom", json!({})); + h.run_steps(2); + let d = h.state().dialog.clone().unwrap(); + assert_eq!(d.values["preset"], json!("custom")); + assert_eq!(d.values["percent"], json!(130)); + // An unedited OK keeps the zoom. + let r = control(&mut h, "ui.dialog.confirm", json!({})); + assert_eq!(r["ok"], json!(true), "{r}"); + assert_eq!(zoom(&h), 130); +} + +#[test] +fn a_typed_percentage_selects_custom_and_sets_that_zoom() { + let mut h = harness(); + control(&mut h, "ui.dialog", json!({"name": "zoom"})); + control(&mut h, "ui.dialog.set", json!({"field": "preset", "value": "fit"})); + control(&mut h, "ui.dialog.set", json!({"field": "percent", "value": 150})); + assert_eq!(h.state().dialog.as_ref().unwrap().values["preset"], json!("custom")); + let r = control(&mut h, "ui.dialog.confirm", json!({})); + assert_eq!(r["ok"], json!(true), "{r}"); + assert_eq!(zoom(&h), 150); +} + +#[test] +fn fit_selection_zooms_so_the_selected_cells_fill_the_grid() { + let mut h = harness(); + h.state_mut().run("selection.set", json!({"range": "A1:J40"})).unwrap(); + h.run_steps(2); + let view = h.state().grid.cells_rect.unwrap(); + h.state_mut().open_dialog("zoom", json!({})); + h.run_steps(6); + h.get_by_label("Fit selection").click(); + h.run_steps(6); + h.get_by_label(" OK ").click(); + h.run_steps(6); + assert!(h.state().dialog.is_none()); + // The same answer the ribbon's Zoom to Selection gives for this grid. + let mut expected = Session::new(); + expected.new_workbook(); + expected.execute("selection.set", json!({"range": "A1:J40"})).unwrap(); + let r = expected.execute("view.zoomToSelection", json!({"viewWidth": view.width(), "viewHeight": view.height()})).unwrap(); + assert_eq!(json!(zoom(&h)), r["zoom"]); + assert!(zoom(&h) > 10 && zoom(&h) < 100, "{}", zoom(&h)); + // Fit selection over the control channel, before any frame is drawn, uses the grid size captured on opening. + h.state_mut().run("selection.set", json!({"range": "A1"})).unwrap(); + control(&mut h, "ui.dialog", json!({"name": "zoom"})); + control(&mut h, "ui.dialog.set", json!({"field": "preset", "value": "fit"})); + let r = control(&mut h, "ui.dialog.confirm", json!({})); + assert_eq!(r["ok"], json!(true), "{r}"); + assert_eq!(zoom(&h), 400); +} From 78ffd3b4c3907f6d1695a3ad91996c2b746bda3f Mon Sep 17 00:00:00 2001 From: derechopiano <257026466+derechopiano@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:40:16 +0100 Subject: [PATCH 05/20] fix(pdf): preserve exported links and allow CLI PDF conversion (#954) --- apps/gridcraft-cli/src/main.rs | 29 ++++++++-- crates/engine/src/cmd/print.rs | 92 +++++++++++++++++++++++++++++++ crates/pdf/src/lib.rs | 98 ++++++++++++++++++++++++++++++++-- crates/pdf/src/tests.rs | 23 ++++++++ docs/file-format-parity.md | 5 +- 5 files changed, 239 insertions(+), 8 deletions(-) diff --git a/apps/gridcraft-cli/src/main.rs b/apps/gridcraft-cli/src/main.rs index 152ddbcbe..0b763e378 100644 --- a/apps/gridcraft-cli/src/main.rs +++ b/apps/gridcraft-cli/src/main.rs @@ -36,7 +36,7 @@ use serde_json::{Value, json}; const USAGE: &str = "usage: gridcraft-cli info [--json] sheets, used ranges, tables, charts, names - gridcraft-cli convert [--sheet NAME] xlsx/csv/tsv/json/html by extension + gridcraft-cli convert [--sheet NAME] xlsx/csv/tsv/json/html/pdf by extension gridcraft-cli eval [--in FILE] [--sheet S] [--cell A1] [--json] gridcraft-cli cat [--range A1:D20] [--sheet S] [--formulas] [--csv] gridcraft-cli check ... [--json] [--tolerance 1e-9] [--max-report N] @@ -271,11 +271,17 @@ fn convert(args: &[String]) -> Result<(), String> { let input = a.pos(0, "")?; let out = a.pos(1, "")?; a.no_extra(2)?; - if gridcraft_engine::io::FileKind::from_path(out).is_none() { - return Err(format!("{out}: unknown output format (use .xlsx, .csv, .tsv, .json or .html)")); + let pdf = out.to_ascii_lowercase().ends_with(".pdf"); + if !pdf && gridcraft_engine::io::FileKind::from_path(out).is_none() { + return Err(format!("{out}: unknown output format (use .xlsx, .csv, .tsv, .json, .html or .pdf)")); } let mut s = open(input)?; activate_sheet(&mut s, a.opt("sheet"))?; + if pdf { + let result = exec(&mut s, "file.exportPdf", json!({"path": out}))?; + eprintln!("wrote {out} ({} bytes, {} pages)", result["bytes"], result["pages"]); + return Ok(()); + } let d = s.doc().map_err(|e| e.to_string())?; let bytes = gridcraft_engine::io::save_file_bytes(&d.wb, out).map_err(|e| e.to_string())?; gridcraft_engine::io::write_file(out, &bytes).map_err(|e| e.to_string())?; @@ -583,4 +589,21 @@ mod tests { assert!(Args::parse(&["--nope".to_string()], &[], &[]).is_err()); assert!(Args::parse(&["--range".to_string()], &["range"], &[]).is_err()); } + + #[test] + fn convert_xlsx_to_pdf_keeps_links() { + let dir = std::env::temp_dir().join(format!("gridcraft-cli-pdf-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + let input = dir.join("links.xlsx"); + let output = dir.join("links.pdf"); + let mut s = blank(); + s.execute("insert.link", json!({"cell": "A1", "target": "https://example.org"})).unwrap(); + let bytes = gridcraft_engine::io::save_file_bytes(&s.doc().unwrap().wb, input.to_str().unwrap()).unwrap(); + gridcraft_engine::io::write_file(input.to_str().unwrap(), &bytes).unwrap(); + convert(&[input.to_string_lossy().into_owned(), output.to_string_lossy().into_owned()]).unwrap(); + let pdf = std::fs::read(&output).unwrap(); + assert!(pdf.starts_with(b"%PDF-")); + assert!(pdf.windows(b"/URI (https://example.org)".len()).any(|w| w == b"/URI (https://example.org)")); + std::fs::remove_dir_all(dir).unwrap(); + } } diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index a10f19d46..aefa948cc 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -996,6 +996,62 @@ fn draw_block(pg: &mut Page, b: &Block, gridlines: bool, cf: &mut Option) { + let range = b.range(); + let (Some(&bx), Some(&by), Some(&right), Some(&bottom)) = (b.xs.first(), b.ys.first(), b.xs.last(), b.ys.last()) else { return }; + for (cell, link) in b.sh.hyperlinks.range(range.start..=range.end) { + if !range.contains(*cell) { + continue; + } + let cell_range = b.sh.merges.iter().find(|m| m.start == *cell).copied().unwrap_or(RangeRef::cell(*cell)); + let (x, y, w, h) = b.rect(cell_range); + let (x0, y0, x1, y1) = (x.max(bx), y.max(by), (x + w).min(right), (y + h).min(bottom)); + if x1 <= x0 || y1 <= y0 { + continue; + } + let rect = (x0, y0, x1 - x0, y1 - y0); + match crate::links::classify(&link.target) { + crate::links::LinkKind::External(uri) => pg.link_uri(rect.0, rect.1, rect.2, rect.3, &uri), + crate::links::LinkKind::Internal(target) if !link.external => { + pending.push(PendingPdfLink { page, sheet: b.si, rect, target }); + } + _ => {} + } + } +} + +fn printed_target(wb: &Workbook, blocks: &[PrintedBlock], source_sheet: usize, target: &str) -> Option<(usize, f32, f32)> { + let target = target.trim().trim_start_matches('#').trim_start_matches('='); + let (sheet_name, range) = split_sheet(target); + let sheet = sheet_name.map_or(Some(source_sheet), |name| wb.sheet_index(&name))?; + let cell = RangeRef::parse(&range.replace('$', ""))?.start; + let sh = wb.sheet(sheet)?; + let block = blocks.iter().find(|b| b.sheet == sheet && (b.rows.0..=b.rows.1).contains(&cell.row) && (b.cols.0..=b.cols.1).contains(&cell.col))?; + let x = block.x + (sh.col_left(cell.col) - sh.col_left(block.cols.0)) as f32 * block.k; + let y = block.y + (sh.row_top(cell.row) - sh.row_top(block.rows.0)) as f32 * block.k; + Some((block.page, x, y)) +} + /// Cell pictures fit the current cell (or merged-cell) rectangle without changing aspect ratio. fn draw_cell_picture(pg: &mut Page, data: &[u8], rect: (f32, f32, f32, f32), k: f32) { let (x, y, w, h) = rect; @@ -1462,6 +1518,8 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str let total: usize = layouts.iter().map(|l| l.pages.len()).sum(); let (date, time) = hf_date_time(wb); let mut page_no = 0usize; + let mut pending_links = Vec::new(); + let mut printed_blocks = Vec::new(); for lay in layouts { let Some(sh) = wb.sheet(lay.sheet) else { continue }; let mut cf = if sh.cond_formats.is_empty() { None } else { Some(crate::cf::CfCache::new()) }; @@ -1476,6 +1534,7 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str let content_h = (lay.heading_h + rsegs.iter().map(|s| seg_size(sh, *s, true)).sum::()) * k; let ox = lay.margins[0] + if lay.settings.center_h { ((aw - content_w) / 2.0).max(0.0) } else { 0.0 }; let oy = lay.margins[2] + if lay.settings.center_v { ((ah - content_h) / 2.0).max(0.0) } else { 0.0 }; + let page_index = doc.page_count(); let pg = doc.add_page(lay.page_w, lay.page_h); let mut y = oy + lay.heading_h * k; let mut body: Option = None; @@ -1484,6 +1543,8 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str for (csi, cs) in csegs.iter().enumerate() { let b = make_block(wb, lay.sheet, sh, *rs, *cs, x, y, lay); draw_block(pg, &b, lay.settings.gridlines, &mut cf); + draw_block_links(pg, &b, page_index, &mut pending_links); + printed_blocks.push(PrintedBlock { page: page_index, sheet: lay.sheet, rows: *rs, cols: *cs, x, y, k }); if lay.settings.headings { draw_headings(pg, &b, rsi == 0, csi == 0, (ox, oy), (lay.heading_w * k, lay.heading_h * k)); } @@ -1514,6 +1575,14 @@ pub fn render_pdf(wb: &Workbook, layouts: &[SheetLayout], file: &str, path: &str draw_header_footer(pg, lay, lay.settings.hf_page(pi == 0, page_no), &cx); } } + for link in pending_links { + if let Some((target_page, x, y)) = printed_target(wb, &printed_blocks, link.sheet, &link.target) + && let Some(page) = doc.page_mut(link.page) + { + let (lx, ly, w, h) = link.rect; + page.link_page(lx, ly, w, h, target_page, x, y); + } + } let n = doc.page_count(); (doc.finish(), n) } @@ -1759,6 +1828,29 @@ mod tests { hay.windows(needle.len()).any(|w| w == needle.as_bytes()) } + #[test] + fn pdf_export_keeps_safe_links_on_their_printed_pages() { + let mut s = s(); + s.execute("insert.link", json!({"cell": "A1", "target": "https://example.org/report"})).unwrap(); + s.execute("insert.link", json!({"cell": "A2", "target": "Sheet1!A60"})).unwrap(); + s.execute("insert.link", json!({"cell": "A3", "target": "file:///private/report"})).unwrap(); + s.execute("cell.set", json!({"cell": "A60", "input": "Destination"})).unwrap(); + let (pdf, pages) = pdf_of(&mut s, json!({})); + assert_eq!(pages, 2); + let text = String::from_utf8_lossy(&pdf); + assert_eq!(text.matches("/Subtype /Link").count(), 2); + assert!(text.contains("/URI (https://example.org/report)")); + assert!(text.contains("/Dest [") && text.contains("/XYZ ")); + assert!(!text.contains("/URI (file:")); + + // A link to a cell omitted by the selected print range must not point to a nonexistent + // destination. The external link in the selected range remains clickable. + let (cropped, _) = pdf_of(&mut s, json!({"range": "A1:A3"})); + let cropped = String::from_utf8_lossy(&cropped); + assert_eq!(cropped.matches("/Subtype /Link").count(), 1); + assert!(!cropped.contains("/Dest [")); + } + #[test] fn header_sections_split_join_and_field_names() { let spec = "&L&\"Arial,Bold\"Sales &&Co&CPage &P of &N&R&D"; diff --git a/crates/pdf/src/lib.rs b/crates/pdf/src/lib.rs index b55f247cf..a0a5afde1 100644 --- a/crates/pdf/src/lib.rs +++ b/crates/pdf/src/lib.rs @@ -210,6 +210,19 @@ fn pdf_string(text: &str) -> String { s } +/// URI actions use byte strings. Percent-encode non-ASCII UTF-8 before PDF string escaping. +fn pdf_uri_string(uri: &str) -> String { + let mut ascii = String::with_capacity(uri.len()); + for byte in uri.bytes() { + if byte.is_ascii() { + ascii.push(byte as char); + } else { + let _ = write!(ascii, "%{byte:02X}"); + } + } + pdf_string(&ascii) +} + /// A number for a content stream: finite, short. fn n(v: f32) -> String { let v = if v.is_finite() { v.clamp(-1.0e7, 1.0e7) } else { 0.0 }; @@ -260,6 +273,17 @@ struct ImageData { alpha: Option>, } +enum LinkTarget { + Uri(String), + Page { index: usize, x: f32, y: f32 }, +} + +struct LinkAnnotation { + /// PDF coordinates, with the origin at the bottom left. + rect: [f32; 4], + target: LinkTarget, +} + /// A page under construction. Obtain one with [`PdfDoc::add_page`]. pub struct Page { width: f32, @@ -267,6 +291,7 @@ pub struct Page { content: String, fonts: [bool; 12], images: Vec, + links: Vec, depth: u32, } @@ -275,6 +300,8 @@ pub struct Page { const MAX_PAGE_CONTENT: usize = 64 * 1024 * 1024; /// Largest image (pixels) embedded. const MAX_IMAGE_PIXELS: u64 = 40_000_000; +const MAX_PAGE_LINKS: usize = 4096; +const MAX_LINK_URI_BYTES: usize = 4096; impl Page { pub fn width(&self) -> f32 { @@ -296,6 +323,42 @@ impl Page { } } + fn link_rect(&self, x: f32, y: f32, w: f32, h: f32) -> Option<[f32; 4]> { + if !finite(&[x, y, w, h]) || !extent(w) || !extent(h) || self.links.len() >= MAX_PAGE_LINKS { + return None; + } + let (right, bottom) = (x + w, y + h); + if !finite(&[right, bottom]) { + return None; + } + let (left, right) = (x.max(0.0), right.min(self.width)); + let (top, bottom) = (y.max(0.0), bottom.min(self.height)); + (right > left && bottom > top).then(|| [left, self.y(bottom), right, self.y(top)]) + } + + /// Adds an invisible clickable area for an external URI. Invalid or excessive links are ignored. + pub fn link_uri(&mut self, x: f32, y: f32, w: f32, h: f32, uri: &str) { + let allowed = uri + .split_once(':') + .is_some_and(|(scheme, rest)| matches!(scheme.to_ascii_lowercase().as_str(), "http" | "https" | "mailto") && !rest.is_empty()); + if !allowed || uri.len() > MAX_LINK_URI_BYTES || uri.chars().any(char::is_control) { + return; + } + if let Some(rect) = self.link_rect(x, y, w, h) { + self.links.push(LinkAnnotation { rect, target: LinkTarget::Uri(uri.to_string()) }); + } + } + + /// Adds a link to a location on another page of this document (zero-based page index). + pub fn link_page(&mut self, x: f32, y: f32, w: f32, h: f32, page: usize, target_x: f32, target_y: f32) { + if !finite(&[target_x, target_y]) { + return; + } + if let Some(rect) = self.link_rect(x, y, w, h) { + self.links.push(LinkAnnotation { rect, target: LinkTarget::Page { index: page, x: target_x, y: target_y } }); + } + } + /// Saves the graphics state (pair with [`Page::restore`]). pub fn save(&mut self) { self.depth = self.depth.saturating_add(1); @@ -583,7 +646,7 @@ impl PdfDoc { pub fn add_page(&mut self, width_pt: f32, height_pt: f32) -> &mut Page { let width = if width_pt.is_finite() { width_pt.clamp(3.0, 14_400.0) } else { 612.0 }; let height = if height_pt.is_finite() { height_pt.clamp(3.0, 14_400.0) } else { 792.0 }; - self.pages.push(Page { width, height, content: String::new(), fonts: [false; 12], images: Vec::new(), depth: 0 }); + self.pages.push(Page { width, height, content: String::new(), fonts: [false; 12], images: Vec::new(), links: Vec::new(), depth: 0 }); let i = self.pages.len() - 1; &mut self.pages[i] } @@ -619,6 +682,8 @@ impl PdfDoc { } let mut objects: Vec<(u32, Vec)> = Vec::new(); let mut kids = Vec::new(); + let mut annotations = Vec::new(); + let page_count = self.pages.len(); let mut font_dict = String::new(); for (i, id) in font_ids.iter().enumerate() { if *id != 0 { @@ -678,20 +743,47 @@ impl PdfDoc { o.extend_from_slice(b"endstream"); objects.push((content_id, o)); let xo = if xobjects.is_empty() { String::new() } else { format!(" /XObject << {xobjects}>>") }; + let mut link_refs = String::new(); + for link in std::mem::take(&mut page.links) { + if matches!(&link.target, LinkTarget::Page { index, .. } if *index >= page_count) { + continue; + } + let id = next; + next += 1; + let _ = write!(link_refs, "{id} 0 R "); + annotations.push((id, link)); + } + let annots = if link_refs.is_empty() { String::new() } else { format!(" /Annots [{link_refs}]") }; objects.push(( page_id, format!( - "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 {} {}] /Resources << /ProcSet [/PDF /Text /ImageB /ImageC] /Font << {}>>{} >> /Contents {} 0 R >>", + "<< /Type /Page /Parent 2 0 R /MediaBox [0 0 {} {}] /Resources << /ProcSet [/PDF /Text /ImageB /ImageC] /Font << {}>>{} >> /Contents {} 0 R{} >>", n(page.width), n(page.height), font_dict, xo, - content_id + content_id, + annots ) .into_bytes(), )); kids.push(page_id); } + for (id, link) in annotations { + let [x0, y0, x1, y1] = link.rect; + let action = match link.target { + LinkTarget::Uri(uri) => format!("/A << /S /URI /URI {} >>", pdf_uri_string(&uri)), + LinkTarget::Page { index, x, y } => { + let Some(&page_id) = kids.get(index) else { continue }; + let height = self.pages.get(index).map_or(0.0, |p| p.height); + format!("/Dest [{page_id} 0 R /XYZ {} {} null]", n(x), n(height - y)) + } + }; + objects.push(( + id, + format!("<< /Type /Annot /Subtype /Link /Rect [{} {} {} {}] /Border [0 0 0] {action} >>", n(x0), n(y0), n(x1), n(y1)).into_bytes(), + )); + } objects.push((1, b"<< /Type /Catalog /Pages 2 0 R >>".to_vec())); let kids_s: Vec = kids.iter().map(|k| format!("{k} 0 R")).collect(); objects.push((2, format!("<< /Type /Pages /Kids [{}] /Count {} >>", kids_s.join(" "), kids.len()).into_bytes())); diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 7d6a61a45..b0b2d14c6 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -76,6 +76,29 @@ fn drawing_and_text() { assert!(find(&pdf, b"0 792 m 612 0 l S").is_some()); } +#[test] +fn uri_and_page_links_are_real_pdf_annotations() { + let mut d = PdfDoc::new(); + let first = d.add_page(200.0, 300.0); + first.link_uri(10.0, 20.0, 80.0, 30.0, "https://example.org/a(b)?q=£"); + first.link_page(100.0, 40.0, 50.0, 20.0, 1, 35.0, 60.0); + first.link_uri(-20.0, -20.0, 40.0, 40.0, "mailto:a@example.org"); + first.link_uri(0.0, 0.0, f32::NAN, 10.0, "https://bad.example"); + first.link_uri(0.0, 0.0, 10.0, 10.0, "javascript:alert(1)"); + let second = d.add_page(100.0, 200.0); + second.link_page(0.0, 0.0, 10.0, 10.0, 99, 0.0, 0.0); + let pdf = d.finish(); + check_xref(&pdf); + let s = String::from_utf8_lossy(&pdf); + assert_eq!(s.matches("/Subtype /Link").count(), 3); + assert!(s.contains("/Annots [")); + assert!(s.contains("/Rect [10 250 90 280]")); + assert!(s.contains("/Rect [0 280 20 300]")); + assert!(s.contains("/URI (https://example.org/a\\(b\\)?q=%C2%A3)")); + assert!(s.contains("/XYZ 35 140 null")); + assert!(!s.contains("https://bad.example")); +} + #[test] fn images_with_alpha() { let mut d = PdfDoc::new(); diff --git a/docs/file-format-parity.md b/docs/file-format-parity.md index 988472414..2360fd1a6 100644 --- a/docs/file-format-parity.md +++ b/docs/file-format-parity.md @@ -1,6 +1,6 @@ # File-format parity -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** trivial (XLSX chart sheets are kept on save (#727); previously minor: CSV save keeps the separator it was opened with) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-14 · **Change:** minor (PDF export preserves safe cell hyperlinks to URLs and printed destinations, #954) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 Every format Excel opens or saves, with GridCraft's read and write support, fidelity and tests. Part of [target-app-parity.md](target-app-parity.md). **File formats: ~50% ready, 80–120 h** @@ -56,7 +56,7 @@ and repair-free opening remain unverified. | Format | Excel | GridCraft | Notes | |---|---|---|---| -| PDF | Save As / Export | yes (own PDF 1.7 writer) | Only the 14 standard PDF fonts, no embedding: **non-Latin text (CJK, Arabic, Devanagari, Thai) does not export**; web build exports without a file prompt (#141) | +| PDF | Save As / Export | yes (own PDF 1.7 writer; CLI `convert` output) | Printed cell hyperlinks export as clickable HTTP(S)/mailto links or destinations within printed pages (#954); unsafe schemes and destinations outside the print range are omitted. Only the 14 standard PDF fonts, no embedding: **non-Latin text (CJK, Arabic, Devanagari, Thai) does not export**; web build exports without a file prompt (#141) | | XPS | Windows | no | Low priority | | Print | yes | yes (via PDF, system dialog on desktop; PR #143 for web) | | | Images of ranges/charts (Copy as Picture) | yes | chart raster in `crates/chart` | No range-to-image command | @@ -85,6 +85,7 @@ refresh. Counted under features (Get & Transform, ~10% ready), not here. | Date | Change | Summary | |---|---|---| +| 2026-10-14 | minor | PDF export writes clickable annotations for safe cell URLs and destinations on printed pages; CLI `convert` accepts `.pdf` output (#954) | | 2026-10-12 | trivial | XLSX chart sheets are kept on save (#727) | | 2026-10-12 | trivial | CSV and TSV export is capped at 512 MiB and refuses a larger used range with an error (#528) | | 2026-10-12 | minor | `.xls` import reads the calculation settings (CalcMode, CalcIter, CalcCount, CalcDelta, CalcPrecision), so circular models calculate instead of showing `#CIRC!` (#724) | From 45644d3f43c19fce54ddc179d79e282d84ff8676 Mon Sep 17 00:00:00 2001 From: wstock <1394687+wstock@users.noreply.github.com> Date: Thu, 15 Oct 2026 07:06:31 +0000 Subject: [PATCH 06/20] Control channel: ui.dialog.confirm sends what OK sends (#1459) OK runs a dialog's command with build_params(&d), which reshapes the field values for several dialogs (the rule wrapper of cfQuick, the delimiters list of Text to Columns, the columns of Remove Duplicates and Subtotal, Data Validation's unused fields, Zoom presets, Series, Define Name). ui.dialog.confirm passed the raw field values instead, so a Highlight Cells rule confirmed over the control channel added nothing and Text to Columns ignored the ticked delimiter. Make build_params pub(crate) and use it from ui.dialog.confirm, say so in the control protocol doc, and add tests driving the Greater Than and Text to Columns dialogs over the control channel. --- crates/ui-egui/src/control.rs | 2 +- crates/ui-egui/src/dialogs.rs | 3 +- crates/ui-egui/tests/dialog_confirm.rs | 60 ++++++++++++++++++++++++++ docs/control-protocol.md | 2 +- 4 files changed, 64 insertions(+), 3 deletions(-) create mode 100644 crates/ui-egui/tests/dialog_confirm.rs diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 2ea5cfae0..344284598 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -252,7 +252,7 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) - "ui.dialog.confirm" => match app.dialog.take() { Some(d) => match d.command { Some(cmd) => { - let params = Json::Object(d.values.clone()); + let params = crate::dialogs::build_params(&d); let r = app.run(cmd, params); if r.is_err() { app.dialog = Some(d); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..aeb43f5c4 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -939,7 +939,8 @@ fn data_validation_form(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, o } /// Builds the command params from form values for dialogs that need translation. -fn build_params(d: &Dialog) -> Json { +/// The parameters OK sends to the dialog's command: its field values, reshaped per dialog. +pub(crate) fn build_params(d: &Dialog) -> Json { let mut v = Json::Object(d.values.clone()); match d.name.as_str() { "dataValidation" => { diff --git a/crates/ui-egui/tests/dialog_confirm.rs b/crates/ui-egui/tests/dialog_confirm.rs new file mode 100644 index 000000000..8ec98b040 --- /dev/null +++ b/crates/ui-egui/tests/dialog_confirm.rs @@ -0,0 +1,60 @@ +//! `ui.dialog.confirm` runs a dialog's command with the parameters its OK button sends (#1459). + +use egui::vec2; +use gridcraft_engine::Session; +use gridcraft_engine::model::CfRule; +use gridcraft_ui_egui::SheetApp; +use gridcraft_ui_egui::control::{Outcome, handle}; +use serde_json::{Value as Json, json}; + +fn harness() -> egui_kittest::Harness<'static, SheetApp> { + let mut session = Session::new(); + session.new_workbook(); + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(6); + h +} + +fn control(h: &mut egui_kittest::Harness<'static, SheetApp>, method: &str, params: Json) -> Json { + let ctx = h.ctx.clone(); + match handle(h.state_mut(), &ctx, method, ¶ms) { + Outcome::Done(v) => v, + Outcome::Screenshot { .. } => json!(null), + } +} + +#[test] +fn confirming_a_highlight_cells_rule_adds_the_rule_like_ok() { + let mut h = harness(); + h.state_mut().run("range.setValues", json!({"range": "A1", "values": [[1], [3]]})).unwrap(); + h.state_mut().run("selection.set", json!({"range": "A1:A2"})).unwrap(); + // Home › Conditional Formatting › Highlight Cells Rules › Greater Than… + control(&mut h, "ui.dialog", json!({"name": "cfQuick", "type": "cellIs", "operator": "greaterThan", "title": "Greater Than"})); + control(&mut h, "ui.dialog.set", json!({"field": "value", "value": "2"})); + let r = control(&mut h, "ui.dialog.confirm", json!({})); + assert_eq!(r["ok"], json!(true), "{r}"); + assert!(h.state().dialog.is_none(), "the dialog closed"); + let sheet = h.state().session.active().unwrap().wb.active().unwrap(); + assert_eq!(sheet.cond_formats.len(), 1, "{:?}", sheet.cond_formats); + let rule = &sheet.cond_formats[0].rule; + assert!(matches!(rule, CfRule::CellIs { a, .. } if a == "2"), "{rule:?}"); +} + +#[test] +fn confirming_text_to_columns_splits_on_the_ticked_delimiter() { + let mut h = harness(); + h.state_mut().run("range.setValues", json!({"range": "A1", "values": [["a;b"], ["c;d"]]})).unwrap(); + h.state_mut().run("selection.set", json!({"range": "A1:A2"})).unwrap(); + control(&mut h, "ui.dialog", json!({"name": "textToColumns"})); + control(&mut h, "ui.dialog.set", json!({"field": "semicolon", "value": true})); + let r = control(&mut h, "ui.dialog.confirm", json!({})); + assert_eq!(r["ok"], json!(true), "{r}"); + let read = h.state_mut().run("sheet.read", json!({"range": "A1:B2"})).unwrap(); + assert_eq!(read["values"], json!([["a", "b"], ["c", "d"]]), "{read}"); +} diff --git a/docs/control-protocol.md b/docs/control-protocol.md index 7cc6ec230..698f0f010 100644 --- a/docs/control-protocol.md +++ b/docs/control-protocol.md @@ -31,7 +31,7 @@ Errors come back as `{"id": …, "ok": false, "error": "…"}`. | `ui.key` / `ui.text` | `{key, shift?, cmd?, alt?, ctrl?}` / `{text}` | Keyboard input (typing in the grid starts editing) | | `ui.edit.begin` / `ui.edit.text` / `ui.edit.commit` / `ui.edit.cancel` | `{text?}` / `{move?: down\|right\|up\|left\|none}` | Drive the cell editor | | `ui.ribbon` | `{tab}` | Switch ribbon tab | -| `ui.dialog` / `ui.dialog.set` / `ui.dialog.confirm` / `ui.dialog.cancel` | `{name}` / `{field, value}` | Open, fill in and confirm dialogs | +| `ui.dialog` / `ui.dialog.set` / `ui.dialog.confirm` / `ui.dialog.cancel` | `{name}` / `{field, value}` | Open, fill in and confirm dialogs; `confirm` runs the dialog's command with the same parameters as its OK button | | `ui.backstage` (via `engine.execute`) | `{page?: home\|new\|open\|info\|saveAs\|print\|export\|options\|close}` | Open the File tab's Backstage view on a page, or close it (`ui.inspect` reports the open page as `backstage`) | | `ui.screenshot` | `{path?}` | PNG of the window (base64 when no path). Animations are settled first: the picture shows the final state | | `ui.set` | `{dark?, formulaBar?, ribbonCollapsed?, animations?}` | UI preferences (`formulaBar` runs `view.formulaBar`) | From 18fecf8e8ce05afa01271779ab66562109ceb16f Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:16:52 +0300 Subject: [PATCH 07/20] engine: pageLayout.printTitles only touches the axes it is given MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit print_titles read both rows and cols as Options and assigned both unconditionally, so {"rows":"2:2"} set title_cols = None and {"cols":"A:A"} set title_rows = None: the repeated rows or columns disappeared from every printed page with no message, and there was no way to set or clear just one axis. The sibling pageLayout.pageSetup already distinguishes "key absent" from "key present but empty"; read both axes the same way — absent leaves the axis alone, empty/null clears it, anything else that is not a range is an error rather than a silent wipe. Regression test: print_titles_keeps_the_axis_it_is_not_given — on main the rows-only call clears the columns; with the fix each axis is touched only when named, an explicit empty value still clears, and a bad range errors. --- crates/engine/src/cmd/view.rs | 26 ++++++++++++++++++++++---- crates/engine/src/tests.rs | 27 +++++++++++++++++++++++++++ 2 files changed, 49 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/view.rs b/crates/engine/src/cmd/view.rs index edcacf57d..e99a13c8f 100644 --- a/crates/engine/src/cmd/view.rs +++ b/crates/engine/src/cmd/view.rs @@ -232,13 +232,31 @@ fn print_area(s: &mut Session, p: &Json) -> Result { } fn print_titles(s: &mut Session, p: &Json) -> Result { - let rows = str_param(p, "rows").and_then(RangeRef::parse).map(|r| (r.start.row, r.end.row)); - let cols = str_param(p, "cols").and_then(RangeRef::parse).map(|r| (r.start.col, r.end.col)); + // An absent key leaves that axis alone, and an empty or null value clears it — the same + // reading `pageLayout.pageSetup` gives `titleRows`/`titleCols`. Assigning `None` for a + // key that was never sent used to wipe the other axis. + let axis = |key: &str, is_cols: bool| -> Result>> { + match p.get(key) { + None => Ok(None), + Some(Json::Null) => Ok(Some(None)), + Some(Json::String(t)) if t.trim().is_empty() => Ok(Some(None)), + Some(Json::String(t)) => match RangeRef::parse(t) { + Some(r) => Ok(Some(Some(if is_cols { (r.start.col, r.end.col) } else { (r.start.row, r.end.row) }))), + None => Err(bad("pageLayout.printTitles", format!("`{key}` is not a range: `{t}`"))), + }, + Some(_) => Err(bad("pageLayout.printTitles", format!("`{key}` must be text like \"1:2\" or \"A:A\""))), + } + }; + let (rows, cols) = (axis("rows", false)?, axis("cols", true)?); edit(s, |cx| { let si = cx.sheet_index(); let pr = &mut cx.sheet_mut(si)?.print; - pr.title_rows = rows; - pr.title_cols = cols; + if let Some(t) = rows { + pr.title_rows = t; + } + if let Some(t) = cols { + pr.title_cols = t; + } Ok(Json::Null) }) } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..503f08d87 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2925,6 +2925,33 @@ fn notes_comments_and_links_survive_a_json_save() { assert_eq!(after.cells, before.cells); } +/// Page Layout ▸ Print Titles: naming one axis must not clear the other. `print_titles` read +/// both `rows` and `cols` into `Option`s and assigned both unconditionally, so +/// `{"rows":"2:2"}` silently dropped the repeated columns from every printed page. +#[test] +fn print_titles_keeps_the_axis_it_is_not_given() { + let mut s = s(); + s.execute("pageLayout.printTitles", json!({"rows": "1:1", "cols": "A:A"})).unwrap(); + let titles = |s: &Session| { + let d = s.doc().unwrap(); + let sh = d.wb.active().unwrap(); + (sh.print.title_rows, sh.print.title_cols) + }; + assert_eq!(titles(&s), (Some((0, 0)), Some((0, 0)))); + // Only rows: the columns stay. + s.execute("pageLayout.printTitles", json!({"rows": "2:2"})).unwrap(); + assert_eq!(titles(&s), (Some((1, 1)), Some((0, 0))), "rows alone must not clear the columns"); + // Only columns: the rows stay. + s.execute("pageLayout.printTitles", json!({"cols": "B:B"})).unwrap(); + assert_eq!(titles(&s), (Some((1, 1)), Some((1, 1))), "columns alone must not clear the rows"); + // An empty value clears that axis on purpose. + s.execute("pageLayout.printTitles", json!({"cols": ""})).unwrap(); + assert_eq!(titles(&s), (Some((1, 1)), None)); + // A bad range is an error, not a silent wipe. + assert!(s.execute("pageLayout.printTitles", json!({"rows": "not a range"})).is_err()); + assert_eq!(titles(&s), (Some((1, 1)), None)); +} + /// Insert › Link sorts a target as Excel's dialog does: a sheet reference, a defined name or `#…` is /// a place in this document, anything else an existing file or web page. A file saved as a /// `location` opens nothing in Excel (#800). From f8cbfb7cb602bd15a9c2a2cf70009fcf0af9cec6 Mon Sep 17 00:00:00 2001 From: midasmatterhorn <248507632+midasmatterhorn@users.noreply.github.com> Date: Thu, 15 Oct 2026 17:40:18 +0300 Subject: [PATCH 08/20] engine: sort and remove duplicates move the rows' notes and hyperlinks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit do_sort moved cells, formulas, cell pictures, row heights and floating object anchors through the sort permutation, and remove_duplicates moved cells and pictures through the compaction, but neither touched Sheet::comments or Sheet::hyperlinks (keyed by CellRef): a note that described one record stayed behind and annotated whatever landed on that row — 'late payment' on a paid invoice — and a note on a row removed by Remove Duplicates annotated blank cells. Sheet::comments/hyperlinks are exactly the range-scoped side data that row and column inserts already remap and edit.clear already clears, so the omission looks unintended. Both maps now travel through the same permutation the cells use (the sort collects before re-inserting, so a swap cannot resurrect an entry from a destination that is also a source; Remove Duplicates compacts the kept rows and drops the rest). Regression test: sort_and_remove_duplicates_move_the_rows_notes_and_links — on main the note and link stay on their old cells; with the fix they travel with their record. --- crates/engine/src/cmd/data.rs | 58 +++++++++++++++++++++++++++++++++++ crates/engine/src/tests.rs | 44 ++++++++++++++++++++++++++ 2 files changed, 102 insertions(+) diff --git a/crates/engine/src/cmd/data.rs b/crates/engine/src/cmd/data.rs index 383624b70..357e0e207 100644 --- a/crates/engine/src/cmd/data.rs +++ b/crates/engine/src/cmd/data.rs @@ -295,6 +295,21 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_co } // Move whole lines (cells with formulas adjusted for relative references). let lanes: Vec = if by_cols { (body.start.row..=body.end.row).collect() } else { (body.start.col..=body.end.col).collect() }; + // Notes and hyperlinks hang off a cell, not a row, so they travel with the row they + // describe — the way cell pictures and row heights do. Collect them before inserting, + // so a swap can't resurrect one from a destination that is also a source. + let side: Vec<(CellRef, Comment)> = order + .iter() + .enumerate() + .flat_map(|(dst_i, &src_i)| lanes.iter().map(move |&k| (src_i, dst_i as u32, k))) + .filter_map(|(src_i, dst_i, k)| sh.comments.get(&line(src_i, k)).map(|cm| (line(dst_i, k), cm.clone()))) + .collect(); + let links: Vec<(CellRef, Hyperlink)> = order + .iter() + .enumerate() + .flat_map(|(dst_i, &src_i)| lanes.iter().map(move |&k| (src_i, dst_i as u32, k))) + .filter_map(|(src_i, dst_i, k)| sh.hyperlinks.get(&line(src_i, k)).map(|h| (line(dst_i, k), h.clone()))) + .collect(); let mut moved: Vec<(CellRef, Option)> = Vec::with_capacity(body.count() as usize); let mut pictures = Vec::new(); for (dst_i, &src_i) in order.iter().enumerate() { @@ -354,6 +369,18 @@ fn do_sort(s: &mut Session, r: RangeRef, header: bool, keys: Vec, by_co } } } + // Every cell of the body is the destination of exactly one source cell, so clearing the + // body and inserting the permuted entries is the same permutation the cells went through. + for c in body.iter() { + shm.comments.remove(&c); + shm.hyperlinks.remove(&c); + } + for (at, cm) in side { + shm.comments.insert(at, cm); + } + for (at, h) in links { + shm.hyperlinks.insert(at, h); + } shm.cell_pictures.extend(pictures); for (i, h) in heights.into_iter().enumerate() { let row = body.start.row + i as u32; @@ -774,6 +801,24 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result { .filter_map(move |col| sh.cell_pictures.get(&CellRef::new(*row, col)).map(|p| (CellRef::new(start + i as u32, col), p.clone()))) }) .collect(); + // Rows a filter keeps are compacted to `start + i`, so the notes and hyperlinks that + // describe them move with them; the ones on removed rows are dropped with the row. + let notes: Vec<_> = keep + .iter() + .enumerate() + .flat_map(|(i, row)| { + (r.start.col..=r.end.col) + .filter_map(move |col| sh.comments.get(&CellRef::new(*row, col)).map(|cm| (CellRef::new(start + i as u32, col), cm.clone()))) + }) + .collect(); + let links: Vec<_> = keep + .iter() + .enumerate() + .flat_map(|(i, row)| { + (r.start.col..=r.end.col) + .filter_map(move |col| sh.hyperlinks.get(&CellRef::new(*row, col)).map(|h| (CellRef::new(start + i as u32, col), h.clone()))) + }) + .collect(); let rows: Vec>> = keep.iter().map(|row| (r.start.col..=r.end.col).map(|c| sh.cell(CellRef::new(*row, c)).cloned()).collect()).collect(); let shm = cx.sheet_mut(sheet)?; @@ -782,6 +827,13 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result { shm.remove_cell(CellRef::new(row, c)); } } + for row in start..=r.end.row { + for c in r.start.col..=r.end.col { + let at = CellRef::new(row, c); + shm.comments.remove(&at); + shm.hyperlinks.remove(&at); + } + } for (i, cells) in rows.into_iter().enumerate() { for (j, cell) in cells.into_iter().enumerate() { if let Some(cell) = cell { @@ -790,6 +842,12 @@ fn remove_duplicates(s: &mut Session, p: &Json) -> Result { } } shm.cell_pictures.extend(pictures); + for (at, cm) in notes { + shm.comments.insert(at, cm); + } + for (at, h) in links { + shm.hyperlinks.insert(at, h); + } for c in RangeRef::new(CellRef::new(start, r.start.col), r.end).iter() { cx.changed.push((sheet, c)); } diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index ae499a189..54c348606 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -2925,6 +2925,50 @@ fn notes_comments_and_links_survive_a_json_save() { assert_eq!(after.cells, before.cells); } +/// Sorting and removing duplicates rewrite the rows of a range. The cells, pictures, row heights +/// and anchors travel with them, but the notes and hyperlinks hung off their old cells: a note +/// describing one record ended up annotating whatever landed on that row, and on a removed row it +/// annotated blank cells. +#[test] +fn sort_and_remove_duplicates_move_the_rows_notes_and_links() { + fn setup(values: [&str; 5]) -> Session { + let mut s = s(); + for (row, text) in values.iter().enumerate() { + s.execute("cell.set", json!({"cell": format!("A{}", row + 1), "input": text})).unwrap(); + } + s.execute("review.newNote", json!({"cell": "B5", "text": "late", "author": "Example"})).unwrap(); + s.execute("insert.link", json!({"cell": "B4", "target": "https://example.com/4"})).unwrap(); + s + } + let notes_and_links = |s: &Session| { + let sh = s.doc().unwrap().wb.active().unwrap().clone(); + let cells = ["B2", "B3", "B4", "B5"].map(|c| CellRef::parse(c).unwrap()); + ( + cells.map(|c| sh.comments.get(&c).map(|m| m.text.clone())), + cells.map(|c| sh.hyperlinks.get(&c).map(|h| h.target.clone())), + ) + }; + + // Sort ascending with unique keys d,a,e,b,c: the order becomes a(2), b(4), c(5), d(1), e(3). + // The note on B5 describes row 5's record ("c"), which lands on row 3; the link on B4 + // describes row 4's record ("b"), which lands on row 2. Both must travel with their record. + let mut s = setup(["d", "a", "e", "b", "c"]); + s.execute("data.sort", json!({"range": "A1:B5", "keys": [{"column": "A"}]})).unwrap(); + assert_eq!(v(&s, "A3"), Value::text("c")); + assert_eq!(v(&s, "A5"), Value::text("e")); + let (notes, links) = notes_and_links(&s); + assert_eq!(notes[1], Some("late".to_string()), "the note moved with its record: {notes:?}"); + assert_eq!(links[0], Some("https://example.com/4".to_string()), "the link moved with its record: {links:?}"); + + // Remove duplicates keeps the first of each value: rows 3 and 5 go, and row 4's record moves + // to row 3. The note on B5 described row 5 and goes with it. + let mut s = setup(["a", "b", "a", "c", "a"]); + s.execute("data.removeDuplicates", json!({"range": "A1:B5"})).unwrap(); + assert_eq!(v(&s, "A3"), Value::text("c")); + let (notes, _) = notes_and_links(&s); + assert!(notes.iter().all(Option::is_none), "a note on a removed row goes with it: {notes:?}"); +} + /// Insert › Link sorts a target as Excel's dialog does: a sheet reference, a defined name or `#…` is /// a place in this document, anything else an existing file or web page. A file saved as a /// `location` opens nothing in Excel (#800). From 3379fb63a57add83e5fbd8d5f7ea7f42d7a3b5d7 Mon Sep 17 00:00:00 2001 From: derechopiano <257026466+derechopiano@users.noreply.github.com> Date: Fri, 16 Oct 2026 05:33:52 +0100 Subject: [PATCH 09/20] fix(#1031): constrain Advanced Filter copy output --- crates/engine/src/cmd/whatif.rs | 55 +++++++++++++++++++++++++++------ docs/target-app-parity.md | 5 +-- 2 files changed, 49 insertions(+), 11 deletions(-) diff --git a/crates/engine/src/cmd/whatif.rs b/crates/engine/src/cmd/whatif.rs index 09c716cab..d9d1ace89 100644 --- a/crates/engine/src/cmd/whatif.rs +++ b/crates/engine/src/cmd/whatif.rs @@ -867,28 +867,30 @@ fn advanced_filter(s: &mut Session, p: &Json) -> Result { if let Some(t) = str_param(p, "copyTo") { let (dsi, dest) = parse_range(wb, t, CMD)?; let dest = dest.start; - let extract = RangeRef::new(dest, dest.offset_clamped(list.height() as i64 - 1, list.width() as i64 - 1)); - if dsi == si && extract.intersects(&list) { - return Err(EngineError::Other("The extract range can't overlap the list range.".into())); - } let mut rows_out: Vec = vec![list.start.row]; rows_out.extend(keep.iter().filter(|(_, k)| *k).map(|(r, _)| *r)); + let end = + dest.offset(rows_out.len() as i64 - 1, list.width() as i64 - 1).ok_or_else(|| bad(CMD, "the extract range exceeds the worksheet"))?; + let out_range = RangeRef::new(dest, end); + if dsi == si && out_range.intersects(&list) { + return Err(EngineError::Other("The extract range can't overlap the list range.".into())); + } let cells: Vec<(CellRef, Cell)> = rows_out .iter() .enumerate() .flat_map(|(i, row)| { - (list.start.col..=list.end.col).filter_map(move |c| { + (list.start.col..=list.end.col).map(move |c| { let src = CellRef::new(*row, c); - let to = dest.offset(i as i64, (c - list.start.col) as i64)?; + // The full output footprint was checked above. + let to = CellRef::new(dest.row + i as u32, dest.col + c - list.start.col); let style = sh.style_id(src); - Some((to, Cell { value: sh.value(src), formula: None, style })) + (to, Cell { value: sh.value(src), formula: None, style }) }) }) .collect(); - let out_range = RangeRef::new(dest, dest.offset_clamped(rows_out.len() as i64 - 1, list.width() as i64 - 1)); edit(s, |cx| { let shm = cx.sheet_mut(dsi)?; - let old: Vec = shm.cells.iter_range(extract).map(|(c, _)| c).collect(); + let old: Vec = shm.cells.iter_range(out_range).map(|(c, _)| c).collect(); for c in &old { shm.remove_cell(*c); } @@ -1317,6 +1319,41 @@ mod tests { assert!(s.execute("data.advancedFilter", json!({"range": "A1:B6", "criteria": "H1:I3", "copyTo": "B2"})).is_err()); } + #[test] + fn advanced_filter_copy_preserves_cells_beyond_result() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name", "Score"], ["A", 1], ["B", 2], ["C", 3]]})).unwrap(); + s.execute("range.setValues", json!({"range": "H1", "values": [["Name"], ["B"]]})).unwrap(); + s.execute("cell.set", json!({"cell": "K4", "input": "Keep me"})).unwrap(); + let r = s.execute("data.advancedFilter", json!({"range": "A1:B4", "criteria": "H1:H2", "copyTo": "K1"})).unwrap(); + assert_eq!(r["range"], "K1:L2"); + assert_eq!(v(&s, "K2"), Value::from("B")); + assert_eq!(v(&s, "K4"), Value::from("Keep me")); + } + + #[test] + fn advanced_filter_copy_rejects_overflow_without_mutation() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name", "Score"], ["A", 1]]})).unwrap(); + s.execute("range.setValues", json!({"range": "H1", "values": [["Name"], ["A"]]})).unwrap(); + for dest in ["XFD1", "K1048576"] { + s.execute("cell.set", json!({"cell": dest, "input": "Keep me"})).unwrap(); + assert!(s.execute("data.advancedFilter", json!({"range": "A1:B2", "criteria": "H1:H2", "copyTo": dest})).is_err()); + assert_eq!(v(&s, dest), Value::from("Keep me")); + } + } + + #[test] + fn advanced_filter_copy_allows_output_above_source_when_only_old_clear_area_overlaps() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "C4", "values": [["Name", "Score"], ["A", 1], ["B", 2], ["C", 3]]})).unwrap(); + s.execute("range.setValues", json!({"range": "H1", "values": [["Name"], ["A"]]})).unwrap(); + let r = s.execute("data.advancedFilter", json!({"range": "C4:D7", "criteria": "H1:H2", "copyTo": "B1"})).unwrap(); + assert_eq!(r["range"], "B1:C2"); + assert_eq!(v(&s, "C4"), Value::from("Name")); + assert_eq!(v(&s, "C2"), Value::Number(1.0)); + } + #[test] fn criteria_semantics() { use super::criterion_matches as m; diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index b623124d9..a10e34690 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1,6 +1,6 @@ # Target-app parity: GridCraft vs Microsoft Excel -> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-12 · **Change:** minor (rows auto-fit oversized fonts on open and row formatting) · **Target:** Microsoft Excel (Microsoft 365) +> **Last reviewed:** 2026-10-11 · **Last updated:** 2026-10-16 · **Change:** minor (Advanced Filter copy-to safety, #1031) · **Target:** Microsoft Excel (Microsoft 365) This is the authoritative parity assessment. [`ROADMAP.md`](../ROADMAP.md) summarizes it, [`gaps.md`](gaps.md) lists every shortfall, and the area checklists hold the detail: @@ -83,7 +83,7 @@ performance 8, stability 8, localization 4, hardware 2, platforms 2, ecosystem 2 | Worksheet functions | 12 | 98.1% (measured) | ~82% | 16–26 | 513 / 523 names, 504 working. Missing: GETPIVOTDATA, IMAGE plus cloud functions. No conformance suite against Excel-computed oracles; edge-case bugs keep surfacing (#137, #138). [function-parity.md](function-parity.md) | | Editing, navigation, clipboard, fill | 12 | ~90% | ~70% | 15–25 | Point mode, F4, AutoComplete, Paste Special, AutoFill, Flash Fill, Find/Replace, Go To Special. Bugs: rectangular drag-select (#28), monthly fill (#132), cut across sheets (#128), Insert Copied Cells missing (#21), key tips only on part of the Home tab (#43; #33, #49). [ui-parity.md](ui-parity.md) | | Cell formatting, styles, conditional formatting | 10 | ~95% | ~80% | 8–14 | Full number-format language, borders, 47 cell styles, 60 table styles, all CF rule kinds including data bars, colour scales and icon sets. Gaps: font weights beyond regular/bold (#168), system fonts list (#4), themes | -| Sort, filter, tables, validation, outline | 8 | ~82% | ~70% | 10–16 | Multi-level sort, AutoFilter, Advanced Filter, tables with totals, validation, outline, subtotals, consolidate. Bugs: subtotal header (#131), validation edits (#129), table style panic caught (#130); no slicers on tables | +| Sort, filter, tables, validation, outline | 8 | ~82% | ~70% | 10–16 | Multi-level sort, AutoFilter, Advanced Filter, tables with totals, validation, outline, subtotals, consolidate. Advanced Filter copy-to preserves cells outside its result and rejects worksheet overflow (#1031). Bugs: subtotal header (#131), validation edits (#129), table style panic caught (#130); no slicers on tables | | PivotTables | 7 | ~65% | ~45% | 25–40 | Insert/Recommended, field list, three layouts, values on rows or columns, date grouping, show-values-as, refresh, calculated fields, XLSX round-trip. Missing: slicers, timelines, calculated items, PivotCharts, value/label filters, number grouping, GETPIVOTDATA, data-model pivots. Round-trip never checked in Excel | | Charts and sparklines | 8 | ~50% | ~30% | 40–60 | 25 chart kinds, all 2-D. The chart model holds title, legend position, a data-labels flag, gridlines flag and axis titles; no axis scaling, trendlines, error bars, per-point formatting, chart sheets, 3-D, surface, map, Pareto. Several kinds don't survive XLSX round trip (open PRs #158, #161). [chart-parity.md](chart-parity.md) | | Page layout, print, PDF | 5 | ~80% | ~55% | 12–20 | Page setup, print titles, breaks, Page Break Preview, headers/footers, PDF export and printing. No true Page Layout view; the PDF writer uses only the 14 standard fonts without embedding (no CJK or other non-Latin text in PDF); web print only exports (#141, #143) | @@ -332,6 +332,7 @@ The inventory carried over from the 2026-10-07 ROADMAP.md, updated for what land | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | Advanced Filter copy-to clears only its actual output and rejects overflow without changing cells (#1031) | | 2026-10-12 | minor | Essentials AutoFit evidence: rows auto-fit oversized fonts on open and row formatting (#833) | | 2026-10-12 | minor | Sheet tabs: reveal the selected tab on navigation and resize while preserving manual scrolling (#681); no score change | | 2026-10-12 | minor | PivotTables: values on rows (Σ Values in Rows at any position, all three layouts; `pivot.moveField` and the field pane move it; `dataOnRows` round-trips) (no score change) | From 214a67d53cac60e29496d8bf1c3f35bd0cc75255 Mon Sep 17 00:00:00 2001 From: derechopiano <257026466+derechopiano@users.noreply.github.com> Date: Fri, 16 Oct 2026 05:57:03 +0100 Subject: [PATCH 10/20] feat(#1622): add Advanced Filter ribbon dialog --- crates/engine/src/cmd/whatif.rs | 61 ++++++++++------ crates/ui-egui/src/control.rs | 2 +- crates/ui-egui/src/dialogs.rs | 62 +++++++++++++++- .../src/keytips/keytip_table/generated.rs | 2 +- crates/ui-egui/src/keytips/keytips.json | 6 +- crates/ui-egui/src/ribbon.rs | 5 ++ crates/ui-egui/tests/advanced_filter.rs | 72 +++++++++++++++++++ crates/ui-egui/tests/keytips_generated.rs | 2 +- docs/ui-parity.md | 4 +- 9 files changed, 184 insertions(+), 32 deletions(-) create mode 100644 crates/ui-egui/tests/advanced_filter.rs diff --git a/crates/engine/src/cmd/whatif.rs b/crates/engine/src/cmd/whatif.rs index d9d1ace89..318d0cb72 100644 --- a/crates/engine/src/cmd/whatif.rs +++ b/crates/engine/src/cmd/whatif.rs @@ -70,7 +70,7 @@ pub fn specs() -> Vec { "Advanced…", ["Data", "Sort & Filter"], None, - "{range? (list with headers; default current region), criteria: \"H1:I3\" (header row of field names; rows OR'ed, columns AND'ed; \">100\", \"=abc\", \"a*\", \"<>\"; blank-headed formula cells are computed criteria for the first data row), copyTo?: \"K1\" (else filter in place by hiding rows), unique?: bool}", + "{range? (list with headers; default current region), criteria?: \"H1:I3\" (omitted/blank matches all rows; header row of field names; rows OR'ed, columns AND'ed; \">100\", \"=abc\", \"a*\", \"<>\"; blank-headed formula cells are computed criteria for the first data row), copyTo?: \"K1\" (else filter in place by hiding rows), unique?: bool}", has_doc, advanced_filter ), @@ -795,41 +795,45 @@ fn advanced_filter(s: &mut Session, p: &Json) -> Result { (d.wb.active_sheet, crate::selection::current_region(sh, d.selection.active)) } }; - let (csi, crit) = range_arg(s, p, "criteria", CMD)?; + let criteria = str_param(p, "criteria").filter(|t| !t.trim().is_empty()).map(|t| parse_range(&d.wb, t, CMD)).transpose()?; if list.height() < 1 || list.count() > 50_000_000 { return Err(bad(CMD, "the list range is too large")); } - if crit.width() > 1024 || crit.height() > 10_000 { + if let Some((_, crit)) = criteria + && (crit.width() > 1024 || crit.height() > 10_000) + { return Err(bad(CMD, "the criteria range is too large")); } let wb = &d.wb; let sh = wb.sheet(si).ok_or(EngineError::NoDocument)?; - let csh = wb.sheet(csi).ok_or(EngineError::NoDocument)?; let headers: Vec = (list.start.col..=list.end.col).map(|c| crate::display::cell_text(wb, sh, CellRef::new(list.start.row, c)).trim().to_lowercase()).collect(); let first_data = list.start.row + 1; // Criteria rows → conditions. let mut rows: Vec> = Vec::new(); - for cr in crit.start.row + 1..=crit.end.row { - let mut conds = Vec::new(); - for cc in crit.start.col..=crit.end.col { - let at = CellRef::new(cr, cc); - let cell = csh.cell(at); - let v = csh.value(at); - let formula = cell.and_then(|c| c.formula.as_ref()); - if v.is_empty() && formula.is_none() { - continue; - } - let head = crate::display::cell_text(wb, csh, CellRef::new(crit.start.row, cc)).trim().to_lowercase(); - match headers.iter().position(|h| !head.is_empty() && *h == head) { - Some(off) => conds.push(Cond::Compare(off as u32, v)), - None => match formula.and_then(|f| f.expr()) { - Some(e) => conds.push(Cond::Computed(e, at)), - None => return Err(EngineError::Other(format!("The criteria label `{head}` doesn't match a column of the list."))), - }, + if let Some((csi, crit)) = criteria { + let csh = wb.sheet(csi).ok_or(EngineError::NoDocument)?; + for cr in crit.start.row + 1..=crit.end.row { + let mut conds = Vec::new(); + for cc in crit.start.col..=crit.end.col { + let at = CellRef::new(cr, cc); + let cell = csh.cell(at); + let v = csh.value(at); + let formula = cell.and_then(|c| c.formula.as_ref()); + if v.is_empty() && formula.is_none() { + continue; + } + let head = crate::display::cell_text(wb, csh, CellRef::new(crit.start.row, cc)).trim().to_lowercase(); + match headers.iter().position(|h| !head.is_empty() && *h == head) { + Some(off) => conds.push(Cond::Compare(off as u32, v)), + None => match formula.and_then(|f| f.expr()) { + Some(e) => conds.push(Cond::Computed(e, at)), + None => return Err(EngineError::Other(format!("The criteria label `{head}` doesn't match a column of the list."))), + }, + } } + rows.push(conds); } - rows.push(conds); } let matches_row = |row: u32| -> bool { if rows.is_empty() { @@ -845,7 +849,7 @@ fn advanced_filter(s: &mut Session, p: &Json) -> Result { } Cond::Computed(e, at) => { let shifted = gridcraft_formula::adjust::shift_relative(e.clone(), row as i64 - first_data as i64, 0); - let v = gridcraft_calc::recalc::evaluate_expr(wb, csi, *at, &shifted); + let v = gridcraft_calc::recalc::evaluate_expr(wb, criteria.map(|(si, _)| si).unwrap_or(si), *at, &shifted); matches!(v.scalar().to_bool(), Ok(true)) } }) @@ -1354,6 +1358,17 @@ mod tests { assert_eq!(v(&s, "C2"), Value::Number(1.0)); } + #[test] + fn advanced_filter_unique_without_criteria() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Name"], ["A"], ["A"], ["B"]]})).unwrap(); + let r = s.execute("data.advancedFilter", json!({"range": "A1:A4", "copyTo": "C1", "unique": true})).unwrap(); + assert_eq!(r["matched"], 2); + assert_eq!(r["range"], "C1:C3"); + assert_eq!(v(&s, "C2"), Value::from("A")); + assert_eq!(v(&s, "C3"), Value::from("B")); + } + #[test] fn criteria_semantics() { use super::criterion_matches as m; diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index 2ea5cfae0..0bd14ebce 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -252,7 +252,7 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) - "ui.dialog.confirm" => match app.dialog.take() { Some(d) => match d.command { Some(cmd) => { - let params = Json::Object(d.values.clone()); + let params = if d.name == "advancedFilter" { crate::dialogs::build_params(&d) } else { Json::Object(d.values.clone()) }; let r = app.run(cmd, params); if r.is_err() { app.dialog = Some(d); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..b33041085 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -437,6 +437,26 @@ impl Dialog { vec![Check { key: "header", label: "My data has headers" }, Text { key: "columnsText", label: "Columns (blank = all, e.g. A,C):" }], json!({"header": true}), ), + "advancedFilter" => { + let list = app + .session + .active() + .and_then(|d| d.wb.active().map(|sh| gridcraft_engine::selection::current_region(sh, d.selection.active).a1())) + .unwrap_or(sel.clone()); + let mut d = Dialog::form( + "advancedFilter", + "Advanced Filter", + "data.advancedFilter", + vec![], + json!({"action": "inPlace", "range": list, "criteria": "", "copyTo": "", "unique": false}), + ); + for key in ["action", "range", "criteria", "copyTo", "unique"] { + if let Some(value) = params.get(key) { + d.set_value(key, value.clone()); + } + } + d + } "goalSeek" => Dialog::custom("goalSeek", "Goal Seek", json!({"set": active, "to": "0", "changing": ""})), "forecastSheet" => Dialog::form( "forecastSheet", @@ -938,10 +958,43 @@ fn data_validation_form(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, o ok_cancel(ui, confirm, open); } +fn advanced_filter_form(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, open: &mut bool) { + use Field::*; + let copy = d.values.get("action").and_then(Json::as_str) == Some("copy"); + d.fields = vec![ + Choice { + key: "action", + label: "Action", + options: vec![("inPlace", "Show matching rows in place"), ("copy", "Copy matching rows elsewhere")], + }, + Text { key: "range", label: "List range:" }, + Text { key: "criteria", label: "Criteria range (optional):" }, + ]; + if copy { + d.fields.push(Text { key: "copyTo", label: "Copy to:" }); + } + d.fields.push(Check { key: "unique", label: "Unique rows only" }); + form_fields(ui, d); + // Constrain the button row: the shared right-to-left layout otherwise + // consumes the window's remaining height instead of fitting the buttons. + ui.allocate_ui(vec2(ui.available_width(), ui.spacing().interact_size.y + 8.0), |ui| ok_cancel(ui, confirm, open)); +} + /// Builds the command params from form values for dialogs that need translation. -fn build_params(d: &Dialog) -> Json { +pub(crate) fn build_params(d: &Dialog) -> Json { let mut v = Json::Object(d.values.clone()); match d.name.as_str() { + "advancedFilter" => { + if let Some(m) = v.as_object_mut() { + m.remove("action"); + if d.values.get("action").and_then(Json::as_str) != Some("copy") { + m.remove("copyTo"); + } + if m.get("criteria").and_then(Json::as_str).is_some_and(|s| s.trim().is_empty()) { + m.remove("criteria"); + } + } + } "dataValidation" => { let kind = d.values.get("type").and_then(Json::as_str).unwrap_or("any"); let operator = d.values.get("operator").and_then(Json::as_str).unwrap_or("between"); @@ -1042,12 +1095,17 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { .default_width(width) .anchor(egui::Align2::CENTER_CENTER, vec2(0.0, -40.0)) .open(&mut win_open); - let window = if d.name == "insertPicture" { window.default_height(240.0) } else { window }; + let window = match d.name.as_str() { + "insertPicture" => window.default_height(240.0), + "advancedFilter" => window.default_height(0.0), + _ => window, + }; window.show(ctx, |ui| { ui.set_min_width(width - 20.0); match d.name.as_str() { "formatCells" => format_cells(app, ui, &mut d, &mut confirm), "dataValidation" => data_validation_form(ui, &mut d, &mut confirm, &mut open), + "advancedFilter" => advanced_filter_form(ui, &mut d, &mut confirm, &mut open), "insertFunction" => insert_function(app, ui, &mut d, &mut confirm), "find" => find(app, ui, &mut d), "goTo" => go_to(app, ui, &mut d, &mut confirm), diff --git a/crates/ui-egui/src/keytips/keytip_table/generated.rs b/crates/ui-egui/src/keytips/keytip_table/generated.rs index 79920b5b8..17f0ff0cd 100644 --- a/crates/ui-egui/src/keytips/keytip_table/generated.rs +++ b/crates/ui-egui/src/keytips/keytip_table/generated.rs @@ -291,7 +291,7 @@ pub const SEQUENCES: &[(&str, Entry)] = &[ ("APR", Entry::Finish(Action::Pending("data.recentSources"))), // pending; Excel: Recent Sources; gap #12; reserved until the command lands ("APS", Entry::Finish(Action::Command("data.queriesConnections"))), // Excel: Show Queries ("APT", Entry::Finish(Action::Pending("data.fromTable"))), // pending; Excel: From Table (query); gap #12; reserved until the command lands - ("AQ", Entry::Finish(Action::Command("data.advancedFilter"))), // Excel: Advanced Filter + ("AQ", Entry::Finish(Action::Dialog("advancedFilter"))), // Excel: Advanced Filter ("AR", Entry::Continue), // group: group: Queries & Connections ("ARA", Entry::Finish(Action::Command("data.refreshAll"))), // Excel: Refresh All ("ARC", Entry::Finish(Action::Pending("data.cancelRefresh"))), // pending; Excel: Cancel Refresh; gap #12; reserved until the command lands diff --git a/crates/ui-egui/src/keytips/keytips.json b/crates/ui-egui/src/keytips/keytips.json index 59aeadcfb..efe5c938b 100644 --- a/crates/ui-egui/src/keytips/keytips.json +++ b/crates/ui-egui/src/keytips/keytips.json @@ -1965,8 +1965,8 @@ }, { "seq": "AQ", - "kind": "command", - "target": "data.advancedFilter", + "kind": "dialog", + "target": "advancedFilter", "excel": "Advanced Filter", "note": "" }, @@ -2523,4 +2523,4 @@ "note": "Alt+Q search box does not exist yet (issue #49)" } ] -} \ No newline at end of file +} diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 9c6d89eeb..be1a9604b 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -1925,6 +1925,11 @@ fn data(app: &mut SheetApp, ui: &mut Ui) { if __k.clicked() { act(app, "data.reapply", json!({})); } + let __k = small_button(ui, Icon::Filter, "Advanced", "Advanced Filter", false); + control_keytip(app, ui, &__k, "dialog:advancedFilter"); + if __k.clicked() { + app.open_dialog("advancedFilter", json!({})); + } }); sep(ui); let __k = big_button(ui, Icon::TextColumns, "Text to\nColumns", "Text to Columns", false); diff --git a/crates/ui-egui/tests/advanced_filter.rs b/crates/ui-egui/tests/advanced_filter.rs new file mode 100644 index 000000000..449c6a47e --- /dev/null +++ b/crates/ui-egui/tests/advanced_filter.rs @@ -0,0 +1,72 @@ +//! Advanced Filter's dialog must route both actions to the same engine command. + +use egui::vec2; +use egui_kittest::kittest::Queryable; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use gridcraft_ui_egui::control::{Outcome, handle}; +use serde_json::{Value as Json, json}; + +fn harness() -> egui_kittest::Harness<'static, SheetApp> { + let mut session = Session::new(); + session.new_workbook(); + let mut h = egui_kittest::Harness::builder().with_size(vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(&ui.ctx().clone()); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(6); + h +} + +fn control(h: &mut egui_kittest::Harness<'static, SheetApp>, method: &str, params: Json) -> Json { + let ctx = h.ctx.clone(); + match handle(h.state_mut(), &ctx, method, ¶ms) { + Outcome::Done(v) => v, + Outcome::Screenshot { .. } => json!(null), + } +} + +#[test] +fn dialog_offers_copy_action_and_runs_unique_extract() { + let mut h = harness(); + h.state_mut().run("range.setValues", json!({"range": "A1", "values": [["Name"], ["A"], ["A"], ["B"]]})).unwrap(); + h.state_mut().open_dialog("advancedFilter", json!({})); + h.run_steps(6); + h.get_by_label("Copy matching rows elsewhere").click(); + h.run_steps(6); + let rect = h.ctx.memory(|m| m.area_rect(egui::Id::new("dialog").with("advancedFilter"))).unwrap(); + assert!(rect.width() < 500.0 && rect.height() < 400.0, "dialog must fit its controls: {rect:?}"); + assert!( + h.state() + .dialog + .as_ref() + .is_some_and(|d| d.fields.iter().any(|f| matches!(f, gridcraft_ui_egui::dialogs::Field::Text { key: "copyTo", .. }))) + ); + assert_eq!(h.state().dialog.as_ref().unwrap().values["range"], "A1:A4"); + control(&mut h, "ui.dialog.set", json!({"field": "copyTo", "value": "C1"})); + control(&mut h, "ui.dialog.set", json!({"field": "unique", "value": true})); + h.run_steps(6); + h.get_by_label(" OK ").click(); + h.run_steps(6); + assert!(h.state().dialog.is_none()); + assert!(h.state().message.is_none()); + assert_eq!(h.state().session.active().unwrap().wb.active().unwrap().value(gridcraft_engine::core::CellRef::parse("C3").unwrap()).display(), "B"); +} + +#[test] +fn in_place_action_does_not_use_stale_copy_destination() { + let mut h = harness(); + h.state_mut().run("range.setValues", json!({"range": "A1", "values": [["Name"], ["A"], ["B"]]})).unwrap(); + h.state_mut().run("range.setValues", json!({"range": "H1", "values": [["Name"], ["B"]]})).unwrap(); + control(&mut h, "ui.dialog", json!({"name": "advancedFilter"})); + control(&mut h, "ui.dialog.set", json!({"field": "range", "value": "A1:A3"})); + control(&mut h, "ui.dialog.set", json!({"field": "criteria", "value": "H1:H2"})); + control(&mut h, "ui.dialog.set", json!({"field": "copyTo", "value": "K1"})); + let r = control(&mut h, "ui.dialog.confirm", json!({})); + assert_eq!(r["ok"], true, "{r}"); + assert!(h.state().session.active().unwrap().wb.active().unwrap().is_row_hidden(1)); + assert!(h.state().session.active().unwrap().wb.active().unwrap().value(gridcraft_engine::core::CellRef::parse("K1").unwrap()).is_empty()); +} diff --git a/crates/ui-egui/tests/keytips_generated.rs b/crates/ui-egui/tests/keytips_generated.rs index 653554857..5d649c56d 100644 --- a/crates/ui-egui/tests/keytips_generated.rs +++ b/crates/ui-egui/tests/keytips_generated.rs @@ -2149,7 +2149,7 @@ fn keytip_apt_() { fn keytip_aq_() { let mut h = harness(blank()); sequence(&mut h, "AQ", false); - assert!(h.state().message.is_some() || h.state().toast.is_some(), "AQ must invoke data.advancedFilter and surface its missing-params error"); + assert!(h.state().dialog.is_some(), "AQ must open the advancedFilter dialog"); } #[test] diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 22e221e69..bbf5ee966 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 (Advanced Filter dialog, #1622) · **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 @@ -51,6 +51,7 @@ this logic, with one undo step. | Point mode (click/drag/arrow references), reference colouring, F4 | done | | AutoComplete in columns, function autocomplete and argument tips | done; cell helpers clip to the worksheet and hide when the editor scrolls out (#200) | | Header resize and double-click AutoFit | done (#42, #88 for header dividers) | +| Data › Advanced Filter | dialog offers in-place or copy-to filtering, list and optional criteria ranges, and unique records; Alt+A,Q opens it (#1622). The engine's copy-to path clears only the result footprint and rejects worksheet overflow (#1031). | | Outline symbols for grouped rows / columns (band beside the headers, brackets, − / +, level buttons 1…n+1) | done (#373): clicks run `data.hideDetail` / `data.showDetail` / `data.showLevels`, frozen panes and right-to-left sheets included; summary rows/columns are always below/right (the "Summary rows below detail" setting isn't modelled), no "Show outline symbols" option yet, and a block of cells still groups rows without the Rows / Columns dialog | | Reorder selected whole columns by dragging their headers | done (10-11) | | Freeze panes, split panes | freeze done; split to verify | @@ -161,6 +162,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---| +| 2026-10-16 | minor | Advanced Filter ribbon dialog, optional criteria for unique-only extraction (#1622); safe copy-to output (#1031) | | 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 5627d2b6bf2bba98afd7dcd7376cfe6aa1837471 Mon Sep 17 00:00:00 2001 From: TheRenegadeDev Date: Fri, 16 Oct 2026 20:35:01 -0500 Subject: [PATCH 11/20] Prefill the Remove Duplicates range from the selection (#1388) --- crates/ui-egui/src/dialogs.rs | 22 ++++++++++++++++++++-- docs/ui-parity.md | 3 ++- 2 files changed, 22 insertions(+), 3 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..f19babf66 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -434,8 +434,12 @@ impl Dialog { "removeDuplicates", "Remove Duplicates", "data.removeDuplicates", - vec![Check { key: "header", label: "My data has headers" }, Text { key: "columnsText", label: "Columns (blank = all, e.g. A,C):" }], - json!({"header": true}), + vec![ + Text { key: "range", label: "Range:" }, + Check { key: "header", label: "My data has headers" }, + Text { key: "columnsText", label: "Columns (blank = all, e.g. A,C):" }, + ], + json!({"range": sel, "header": true}), ), "goalSeek" => Dialog::custom("goalSeek", "Goal Seek", json!({"set": active, "to": "0", "changing": ""})), "forecastSheet" => Dialog::form( @@ -2711,6 +2715,20 @@ mod data_validation_tests { mod tests { use super::*; + #[test] + fn remove_duplicates_prefills_the_selection_and_submits_the_edited_range() { + let mut session = gridcraft_engine::Session::default(); + session.new_workbook(); + session.execute("selection.set", json!({"range": "B2:C5"})).unwrap(); + let mut app = SheetApp::new(session, Default::default()); + let mut dialog = Dialog::open(&mut app, "removeDuplicates", json!({})).unwrap(); + assert_eq!(dialog.values.get("range"), Some(&json!("B2:C5"))); + assert!(dialog.fields.iter().any(|field| matches!(field, Field::Text { key: "range", .. }))); + assert_eq!(build_params(&dialog)["range"], "B2:C5"); + dialog.set_value("range", json!("D2:E6")); + assert_eq!(build_params(&dialog)["range"], "D2:E6"); + } + #[test] fn data_validation_opens_on_the_rule_of_the_active_cell() { let mut session = gridcraft_engine::Session::default(); diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 22e221e69..7af015a9c 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 (Remove Duplicates range prefill, #1388) · **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 | Remove Duplicates shows an editable Range field initialized from the selection and passes it to the engine on OK (#1388). Live range-picking outside the dialog remains unsupported | | 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 9a79dbef47d9a10bd2041aa00df7a111e72da582 Mon Sep 17 00:00:00 2001 From: NetterSilver <133535268+Silvertree2010@users.noreply.github.com> Date: Mon, 19 Oct 2026 10:26:12 +0200 Subject: [PATCH 12/20] Print CJK text with a CID font instead of question marks Fixes #1908 Co-Authored-By: Claude Haiku 5.5 Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/print.rs | 10 ++++ crates/pdf/src/lib.rs | 101 ++++++++++++++++++++++++++------- crates/pdf/src/tests.rs | 18 ++++++ 3 files changed, 107 insertions(+), 22 deletions(-) diff --git a/crates/engine/src/cmd/print.rs b/crates/engine/src/cmd/print.rs index 8350e56d7..0f8b9c3dd 100644 --- a/crates/engine/src/cmd/print.rs +++ b/crates/engine/src/cmd/print.rs @@ -1899,6 +1899,16 @@ mod tests { assert!(contains(&pdf, "(Odd 1) Tj") && contains(&pdf, "(Odd 2) Tj") && !contains(&pdf, "(Cover) Tj")); } + #[test] + fn pdf_prints_cjk_text_instead_of_question_marks() { + let mut s = s(); + s.execute("range.setValues", json!({"range": "A1", "values": [["签字清单", 2]]})).unwrap(); + let (pdf, _) = pdf_of(&mut s, json!({})); + assert!(!contains(&pdf, "(????) Tj"), "CJK cell printed as (????) Tj"); + assert!(contains(&pdf, "<7B7E5B576E055355> Tj")); + assert!(contains(&pdf, "(2) Tj")); + } + #[test] fn pdf_contains_text_and_header_codes_expand() { let mut s = s(); diff --git a/crates/pdf/src/lib.rs b/crates/pdf/src/lib.rs index b55f247cf..20bbb2ed3 100644 --- a/crates/pdf/src/lib.rs +++ b/crates/pdf/src/lib.rs @@ -1,7 +1,9 @@ //! A tiny, self-contained PDF 1.7 writer for GridCraft's print and PDF export. //! //! - The 14 standard fonts (no embedding) with WinAnsiEncoding, plus approximate width tables -//! ([`text_width`]) so callers can measure, align and wrap text. +//! ([`text_width`]) so callers can measure, align and wrap text. Characters outside +//! WinAnsiEncoding (CJK and other scripts) are set in a predefined, non-embedded Adobe-GB1 CID +//! font with the `UniGB-UTF16-H` CMap instead of being replaced by `?`. //! - Vector drawing: rectangles, lines (solid or dashed), polygons, circles (Bézier), clipping. //! - RGBA images as RGB image XObjects with a soft mask for alpha (run-length encoded). //! - Coordinates are points with a **top-left origin** (y grows down); text `y` is the baseline. @@ -125,6 +127,9 @@ impl Font { Font::TimesBold | Font::TimesBoldItalic => Some(&metrics::TIMES_BOLD), _ => None, }; + if needs_cid(c) { + return CID_WIDTH; + } let Some(table) = table else { return 600 }; let ascii = |c: char| -> Option { let i = (c as u32).checked_sub(32)? as usize; @@ -189,6 +194,31 @@ pub fn win_ansi(c: char) -> u8 { } } +/// Index of the CID font in a page's font flags (after the [`Font::ALL`] faces). +const CID_FONT: usize = Font::ALL.len(); +/// Number of font resources a page can use. +const FONT_SLOTS: usize = CID_FONT + 1; +/// Base font of the CID font used for characters WinAnsiEncoding lacks. +const CID_BASE_FONT: &str = "STSong-Light"; +/// Default glyph width of the CID font (full-width), in 1/1000 em. +const CID_WIDTH: u16 = 1000; + +/// True when `c` has no WinAnsiEncoding byte and is drawn with the CID font. +fn needs_cid(c: char) -> bool { + !c.is_control() && c != '?' && win_ansi(c) == b'?' +} + +/// A PDF hex string `< … >` of `text` in UTF-16BE (the `UniGB-UTF16-H` encoding). +fn pdf_utf16_string(text: &str) -> String { + let mut s = String::with_capacity(text.len() * 4 + 2); + s.push('<'); + for unit in text.encode_utf16() { + let _ = write!(s, "{unit:04X}"); + } + s.push('>'); + s +} + /// A PDF literal string `( … )` for `text` in WinAnsiEncoding. fn pdf_string(text: &str) -> String { let mut s = String::with_capacity(text.len() + 2); @@ -265,7 +295,7 @@ pub struct Page { width: f32, height: f32, content: String, - fonts: [bool; 12], + fonts: [bool; FONT_SLOTS], images: Vec, depth: u32, } @@ -460,24 +490,25 @@ impl Page { return; } let fi = font.index(); - if let Some(f) = self.fonts.get_mut(fi) { - *f = true; - } + let size = n(size.min(10_000.0)); let (sn, cs) = if angle.is_finite() { angle.sin_cos() } else { (0.0, 1.0) }; // Clockwise in y-down = counter-clockwise negated in PDF's y-up space. - let s = format!( - "BT\n{}\n/F{} {} Tf\n{} {} {} {} {} {} Tm\n{} Tj\nET", - rgb_op(color, false), - fi + 1, - n(size.min(10_000.0)), - n(cs), - n(-sn), - n(sn), - n(cs), - n(x), - n(self.y(y)), - pdf_string(text) - ); + let mut s = format!("BT\n{}\n{} {} {} {} {} {} Tm\n", rgb_op(color, false), n(cs), n(-sn), n(sn), n(cs), n(x), n(self.y(y))); + // Runs of WinAnsi text use `font`; runs it cannot encode use the CID font. Each `Tj` + // advances the text position, so the runs follow each other on the baseline. + let mut rest = text; + while let Some(first) = rest.chars().next() { + let cid = needs_cid(first); + let end = rest.char_indices().find(|(_, c)| needs_cid(*c) != cid).map_or(rest.len(), |(i, _)| i); + let (run, tail) = rest.split_at(end); + let (slot, string) = if cid { (CID_FONT, pdf_utf16_string(run)) } else { (fi, pdf_string(run)) }; + if let Some(f) = self.fonts.get_mut(slot) { + *f = true; + } + let _ = writeln!(s, "/F{} {} Tf\n{} Tj", slot + 1, size, string); + rest = tail; + } + s.push_str("ET"); self.op(&s); } @@ -583,7 +614,7 @@ impl PdfDoc { pub fn add_page(&mut self, width_pt: f32, height_pt: f32) -> &mut Page { let width = if width_pt.is_finite() { width_pt.clamp(3.0, 14_400.0) } else { 612.0 }; let height = if height_pt.is_finite() { height_pt.clamp(3.0, 14_400.0) } else { 792.0 }; - self.pages.push(Page { width, height, content: String::new(), fonts: [false; 12], images: Vec::new(), depth: 0 }); + self.pages.push(Page { width, height, content: String::new(), fonts: [false; FONT_SLOTS], images: Vec::new(), depth: 0 }); let i = self.pages.len() - 1; &mut self.pages[i] } @@ -603,18 +634,19 @@ impl PdfDoc { self.add_page(612.0, 792.0); } // Object numbering: 1 catalog, 2 page tree, 3 info, then fonts, then per page. - let mut used = [false; 12]; + let mut used = [false; FONT_SLOTS]; for p in &self.pages { for (u, f) in used.iter_mut().zip(p.fonts.iter()) { *u |= *f; } } let mut next = 4u32; - let mut font_ids = [0u32; 12]; + let mut font_ids = [0u32; FONT_SLOTS]; for (i, u) in used.iter().enumerate() { if *u { font_ids[i] = next; - next += 1; + // The CID font also needs its descendant font and font descriptor. + next += if i == CID_FONT { 3 } else { 1 }; } } let mut objects: Vec<(u32, Vec)> = Vec::new(); @@ -623,6 +655,31 @@ impl PdfDoc { for (i, id) in font_ids.iter().enumerate() { if *id != 0 { let _ = write!(font_dict, "/F{} {} 0 R ", i + 1, id); + if i == CID_FONT { + let (cid_id, desc_id) = (id + 1, id + 2); + objects.push(( + *id, + format!( + "<< /Type /Font /Subtype /Type0 /BaseFont /{CID_BASE_FONT} /Encoding /UniGB-UTF16-H /DescendantFonts [{cid_id} 0 R] >>" + ) + .into_bytes(), + )); + objects.push(( + cid_id, + format!( + "<< /Type /Font /Subtype /CIDFontType0 /BaseFont /{CID_BASE_FONT} /CIDSystemInfo << /Registry (Adobe) /Ordering (GB1) /Supplement 4 >> /FontDescriptor {desc_id} 0 R /DW {CID_WIDTH} >>" + ) + .into_bytes(), + )); + objects.push(( + desc_id, + format!( + "<< /Type /FontDescriptor /FontName /{CID_BASE_FONT} /Flags 6 /FontBBox [-25 -254 1000 880] /ItalicAngle 0 /Ascent 880 /Descent -120 /CapHeight 880 /StemV 93 >>" + ) + .into_bytes(), + )); + continue; + } let name = Font::ALL.get(i).copied().unwrap_or_default().base_name(); let enc = if name.starts_with("Symbol") { "" } else { " /Encoding /WinAnsiEncoding" }; objects.push((*id, format!("<< /Type /Font /Subtype /Type1 /BaseFont /{name}{enc} >>").into_bytes())); diff --git a/crates/pdf/src/tests.rs b/crates/pdf/src/tests.rs index 7d6a61a45..3711097b5 100644 --- a/crates/pdf/src/tests.rs +++ b/crates/pdf/src/tests.rs @@ -277,3 +277,21 @@ fn info_strings_are_capped() { let pdf = doc.finish(); assert_eq!(utf16_entry(&pdf, "/Title").chars().count(), MAX_INFO_CHARS); } + +#[test] +fn cjk_text_is_not_replaced_by_question_marks() { + let mut doc = PdfDoc::new(); + let p = doc.add_page(612.0, 792.0); + p.text(10.0, 20.0, 10.0, Font::Helvetica, [0, 0, 0], "签字清单"); + p.text(10.0, 40.0, 10.0, Font::Helvetica, [0, 0, 0], "No. 编号 2"); + let pdf = doc.finish(); + check_xref(&pdf); + assert!(find(&pdf, b"(????) Tj").is_none(), "CJK text written as question marks"); + assert!(find(&pdf, b"<7B7E5B576E055355> Tj").is_some()); + assert!(find(&pdf, b"(No. ) Tj").is_some()); + assert!(find(&pdf, b"<7F1653F7> Tj").is_some()); + assert!(find(&pdf, b"( 2) Tj").is_some()); + assert!(find(&pdf, b"/Encoding /UniGB-UTF16-H").is_some()); + // Measured as full-width glyphs, matching the CID font's default width. + assert_eq!(text_width(Font::Helvetica, 10.0, "签字"), 20.0); +} From da97bde61c5b8f688976d2ade4a683c0b1dcfc29 Mon Sep 17 00:00:00 2001 From: bfenty <1398236+bfenty@users.noreply.github.com> Date: Mon, 19 Oct 2026 17:21:50 +0000 Subject: [PATCH 13/20] fix(ui): seed Sort dialog column from the active cell The Sort dialog's 'Sort by' box was a free-text field hard-coded to 'A', so sorting a region in column B silently sorted column A unless the user noticed and retyped the letter. It now seeds from the column the cursor is in and renders a dropdown of the current region's columns, labelled with their header text. '+ Add Level' seeds the next region column instead of a hard-coded B. Closes #1906 Co-Authored-By: Claude --- crates/ui-egui/src/dialogs.rs | 39 ++++++++++++- crates/ui-egui/tests/sort_dialog_columns.rs | 63 +++++++++++++++++++++ 2 files changed, 99 insertions(+), 3 deletions(-) create mode 100644 crates/ui-egui/tests/sort_dialog_columns.rs diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..59c098343 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -668,7 +668,11 @@ impl Dialog { }], json!({"kind": "blanks"}), ), - "sort" => Dialog::custom("sort", "Sort", json!({"header": true, "levels": [{"column": "A", "order": "asc"}]})), + "sort" => { + // Seed "Sort by" from the column the user is actually in (#1906). + let col = app.session.active().map(|d| gridcraft_engine::core::col_to_letters(d.selection.active.col)).unwrap_or_else(|| "A".into()); + Dialog::custom("sort", "Sort", json!({"header": true, "levels": [{"column": col, "order": "asc"}]})) + } "nameManager" => Dialog::custom("nameManager", "Name Manager", json!({})), "manageRules" => Dialog::custom("manageRules", "Conditional Formatting Rules Manager", json!({})), "commandSearch" => Dialog::custom("commandSearch", "Search Commands", json!({})), @@ -1923,12 +1927,40 @@ fn sort(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut boo ui.checkbox(&mut header, "My list has headers"); d.values.insert("header".into(), json!(header)); let mut levels: Vec = d.values.get("levels").and_then(Json::as_array).cloned().unwrap_or_default(); + // Columns of the region around the active cell, labelled with their header (or first value). + let columns: Vec<(String, String)> = app + .session + .active() + .and_then(|d| { + let sh = d.wb.active()?; + let region = gridcraft_engine::selection::current_region(sh, d.selection.active); + Some( + (region.start.col..=region.end.col) + .map(|c| { + let letter = gridcraft_engine::core::col_to_letters(c); + let label_row = if header { region.start.row + 1 } else { region.start.row }; + let text = gridcraft_engine::display::cell_text(&d.wb, sh, gridcraft_engine::core::CellRef::new(label_row, c)); + let text: String = text.chars().take(24).collect(); + (letter.clone(), if text.is_empty() { letter } else { format!("{letter}: {text}") }) + }) + .collect(), + ) + }) + .unwrap_or_default(); let mut remove = None; for (i, lv) in levels.iter_mut().enumerate() { ui.horizontal(|ui| { ui.label(if i == 0 { "Sort by" } else { "Then by" }); let mut col = lv["column"].as_str().unwrap_or("A").to_string(); - ui.add(egui::TextEdit::singleline(&mut col).desired_width(50.0)); + if columns.is_empty() { + ui.add(egui::TextEdit::singleline(&mut col).desired_width(50.0)); + } else { + egui::ComboBox::from_id_salt(("sort_col", i)).selected_text(col.clone()).show_ui(ui, |ui| { + for (letter, label) in &columns { + ui.selectable_value(&mut col, letter.clone(), label.as_str()); + } + }); + } lv["column"] = json!(col.to_ascii_uppercase()); let mut by = lv["by"].as_str().unwrap_or("values").to_string(); egui::ComboBox::from_id_salt(("by", i)) @@ -1958,7 +1990,8 @@ fn sort(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut boo levels.remove(i); } if ui.button("+ Add Level").clicked() { - levels.push(json!({"column": "B", "order": "asc"})); + let next = columns.get(levels.len()).map(|(l, _)| l.clone()).unwrap_or_else(|| "B".into()); + levels.push(json!({"column": next, "order": "asc"})); } d.values.insert("levels".into(), Json::Array(levels.clone())); let mut open = true; diff --git a/crates/ui-egui/tests/sort_dialog_columns.rs b/crates/ui-egui/tests/sort_dialog_columns.rs new file mode 100644 index 000000000..9a0541b6a --- /dev/null +++ b/crates/ui-egui/tests/sort_dialog_columns.rs @@ -0,0 +1,63 @@ +//! #1906: the Sort dialog seeds "Sort by" from the active column instead of a +//! free-text box stuck on "A", and the column list offers the region's columns. + +use egui::{Key, Modifiers}; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness(session: Session) -> Harness { + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(session, Default::default()), + ); + h.run_steps(4); + h +} + +/// Header row + three rows; the user's cursor sits in column B (Revenue). +fn sales() -> Session { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [["Month", "Revenue"], ["Mar", 30], ["Jan", 10], ["Feb", 20]]})).unwrap(); + s.execute("selection.set", json!({"range": "B2"})).unwrap(); + s +} + +fn col(h: &Harness) -> String { + h.state().dialog.as_ref().unwrap().values["levels"][0]["column"].as_str().unwrap_or("").to_string() +} + +#[test] +fn sort_dialog_seeds_the_active_column() { + let mut h = harness(sales()); + h.state_mut().open_dialog("sort", json!({})); + h.run_steps(2); + assert_eq!(col(&h), "B", "Sort by starts on the column the cursor is in, not A"); +} + +#[test] +fn sorting_by_the_seeded_column_orders_the_rows() { + let mut h = harness(sales()); + h.state_mut().open_dialog("sort", json!({})); + h.run_steps(2); + // Accept the dialog: rows must come back ordered by Revenue (10, 20, 30). + h.input_mut().events.push(egui::Event::Key { + key: Key::Enter, + physical_key: Some(Key::Enter), + pressed: true, + repeat: false, + modifiers: Modifiers::NONE, + }); + h.run_steps(2); + let d = h.state().session.active().unwrap(); + let sh = d.wb.active().unwrap(); + let nums: Vec = (1..4).map(|r| sh.value(gridcraft_engine::core::CellRef::new(r, 1))).filter_map(|v| v.as_f64().map(|n| n as i64)).collect(); + assert_eq!(nums, vec![10, 20, 30], "sort ran on the seeded column B"); +} From 3979e48f89b88f175d6e62f21f1d402ea8218b4d Mon Sep 17 00:00:00 2001 From: Hamza Date: Mon, 19 Oct 2026 22:41:20 +0500 Subject: [PATCH 14/20] Desktop theme: the watcher's link has a type off Linux, so the gate passes there `cargo xtask ci` has not passed on macOS or Windows since cc21643: error: this let-binding has unit value --> crates/ui-egui/src/desktop_theme.rs:36:13 36 | let mut link = Link::default(); `Link` is the session-bus connection the watcher keeps between reads. Off Linux there is no bus to keep, so it was aliased to `()`, which makes that binding a unit binding: `clippy::let_unit_value` reports it and the gate's `-D warnings` turns it into an error. CI runs Linux only, where `Link` is a real `Option`, so nothing catches it there. Off Linux `Link` is now a unit struct rather than `()`. The binding has a type, the lint does not apply, and nothing else changes: `read` still ignores it on those platforms. An `allow` on the line would also work, but a named type says what the thing is rather than silencing a lint that only fires on some platforms. --- crates/ui-egui/src/desktop_theme.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/desktop_theme.rs b/crates/ui-egui/src/desktop_theme.rs index 9d67c337a..35d413b1d 100644 --- a/crates/ui-egui/src/desktop_theme.rs +++ b/crates/ui-egui/src/desktop_theme.rs @@ -67,11 +67,16 @@ impl DesktopTheme { } } -/// The session-bus connection the watcher keeps between reads, opened on first use. +/// The session-bus connection the watcher keeps between reads, opened on first use. Off Linux +/// there is no bus to keep, and a unit struct rather than `()` keeps `let mut link = …` from +/// reading as a unit binding, which `cargo xtask ci` refuses on those platforms. #[cfg(target_os = "linux")] type Link = Option; #[cfg(not(target_os = "linux"))] -type Link = (); +#[derive(Default)] +pub(crate) struct NoLink; +#[cfg(not(target_os = "linux"))] +type Link = NoLink; /// Ask the portal once, over `link`'s connection (opened if absent, dropped if the bus went /// away). D-Bus calls block, so this belongs off the UI thread: the watcher's thread is its From 9f134349729e5b4723b9d6a0113c706e3a0ceea6 Mon Sep 17 00:00:00 2001 From: bfenty <1398236+bfenty@users.noreply.github.com> Date: Mon, 19 Oct 2026 19:40:46 +0000 Subject: [PATCH 15/20] fix(ui): label sort columns from the region start, seed unused level columns - The region already starts at the header row when headers are on, so the dropdown label reads start.row for both cases (was showing the first data value, e.g. B: 30 instead of B: Revenue). - + Add Level picks the first region column no level uses yet instead of columns.get(levels.len()), which re-seeded an already-used column. Co-Authored-By: Claude --- crates/ui-egui/src/dialogs.rs | 14 ++++++++++++-- crates/ui-egui/tests/sort_dialog_columns.rs | 18 ++++++++++++++++++ 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 59c098343..303bae939 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1938,7 +1938,10 @@ fn sort(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut boo (region.start.col..=region.end.col) .map(|c| { let letter = gridcraft_engine::core::col_to_letters(c); - let label_row = if header { region.start.row + 1 } else { region.start.row }; + // The region starts at the header row when one is present, so the + // label always comes from start.row: header text with headers on, + // first data value with them off. + let label_row = region.start.row; let text = gridcraft_engine::display::cell_text(&d.wb, sh, gridcraft_engine::core::CellRef::new(label_row, c)); let text: String = text.chars().take(24).collect(); (letter.clone(), if text.is_empty() { letter } else { format!("{letter}: {text}") }) @@ -1990,7 +1993,14 @@ fn sort(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut boo levels.remove(i); } if ui.button("+ Add Level").clicked() { - let next = columns.get(levels.len()).map(|(l, _)| l.clone()).unwrap_or_else(|| "B".into()); + let used: Vec = levels.iter().filter_map(|l| l.get("column").and_then(Json::as_str).map(str::to_string)).collect(); + let next = columns + .iter() + .map(|(l, _)| l) + .find(|l| !used.contains(l)) + .cloned() + .or_else(|| columns.last().map(|(l, _)| l.clone())) + .unwrap_or_else(|| "B".into()); levels.push(json!({"column": next, "order": "asc"})); } d.values.insert("levels".into(), Json::Array(levels.clone())); diff --git a/crates/ui-egui/tests/sort_dialog_columns.rs b/crates/ui-egui/tests/sort_dialog_columns.rs index 9a0541b6a..fb685c3e0 100644 --- a/crates/ui-egui/tests/sort_dialog_columns.rs +++ b/crates/ui-egui/tests/sort_dialog_columns.rs @@ -2,6 +2,7 @@ //! free-text box stuck on "A", and the column list offers the region's columns. use egui::{Key, Modifiers}; +use egui_kittest::kittest::Queryable; use gridcraft_engine::Session; use gridcraft_ui_egui::SheetApp; use serde_json::json; @@ -42,6 +43,23 @@ fn sort_dialog_seeds_the_active_column() { assert_eq!(col(&h), "B", "Sort by starts on the column the cursor is in, not A"); } +#[test] +fn add_level_picks_a_column_no_level_uses_yet() { + let mut h = harness(sales()); + h.state_mut().open_dialog("sort", json!({})); + h.run_steps(2); + // Level 1 seeded B; clicking "+ Add Level" must not default level 2 to B too (#1906 review). + h.run_steps(6); + h.get_by_label("+ Add Level").click(); + h.run_steps(6); + let cols: Vec = h.state().dialog.as_ref().unwrap().values["levels"] + .as_array() + .map(|a| a.iter().filter_map(|l| l["column"].as_str().map(str::to_string)).collect()) + .unwrap_or_default(); + assert_eq!(cols.len(), 2, "two levels after Add Level"); + assert_ne!(cols[0], cols[1], "the second level defaults to a different column: {cols:?}"); +} + #[test] fn sorting_by_the_seeded_column_orders_the_rows() { let mut h = harness(sales()); From cacb620fecd8e0417ff93e9192daba04318f696d Mon Sep 17 00:00:00 2001 From: dantebertuzzi Date: Tue, 20 Oct 2026 21:26:12 -0300 Subject: [PATCH 16/20] Paste Special: Excel's two-column layout, translated, Paste Link button The dialog was a generic form: one column of fifteen radio buttons and three check boxes, taller than the window, with English labels (the form didn't run them through `tr`). It is now laid out as Excel's: Paste and Operation each in two columns inside a frame, Skip blanks and Transpose side by side under them, and Paste Link as a button that pastes references to the copied cells at once. The letter shortcuts keep working: the options still come from the dialog's fields. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/dialogs.rs | 75 ++++++++++++++++++++ crates/ui-egui/src/i18n/pt.rs | 1 + crates/ui-egui/tests/paste_special_dialog.rs | 56 +++++++++++++++ docs/ui-parity.md | 3 +- 4 files changed, 134 insertions(+), 1 deletion(-) create mode 100644 crates/ui-egui/tests/paste_special_dialog.rs diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index fb899c702..5a7093f28 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1033,6 +1033,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { "recommendedCharts" => 600.0, "analysisToolPak" => 420.0, "solver" => 460.0, + "pasteSpecial" => 480.0, _ => 380.0, }; let window = egui::Window::new(app.ui.language.tr(&d.title)) @@ -1058,6 +1059,7 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { "selectData" => select_data(app, ui, &mut d, &mut confirm, &mut open), "commandSearch" => command_search(app, ui, &mut d, &mut confirm), "goalSeek" => goal_seek(app, ui, &mut d, &mut confirm), + "pasteSpecial" => paste_special(app, ui, &mut d, &mut confirm, &mut open), "analysisToolPak" => analysis_toolpak(app, ui, &mut d, &mut confirm, &mut open), "solver" => solver(ui, &mut d, &mut confirm, &mut open), "solverResults" => solver_results(app, ui, &mut d, &mut open), @@ -1301,6 +1303,79 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { } /// Paste Special's legacy mnemonics select options; Enter still performs the paste. +/// Paste Special as Excel lays it out: Paste and Operation each in two columns of options, Skip +/// blanks and Transpose side by side, and Paste Link as a button that pastes at once. The options +/// come from the dialog's `fields`, so the letter shortcuts and the control channel see the same. +fn paste_special(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, open: &mut bool) { + let lang = app.ui.language; + let t = theme::Tokens::get(ui.ctx()); + let choices: Vec<(&'static str, &'static str, Vec<(&'static str, &'static str)>)> = d + .fields + .iter() + .filter_map(|f| match f { + Field::Choice { key, label, options } => Some((*key, *label, options.clone())), + _ => None, + }) + .collect(); + for (key, label, options) in choices { + ui.add_space(4.0); + ui.label(egui::RichText::new(lang.tr(label)).strong()); + egui::Frame::new().stroke(Stroke::new(1.0, t.separator)).corner_radius(4.0).inner_margin(egui::Margin::symmetric(10, 6)).show(ui, |ui| { + ui.set_min_width(ui.available_width()); + let cur = d.values.get(key).and_then(Json::as_str).unwrap_or("").to_string(); + // Column by column, as Excel fills them. + let rows = options.len().div_ceil(2); + egui::Grid::new(("paste_special", key)).num_columns(2).min_col_width(200.0).spacing(vec2(16.0, 4.0)).show(ui, |ui| { + for r in 0..rows { + for c in 0..2 { + if let Some((value, text)) = options.get(c * rows + r) { + if ui.radio(cur == *value, lang.tr(text)).clicked() { + d.set_value(key, json!(value)); + } + } else { + ui.label(""); + } + } + ui.end_row(); + } + }); + }); + } + ui.add_space(8.0); + // In the same columns as the options above. + egui::Frame::new().inner_margin(egui::Margin::symmetric(11, 0)).show(ui, |ui| { + egui::Grid::new("paste_special_checks").num_columns(2).min_col_width(200.0).spacing(vec2(16.0, 4.0)).show(ui, |ui| { + for (key, label) in [("skipBlanks", "Skip blanks"), ("transpose", "Transpose (E)")] { + let mut b = d.values.get(key).and_then(Json::as_bool).unwrap_or(false); + if ui.checkbox(&mut b, lang.tr(label)).changed() { + d.values.insert(key.into(), json!(b)); + } + } + ui.end_row(); + }); + }); + ui.add_space(8.0); + ui.separator(); + ui.horizontal(|ui| { + // Excel's Paste Link pastes references to the copied cells straight away. + if ui.button(lang.tr("Paste Link")).on_hover_text(lang.tr("Paste formulas that refer to the copied cells")).clicked() { + d.values.insert("what".into(), json!("all")); + d.values.insert("operation".into(), json!("none")); + d.values.insert("link".into(), json!(true)); + *confirm = true; + } + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + let ok = ui.add(egui::Button::new(egui::RichText::new(format!(" {} ", lang.tr("OK"))).color(Color32::WHITE)).fill(t.accent)); + if ok.clicked() || ui.input(|i| i.key_pressed(Key::Enter)) { + *confirm = true; + } + if ui.button(format!(" {} ", lang.tr("Cancel"))).clicked() { + *open = false; + } + }); + }); +} + fn paste_special_shortcuts(d: &mut Dialog, ctx: &egui::Context) { if ctx.text_edit_focused() { return; diff --git a/crates/ui-egui/src/i18n/pt.rs b/crates/ui-egui/src/i18n/pt.rs index fb9236cdb..4e78fe728 100644 --- a/crates/ui-egui/src/i18n/pt.rs +++ b/crates/ui-egui/src/i18n/pt.rs @@ -426,6 +426,7 @@ pub(super) fn translate(text: &str) -> Option<&'static str> { "Select a chart first." => "Selecione um gráfico primeiro.", "Text Box" => "Caixa de Texto", "Select Data" => "Selecionar Dados", + "Paste formulas that refer to the copied cells" => "Colar fórmulas que se referem às células copiadas", "Row Height" => "Altura da Linha", "Column Width" => "Largura da Coluna", "Standard Width" => "Largura Padrão", diff --git a/crates/ui-egui/tests/paste_special_dialog.rs b/crates/ui-egui/tests/paste_special_dialog.rs new file mode 100644 index 000000000..f74a34c88 --- /dev/null +++ b/crates/ui-egui/tests/paste_special_dialog.rs @@ -0,0 +1,56 @@ +//! Paste Special: options in two columns, translated, OK pastes with them and Paste Link links. + +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::Session; +use gridcraft_ui_egui::SheetApp; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +fn harness() -> Harness { + let mut s = Session::new(); + s.new_workbook(); + s.execute("range.setValues", json!({"range": "A1", "values": [[2], [3]]})).unwrap(); + s.execute("range.setValues", json!({"range": "C1", "values": [[10], [10]]})).unwrap(); + s.execute("edit.copy", json!({"range": "A1:A2"})).unwrap(); + s.execute("selection.set", json!({"cell": "C1"})).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + SheetApp::new(s, Default::default()), + ); + h.state_mut().open_dialog("pasteSpecial", json!({})); + h.run_steps(4); + h +} + +fn cell(h: &mut Harness, a1: &str) -> serde_json::Value { + h.state_mut().session.execute("cell.get", json!({"cell": a1})).unwrap() +} + +#[test] +fn an_operation_chosen_in_the_dialog_is_applied() { + let mut h = harness(); + h.get_by_label("Values (V)").click(); + h.get_by_label("Multiply").click(); + h.run_steps(3); + h.get_by_label_contains("OK").click(); + h.run_steps(3); + assert!(h.state().dialog.is_none()); + assert_eq!(cell(&mut h, "C1")["value"], json!(20.0)); + assert_eq!(cell(&mut h, "C2")["value"], json!(30.0)); +} + +#[test] +fn paste_link_pastes_references_at_once() { + let mut h = harness(); + h.get_by_label("Paste Link").click(); + h.run_steps(3); + assert!(h.state().dialog.is_none()); + let c1 = cell(&mut h, "C1"); + assert_eq!(c1["value"], json!(2.0)); + assert!(c1["formula"].as_str().is_some_and(|f| f.contains("A1")), "{c1}"); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 34cb502f5..ad05c35e9 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-19 · **Change:** minor (Ctrl+Enter fill gives each cell the automatic number format typing gives it; keytip letters typed with Alt held, #1846) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4 +> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-21 · **Change:** minor (Paste Special in Excel's two-column layout) · **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-21 | minor | Paste Special laid out as Excel's: Paste and Operation options each in two columns inside a frame, Skip blanks and Transpose side by side, Paste Link as a button that pastes references at once; labels translated (the generic form didn't translate them) and the window no longer runs past the screen | | 2026-10-19 | minor | Ctrl+Enter (`range.fill`) gives each cell what typing the input there gives: a General cell gets the automatic number format (`5%`, `$5`, `3/4/2026`, `=TODAY()`) even when it already held a value, and a cell with its own format, a formatted row or column, or Text keeps it. Cells that held a value used to keep General (`5%` showed 0.05), and empty cells took the active cell's format | | 2026-10-18 | minor | Keytip letters typed with left Alt held continue the sequence (Alt+H, O, A / Alt+H, O, I ran nothing before: Alt+letter cancelled keytips); other Alt combinations, AltGr and macOS Option are unchanged (#1846) | | 2026-10-12 | minor | The File view's recent list tells when each workbook was last changed, read off the UI thread | From c9941763ccda12549b69d8c474e9e7fa3384afad Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 02:36:38 +0000 Subject: [PATCH 17/20] Integration: #2093 move the paste_special_shortcuts doc comment back onto its function --- crates/ui-egui/src/dialogs.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index aebc853e3..412bd3951 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1384,7 +1384,6 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { } } -/// Paste Special's legacy mnemonics select options; Enter still performs the paste. /// Paste Special as Excel lays it out: Paste and Operation each in two columns of options, Skip /// blanks and Transpose side by side, and Paste Link as a button that pastes at once. The options /// come from the dialog's `fields`, so the letter shortcuts and the control channel see the same. @@ -1458,6 +1457,7 @@ fn paste_special(app: &mut SheetApp, ui: &mut egui::Ui, d: &mut Dialog, confirm: }); } +/// Paste Special's legacy mnemonics select options; Enter still performs the paste. fn paste_special_shortcuts(d: &mut Dialog, ctx: &egui::Context) { if ctx.text_edit_focused() { return; From 31d62214635295d9def146d70d5af6a92cf11c9b Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 02:37:01 +0000 Subject: [PATCH 18/20] Integration: #1725 prefill Remove Duplicates with the current region when only the cursor is placed --- crates/ui-egui/src/dialogs.rs | 29 ++++++++++++++++++++++++++++- 1 file changed, 28 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 412bd3951..33dccd0ee 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -458,7 +458,7 @@ impl Dialog { Check { key: "header", label: "My data has headers" }, Text { key: "columnsText", label: "Columns (blank = all, e.g. A,C):" }, ], - json!({"range": sel, "header": true}), + json!({"range": remove_duplicates_range(app, &sel), "header": true}), ), "advancedFilter" => { let list = app @@ -875,6 +875,22 @@ fn validation_defaults(app: &SheetApp) -> Json { v } +/// The range Remove Duplicates starts with. A block selection is used as given; with only the +/// cursor placed, the table's data rows or the current region, as the command picks when it gets +/// no range (a single-cell range would remove nothing). +fn remove_duplicates_range(app: &SheetApp, sel: &str) -> String { + let Some(d) = app.session.active() else { return sel.to_string() }; + let Some(sh) = d.wb.active() else { return sel.to_string() }; + if !d.selection.current().is_single() { + return sel.to_string(); + } + let range = match sh.table_at(d.selection.active) { + Some(t) => t.data_range().map(|dr| gridcraft_engine::core::RangeRef::new(t.range.start, dr.end)).unwrap_or(t.range), + None => gridcraft_engine::selection::current_region(sh, d.selection.active), + }; + range.a1() +} + fn abs(r: &gridcraft_engine::core::RangeRef) -> String { let a = |c: gridcraft_engine::core::CellRef| format!("${}${}", gridcraft_engine::core::col_to_letters(c.col), c.row + 1); if r.is_single() { a(r.start) } else { format!("{}:{}", a(r.start), a(r.end)) } @@ -2958,6 +2974,17 @@ mod tests { assert_eq!(build_params(&dialog)["range"], "D2:E6"); } + #[test] + fn remove_duplicates_with_only_the_cursor_prefills_the_current_region() { + let mut session = gridcraft_engine::Session::default(); + session.new_workbook(); + session.execute("range.setValues", json!({"range": "B2:C4", "values": [["a", 1], ["a", 1], ["b", 2]]})).unwrap(); + session.execute("selection.set", json!({"range": "C3"})).unwrap(); + let mut app = SheetApp::new(session, Default::default()); + let dialog = Dialog::open(&mut app, "removeDuplicates", json!({})).unwrap(); + assert_eq!(dialog.values.get("range"), Some(&json!("B2:C4"))); + } + #[test] fn data_validation_opens_on_the_rule_of_the_active_cell() { let mut session = gridcraft_engine::Session::default(); From 1b4b51225d97e51151738371e48d9ca439163ff3 Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 02:47:32 +0000 Subject: [PATCH 19/20] Integration: #1912 restore the end of add_page lost in the merge resolution --- crates/pdf/src/lib.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/pdf/src/lib.rs b/crates/pdf/src/lib.rs index d1e7d2d06..e00b4fcc3 100644 --- a/crates/pdf/src/lib.rs +++ b/crates/pdf/src/lib.rs @@ -678,6 +678,8 @@ impl PdfDoc { let width = if width_pt.is_finite() { width_pt.clamp(3.0, 14_400.0) } else { 612.0 }; let height = if height_pt.is_finite() { height_pt.clamp(3.0, 14_400.0) } else { 792.0 }; self.pages.push(Page { width, height, content: String::new(), fonts: [false; FONT_SLOTS], images: Vec::new(), links: Vec::new(), depth: 0 }); + let i = self.pages.len() - 1; + &mut self.pages[i] } pub fn page_count(&self) -> usize { From ec61d59b4165920cbc2368ef7a19e0abf558ad7f Mon Sep 17 00:00:00 2001 From: aiongg Date: Thu, 22 Oct 2026 02:50:46 +0000 Subject: [PATCH 20/20] Integration: cargo fmt --- crates/engine/src/tests.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index e4d39ffeb..ae96586a4 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -3006,10 +3006,7 @@ fn sort_and_remove_duplicates_move_the_rows_notes_and_links() { let notes_and_links = |s: &Session| { let sh = s.doc().unwrap().wb.active().unwrap().clone(); let cells = ["B2", "B3", "B4", "B5"].map(|c| CellRef::parse(c).unwrap()); - ( - cells.map(|c| sh.comments.get(&c).map(|m| m.text.clone())), - cells.map(|c| sh.hyperlinks.get(&c).map(|h| h.target.clone())), - ) + (cells.map(|c| sh.comments.get(&c).map(|m| m.text.clone())), cells.map(|c| sh.hyperlinks.get(&c).map(|h| h.target.clone()))) }; // Sort ascending with unique keys d,a,e,b,c: the order becomes a(2), b(4), c(5), d(1), e(3).