From b09a5502e3a7a092561d13aaa262881b7f3d05c2 Mon Sep 17 00:00:00 2001 From: prxnvt <53838599+prxnvt@users.noreply.github.com> Date: Sat, 24 Oct 2026 01:30:15 -0400 Subject: [PATCH] UI: the toolbar's Undo and Redo follow the commands' own availability The ribbon enabled them from the history lengths alone, so while an undo group was open on the document (an action spanning several calls) a click ran edit.undo and got its refusal. The buttons now ask cmd::can_undo and cmd::can_redo, the undo list is empty while a group is open, and the two buttons carry a name and their enabled state for assistive technology. A UI test opens a group, clicks both and checks the history and their state. --- crates/ui-egui/src/ribbon.rs | 22 +++++-- crates/ui-egui/tests/it/main.rs | 1 + .../ui-egui/tests/it/undo_while_grouping.rs | 57 +++++++++++++++++++ 3 files changed, 74 insertions(+), 6 deletions(-) create mode 100644 crates/ui-egui/tests/it/undo_while_grouping.rs diff --git a/crates/ui-egui/src/ribbon.rs b/crates/ui-egui/src/ribbon.rs index 45856565f..ab9cc311d 100644 --- a/crates/ui-egui/src/ribbon.rs +++ b/crates/ui-egui/src/ribbon.rs @@ -74,15 +74,23 @@ pub fn title_bar(app: &mut SheetApp, ui: &mut Ui) { { app.run_or_alert("file.save", json!({})); } - let can_undo = app.session.active().is_some_and(|d| !d.undo.is_empty()); - let can_redo = app.session.active().is_some_and(|d| !d.redo.is_empty()); - if icon_button(ui, Icon::Undo, if can_undo { t.text_dim } else { t.text_disabled }, "Undo", vec2(26.0, ch)).clicked() && can_undo { + // The commands' own availability: there must be a step, and no undo group may be open on + // the document (an action that spans several calls is still running). + let can_undo = gridcraft_engine::cmd::can_undo(&app.session).is_ok(); + let can_redo = gridcraft_engine::cmd::can_redo(&app.session).is_ok(); + // Named and marked enabled or not for assistive technology (and the UI tests). + let undo = icon_button(ui, Icon::Undo, if can_undo { t.text_dim } else { t.text_disabled }, "Undo", vec2(26.0, ch)); + undo.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, can_undo, "Undo")); + if undo.clicked() && can_undo { app.run_or_alert("edit.undo", json!({})); } let undo_list = icon_button(ui, Icon::Chevron, t.text_dim, "Undo list", vec2(14.0, ch)); egui::Popup::menu(&undo_list).show(|ui| { - let labels: Vec = - app.session.active().map(|d| d.undo.iter().rev().take(20).map(|e| e.label.clone()).collect()).unwrap_or_default(); + let labels: Vec = if can_undo { + app.session.active().map(|d| d.undo.iter().rev().take(20).map(|e| e.label.clone()).collect()).unwrap_or_default() + } else { + Vec::new() + }; if labels.is_empty() { ui.label("Can't Undo"); } @@ -92,7 +100,9 @@ pub fn title_bar(app: &mut SheetApp, ui: &mut Ui) { } } }); - if icon_button(ui, Icon::Redo, if can_redo { t.text_dim } else { t.text_disabled }, "Redo", vec2(26.0, ch)).clicked() && can_redo { + let redo = icon_button(ui, Icon::Redo, if can_redo { t.text_dim } else { t.text_disabled }, "Redo", vec2(26.0, ch)); + redo.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, can_redo, "Redo")); + if redo.clicked() && can_redo { app.run_or_alert("edit.redo", json!({})); } let more = icon_button(ui, Icon::More, t.text_dim, "More commands", vec2(26.0, ch)); diff --git a/crates/ui-egui/tests/it/main.rs b/crates/ui-egui/tests/it/main.rs index fc00fb520..b9725aed0 100644 --- a/crates/ui-egui/tests/it/main.rs +++ b/crates/ui-egui/tests/it/main.rs @@ -85,6 +85,7 @@ mod touch_pan; mod trace_navigation; mod translated_form_fields; mod ui_clock; +mod undo_while_grouping; mod validation_alert; mod validation_input_message; mod wheel_zoom; diff --git a/crates/ui-egui/tests/it/undo_while_grouping.rs b/crates/ui-egui/tests/it/undo_while_grouping.rs new file mode 100644 index 000000000..fdc98ebf9 --- /dev/null +++ b/crates/ui-egui/tests/it/undo_while_grouping.rs @@ -0,0 +1,57 @@ +//! Undo and Redo on the Quick Access Toolbar follow the commands' own availability: while an undo +//! group is open on the document (an action that spans several calls is still running), both +//! buttons are off, and a click neither runs the command nor raises its refusal. + +use egui_kittest::kittest::{NodeT as _, 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("cell.set", json!({"cell": "A1", "input": "1"})).unwrap(); + s.execute("cell.set", json!({"cell": "A1", "input": "2"})).unwrap(); + s.execute("edit.undo", json!({})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state( + |ui, app: &mut SheetApp| { + app.logic(ui.ctx()); + app.ui(ui); + }, + SheetApp::new(s, Default::default()), + ); + h.run_steps(4); + h +} + +fn history(h: &Harness) -> (usize, usize) { + let d = h.state().session.active().unwrap(); + (d.undo.len(), d.redo.len()) +} + +#[test] +fn undo_and_redo_wait_while_an_undo_group_is_open() { + let mut h = harness(); + assert_eq!(history(&h), (1, 1)); + let token = h.state_mut().session.group_begin("Long action").unwrap(); + h.run_steps(2); + for button in ["Undo", "Redo"] { + assert!(h.get_by_label(button).accesskit_node().is_disabled(), "{button} is off while the group is open"); + h.get_by_label(button).click(); + h.run_steps(2); + assert!(h.state().message.is_none(), "{button}: no refusal is raised while the group is open"); + assert_eq!(history(&h), (1, 1), "{button}: the history is as it was"); + } + h.state_mut().session.group_end(token).unwrap(); + h.run_steps(2); + assert!(!h.get_by_label("Undo").accesskit_node().is_disabled() && !h.get_by_label("Redo").accesskit_node().is_disabled()); + // With the group ended, the buttons work again. + h.get_by_label("Undo").click(); + h.run_steps(2); + assert_eq!(history(&h), (0, 2)); + h.get_by_label("Redo").click(); + h.run_steps(2); + assert_eq!(history(&h), (1, 1)); +}