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] 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`) |