From 226c055fb7fb269786524dd095b008a3df39fd64 Mon Sep 17 00:00:00 2001 From: Vedran Dobos <19259909+vedran-dobos@users.noreply.github.com> Date: Tue, 20 Oct 2026 16:49:21 +0200 Subject: [PATCH] fix(ui): Esc and Enter close a data validation alert, and the keyboard goes back to the entry (#2089) A data validation alert is the general message box, drawn over the cell editor, which stays open so the entry can be fixed. The editor read Enter and Esc first: the first Esc cancelled the edit behind the alert and only the second closed it, and Enter tried the entry again so the alert came back. The editor now skips its own key handling while a message is open over it, so Esc and Enter close the alert (like its OK button). The message box also had the keyboard focus and didn't give it back, so typing went nowhere until the cell was clicked; closing it now asks the open editor for focus again. Tests: ui-egui tests/validation_alert.rs esc_closes_a_validation_alert_and_leaves_the_entry and enter_closes_a_validation_alert_and_leaves_the_entry (both fail on main, and without the focus change). Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/formula_bar.rs | 4 +- crates/ui-egui/src/widgets.rs | 5 ++ crates/ui-egui/tests/validation_alert.rs | 105 +++++++++++++++++++++++ docs/ui-parity.md | 3 +- 4 files changed, 115 insertions(+), 2 deletions(-) create mode 100644 crates/ui-egui/tests/validation_alert.rs diff --git a/crates/ui-egui/src/formula_bar.rs b/crates/ui-egui/src/formula_bar.rs index 7f1ad87fc..e0038b77d 100644 --- a/crates/ui-egui/src/formula_bar.rs +++ b/crates/ui-egui/src/formula_bar.rs @@ -163,7 +163,9 @@ pub fn editor_widget(app: &mut SheetApp, ui: &mut egui::Ui, id: egui::Id, font: let has_focus = focused == Some(id) || (mine && (ed.request_focus || focused.is_none())); let mut action: Option<(i64, i64, bool, bool)> = None; let mut cancel = false; - if has_focus && mine { + // A message box over the editor (a data validation alert, a formula error) gets Enter and Esc; + // the editor used to take them, so the first Esc cancelled the edit behind the message. + if has_focus && mine && app.message.is_none() { let (enter, tab, esc, alt_enter, cmd_enter, ctrl_shift_enter, shift) = ui.input_mut(|i| { let m = i.modifiers; let alt_enter = diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index e24105b49..ef40c764a 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -293,6 +293,11 @@ pub fn message_box(app: &mut SheetApp, ctx: &egui::Context) { }); if close { app.message = None; + // Back to the entry the message was about (a validation alert, a formula error), so it can + // be fixed from the keyboard without clicking the cell again. + if let Some(ed) = &mut app.editor { + ed.request_focus = true; + } } } diff --git a/crates/ui-egui/tests/validation_alert.rs b/crates/ui-egui/tests/validation_alert.rs new file mode 100644 index 000000000..eb80822c9 --- /dev/null +++ b/crates/ui-egui/tests/validation_alert.rs @@ -0,0 +1,105 @@ +//! A data validation alert over the cell editor answers Enter and Esc itself, and closing it +//! puts the keyboard back in the entry so it can be fixed straight away. + +use egui::{Event, Key, Modifiers}; +use egui_kittest::kittest::Queryable as _; +use gridcraft_engine::Session; +use gridcraft_engine::core::{CellRef, Value}; +use gridcraft_ui_egui::SheetApp; +use gridcraft_ui_egui::i18n::Language; +use serde_json::json; + +type Harness = egui_kittest::Harness<'static, SheetApp>; + +const MESSAGE: &str = "This value doesn't match the data validation restrictions defined for this cell."; + +/// A1 holds 5 and accepts whole numbers from 1 to 10 (Stop alert). +fn harness() -> Harness { + let mut s = Session::new(); + s.new_workbook(); + s.execute("cell.set", json!({"cell": "A1", "input": "5"})).unwrap(); + s.execute("data.validation", json!({"range": "A1", "type": "whole", "operator": "between", "formula1": "1", "formula2": "10"})).unwrap(); + s.execute("selection.set", json!({"range": "A1"})).unwrap(); + let mut app = SheetApp::new(s, Default::default()); + app.ui.language = Language::En; + 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); + }, + app, + ); + h.run_steps(4); + h +} + +fn key(h: &mut Harness, k: Key) { + h.input_mut().events.push(Event::Key { key: k, physical_key: Some(k), pressed: true, repeat: false, modifiers: Modifiers::NONE }); + h.input_mut().events.push(Event::Key { key: k, physical_key: Some(k), pressed: false, repeat: false, modifiers: Modifiers::NONE }); + h.run_steps(2); +} + +/// Types `50` into A1 and presses Enter, which opens the alert. +fn enter_50(h: &mut Harness) { + h.state_mut().begin_edit(Some("50".into()), false); + h.run_steps(2); + key(h, Key::Enter); + assert!(alert_shown(h)); +} + +/// Types text the way a keyboard does (into whatever has focus). +fn type_text(h: &mut Harness, text: &str) { + h.input_mut().events.push(Event::Text(text.into())); + h.run_steps(2); +} + +fn alert_shown(h: &Harness) -> bool { + h.query_by_label(MESSAGE).is_some() +} + +fn entry(h: &Harness) -> Option { + h.state().editor.as_ref().map(|e| e.text.clone()) +} + +fn a1(h: &Harness) -> Value { + h.state().session.active().unwrap().wb.sheet(0).unwrap().value(CellRef::new(0, 0)) +} + +/// The first Esc closes the alert and the entry is still there to fix, with the keyboard in it; a +/// second Esc cancels the edit as usual. Before, the first Esc cancelled the edit behind the alert +/// and the second closed it, and then typing went nowhere until the cell was clicked. +#[test] +fn esc_closes_a_validation_alert_and_leaves_the_entry() { + let mut h = harness(); + enter_50(&mut h); + key(&mut h, Key::Escape); + assert!(!alert_shown(&h), "one Esc closes the alert"); + assert_eq!(entry(&h), Some("50".to_string()), "the entry is still open behind it"); + type_text(&mut h, "9"); + assert_eq!(entry(&h), Some("509".to_string()), "typing goes into the entry"); + key(&mut h, Key::Escape); + assert_eq!(entry(&h), None, "the next Esc cancels the edit"); + assert_eq!(a1(&h), Value::Number(5.0)); +} + +/// Enter closes the alert too, without touching the entry, and the entry can be fixed and +/// committed from the keyboard. Before, the editor took Enter, tried the same entry again and the +/// alert stayed. +#[test] +fn enter_closes_a_validation_alert_and_leaves_the_entry() { + let mut h = harness(); + enter_50(&mut h); + key(&mut h, Key::Enter); + assert!(!alert_shown(&h), "Enter closes the alert"); + assert_eq!(entry(&h), Some("50".to_string())); + assert_eq!(a1(&h), Value::Number(5.0)); + key(&mut h, Key::Backspace); + key(&mut h, Key::Backspace); + type_text(&mut h, "8"); + assert_eq!(entry(&h), Some("8".to_string()), "the entry is fixed from the keyboard"); + key(&mut h, Key::Enter); + assert_eq!(entry(&h), None); + assert_eq!(a1(&h), Value::Number(8.0), "the fixed entry goes in"); + assert!(!alert_shown(&h)); +} diff --git a/docs/ui-parity.md b/docs/ui-parity.md index 34cb502f5..103df0238 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-20 · **Change:** minor (Esc and Enter close a data validation alert over the cell editor; 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 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-20 | minor | Esc and Enter close a message over the cell editor, such as a data validation alert, and the entry stays open to fix, with the keyboard back in it. Before, the editor took them (the first Esc cancelled the edit behind the alert and only the second closed it), and after the message typing went nowhere until the cell was clicked | | 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 |