From e40291fc3d0866d094b4e54871e4514aaffc9253 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Mon, 26 Oct 2026 00:38:32 -0400 Subject: [PATCH] Macro interface: follow per-window dialogs and egui 0.37 accessibility Main now keeps a dialog with its own workbook window, so the step editor is parked with its workbook rather than drawn over another one, and closing that workbook drops the editor without drawing it again: forget_closed_editor clears it each frame. The Macros, Record Macro and editor text boxes are labelled by their captions, and the tests pick the question's Cancel and the chart menu's Delete by role, as the formula bar and ribbon share those names. --- crates/ui-egui/src/lib.rs | 1 + crates/ui-egui/src/macro_editor.rs | 17 +++++++++------ crates/ui-egui/src/macro_ui.rs | 29 ++++++++++++++++--------- crates/ui-egui/tests/it/chart_resize.rs | 3 ++- crates/ui-egui/tests/it/macro_editor.rs | 14 +++++++----- 5 files changed, 41 insertions(+), 23 deletions(-) diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 26c53e337..334399d8b 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -1797,6 +1797,7 @@ impl SheetApp { }); } dialogs::show(self, &ctx); + macro_ui::forget_closed_editor(self); macro_ui::question_box(self, &ctx); widgets::message_box(self, &ctx); widgets::toast(self, &ctx); diff --git a/crates/ui-egui/src/macro_editor.rs b/crates/ui-egui/src/macro_editor.rs index 087675f7c..733921407 100644 --- a/crates/ui-egui/src/macro_editor.rs +++ b/crates/ui-egui/src/macro_editor.rs @@ -1074,13 +1074,16 @@ fn body(app: &mut SheetApp, ui: &mut Ui, d: &mut Dialog, ed: &mut Editor) { let debugging = app.session.macro_debug.is_some(); // Name, description, Save; they wrap in a narrow window. ui.horizontal_wrapped(|ui| { - ui.label("Name:"); - let name = ui.add_enabled(!debugging, egui::TextEdit::singleline(&mut ed.draft.name).desired_width(180.0).id_salt("me_name")); - ui.label("Description:"); - let desc = ui.add_enabled( - !debugging, - egui::TextEdit::singleline(&mut ed.draft.description).desired_width((ui.available_width() - 90.0).max(80.0)).id_salt("me_desc"), - ); + let label = ui.label("Name:"); + let name = + ui.add_enabled(!debugging, egui::TextEdit::singleline(&mut ed.draft.name).desired_width(180.0).id_salt("me_name")).labelled_by(label.id); + let label = ui.label("Description:"); + let desc = ui + .add_enabled( + !debugging, + egui::TextEdit::singleline(&mut ed.draft.description).desired_width((ui.available_width() - 90.0).max(80.0)).id_salt("me_desc"), + ) + .labelled_by(label.id); if name.changed() || desc.changed() { ed.dirty = true; } diff --git a/crates/ui-egui/src/macro_ui.rs b/crates/ui-egui/src/macro_ui.rs index 85d8de17f..4277721b8 100644 --- a/crates/ui-egui/src/macro_ui.rs +++ b/crates/ui-egui/src/macro_ui.rs @@ -181,6 +181,15 @@ pub fn note_result(app: &mut SheetApp, id: &str, params: &Json, v: &Json) { app.toast = Some((text, crate::now_ms())); } +/// Drops the step editor once its workbook is closed. The editor's dialog stays with its +/// workbook's window, so it may not be drawn again to notice. +pub fn forget_closed_editor(app: &mut SheetApp) { + if app.macros.editor.as_ref().is_some_and(|ed| !app.session.documents().iter().any(|d| d.uid == ed.doc)) { + app.macros.editor = None; + app.grid.macro_overlay = None; + } +} + /// The prompt for [`MacroUi::question`]: its text, a button for each choice and Cancel. pub fn question_box(app: &mut SheetApp, ctx: &egui::Context) { if app.message.is_some() { @@ -396,8 +405,8 @@ pub fn macros_dialog(app: &mut SheetApp, ui: &mut Ui, d: &mut Dialog, open: &mut ui.horizontal_top(|ui| { ui.vertical(|ui| { ui.set_width(330.0); - ui.label("Macro name:"); - if ui.add(egui::TextEdit::singleline(&mut name).desired_width(330.0).id_salt("macro_name")).changed() { + let label = ui.label("Macro name:"); + if ui.add(egui::TextEdit::singleline(&mut name).desired_width(330.0).id_salt("macro_name")).labelled_by(label.id).changed() { kind = Kind::Macro; } egui::Frame::NONE.fill(t.input_bg).stroke(egui::Stroke::new(1.0, t.input_border)).inner_margin(2.0).show(ui, |ui| { @@ -528,9 +537,9 @@ fn options(app: &mut SheetApp, ui: &mut Ui, d: &mut Dialog) { ui.label(egui::RichText::new(c).color(Tokens::get(ui.ctx()).danger)); } ui.add_space(4.0); - ui.label("Description:"); + let label = ui.label("Description:"); let mut desc = d.values.get("description").and_then(Json::as_str).unwrap_or("").to_string(); - ui.add(egui::TextEdit::multiline(&mut desc).desired_rows(3).desired_width(f32::INFINITY).id_salt("macro_description")); + ui.add(egui::TextEdit::multiline(&mut desc).desired_rows(3).desired_width(f32::INFINITY).id_salt("macro_description")).labelled_by(label.id); ui.add_space(6.0); let mut done = false; ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { @@ -551,8 +560,8 @@ fn options(app: &mut SheetApp, ui: &mut Ui, d: &mut Dialog) { /// The shortcut letter box, after "Ctrl+Shift+" (or "⌘⇧" on a Mac). Keeps one capital letter. fn shortcut_field(ui: &mut Ui, key: &mut String, id: &str) { ui.horizontal(|ui| { - ui.label(if ui.ctx().os() == egui::os::OperatingSystem::Mac { "⌘ ⇧ +" } else { "Ctrl+Shift+" }); - ui.add(egui::TextEdit::singleline(key).char_limit(1).desired_width(24.0).id_salt(id)); + let label = ui.label(if ui.ctx().os() == egui::os::OperatingSystem::Mac { "⌘ ⇧ +" } else { "Ctrl+Shift+" }); + ui.add(egui::TextEdit::singleline(key).char_limit(1).desired_width(24.0).id_salt(id)).labelled_by(label.id); }); *key = key.to_ascii_uppercase(); } @@ -620,14 +629,14 @@ pub fn record_dialog(app: &mut SheetApp, ui: &mut Ui, d: &mut Dialog, open: &mut let mut name = get(d, "name"); let mut key = get(d, "shortcut"); let mut desc = get(d, "description"); - ui.label("Macro name:"); - ui.add(egui::TextEdit::singleline(&mut name).desired_width(f32::INFINITY).id_salt("record_name")); + let label = ui.label("Macro name:"); + ui.add(egui::TextEdit::singleline(&mut name).desired_width(f32::INFINITY).id_salt("record_name")).labelled_by(label.id); ui.add_space(4.0); ui.label("Shortcut key:"); shortcut_field(ui, &mut key, "record_shortcut"); ui.add_space(4.0); - ui.label("Description:"); - ui.add(egui::TextEdit::multiline(&mut desc).desired_rows(3).desired_width(f32::INFINITY).id_salt("record_description")); + let label = ui.label("Description:"); + ui.add(egui::TextEdit::multiline(&mut desc).desired_rows(3).desired_width(f32::INFINITY).id_salt("record_description")).labelled_by(label.id); ui.checkbox(&mut app.macros.relative, "Use Relative References"); let problem = record_problem(app, &name, &key); if let Some(p) = &problem { diff --git a/crates/ui-egui/tests/it/chart_resize.rs b/crates/ui-egui/tests/it/chart_resize.rs index a2c36e3f3..e2fc537ac 100644 --- a/crates/ui-egui/tests/it/chart_resize.rs +++ b/crates/ui-egui/tests/it/chart_resize.rs @@ -152,7 +152,8 @@ fn right_clicking_a_chart_opens_its_menu_not_the_cells() { let (mut h, id) = fixture(); let inside = corner(&h, id) - vec2(100.0, 80.0); right_click(&mut h, inside, vec2(0.0, 0.0)); - h.get_by_role_and_label(Role::Button, "Delete").click(); + // Home › Cells › Delete has the same accessible name; the menu is drawn over the ribbon, last. + h.get_all_by_role_and_label(Role::Button, "Delete").last().unwrap().click(); h.run_steps(3); assert!(h.state().session.doc().unwrap().wb.active().unwrap().charts.is_empty()); assert!(h.state().grid.object_menu.is_none()); diff --git a/crates/ui-egui/tests/it/macro_editor.rs b/crates/ui-egui/tests/it/macro_editor.rs index 33d3861e4..f2130fcb6 100644 --- a/crates/ui-egui/tests/it/macro_editor.rs +++ b/crates/ui-egui/tests/it/macro_editor.rs @@ -386,12 +386,15 @@ fn the_editor_works_only_on_the_workbook_it_opened_in() { h.state_mut().message = None; h.state_mut().macros.editor = Some(ed); h.run_steps(3); - assert!(h.query_by_label_contains("Switch back to it").is_some(), "the editor says whose macro it is"); - // Back in its own workbook, the editor saves there. + // The editor stays with its own workbook's window: it is not drawn over the other one. + assert!(h.state().dialog.as_ref().is_none_or(|d| d.name() != "macroEditor"), "the editor is not shown here"); + // Back in its own workbook, the editor shows again and saves there. + let i = h.state().session.documents().iter().position(|d| d.uid == book1).unwrap(); + h.state_mut().session.set_active(i); + h.run_steps(3); + assert!(h.state().dialog.as_ref().is_some_and(|d| d.name() == "macroEditor"), "the editor is back with its workbook"); let mut ed = h.state_mut().macros.editor.take().unwrap(); let app = h.state_mut(); - let i = app.session.documents().iter().position(|d| d.uid == book1).unwrap(); - app.session.set_active(i); assert!(gridcraft_ui_egui::macro_editor::save(app, &mut ed)); assert_eq!(app.session.active().unwrap().wb.macros.iter().map(|m| m.name.as_str()).collect::>(), ["Fresh"]); app.macros.editor = Some(ed); @@ -435,7 +438,8 @@ fn quitting_asks_about_the_editors_unsaved_steps_first() { assert!(!sent.contains(&egui::ViewportCommand::Close), "{sent:?}"); assert!(h.query_by_label_contains("Save changes to macro “Unsaved”").is_some()); // Cancel keeps the app and the draft. - h.get_by_label("Cancel").click(); + // The formula bar's Cancel button has the same name; the question's is the button drawn last. + h.get_all_by_role_and_label(egui::Role::Button, "Cancel").last().unwrap().click(); let sent = step_commands(&mut h); assert!(!sent.contains(&egui::ViewportCommand::Close), "{sent:?}"); assert_eq!(h.state().macros.editor.as_ref().map(|e| e.draft.name.as_str()), Some("Unsaved"));