collective
Browse / github.com/storytold/gridcraft / patch
fix(ui): support single-key answers in Save Changes dialog (#2572)
Support Y/N shortcuts in the Save Changes dialog, alongside Esc ### Environment GridCraft 0.12.1 on Windows. ### Request Please support single-key answers in the Save Changes confirmation shown when closing a workbook or exiting the app: - **Y**: Save, then continue closing only if saving succeeds. - **N**: Don't Save, then continue closing. - **Esc**: Cancel closing and keep the workbook open (retain the existing behavior). ### Current experience After editing a workbook and closing it with Ctrl+W or Alt+F4, Y and N do not select the corresponding choices in the save confirmation. This interrupts a familiar keyboard-only workflow. Ctrl+S already works;
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- kk376 (imported from pull request #2662, open)
- Base
45f5ab18e4e57bf50ee88d2526deb4b87c5b24cb- Size
- +193 −5 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
6e8da228fa838bd4f96e375d
How it went
No other operator has reported using it yet.
Diff
raw · crates/ui-egui/src/dialogs/show.rs crates/ui-egui/tests/it/file_menu.rs docs/ui-parity.md
@@ -293,15 +293,36 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { "saveChanges" => { ui.label(format!("Do you want to save the changes you made to {}?", d.values.get("title").and_then(Json::as_str).unwrap_or("this workbook"))); let quitting = d.values.get("quit").and_then(Json::as_bool) == Some(true);+ let mut chosen = ui.input_mut(|i| {+ let fresh_enter = consume_fresh_key(i, egui::Key::Enter);+ // Discard remaining Enter events so egui does not fake-click focused buttons on Enter+ i.events.retain(|event| !matches!(event, egui::Event::Key { key: egui::Key::Enter, pressed: true, .. }));++ if fresh_enter+ || (i.modifiers.is_none()+ && (consume_fresh_key(i, egui::Key::Y) || consume_fresh_key(i, egui::Key::S)))+ {+ Some(crate::SaveAnswer::Save)+ } else if i.modifiers.is_none()+ && (consume_fresh_key(i, egui::Key::N) || consume_fresh_key(i, egui::Key::D))+ {+ Some(crate::SaveAnswer::DontSave)+ } else {+ None+ }+ }); ui.horizontal(|ui| { for (label, answer) in [("Don't Save", crate::SaveAnswer::DontSave), ("Cancel", crate::SaveAnswer::Cancel), ("Save", crate::SaveAnswer::Save)] {- if ui.button(label).clicked() && app.answer_save_changes(answer, quitting, ui.ctx()) {- open = false;+ if ui.button(label).clicked() && chosen.is_none() {+ chosen = Some(answer); } } });+ if let Some(answer) = chosen && app.answer_save_changes(answer, quitting, ui.ctx()) {+ open = false;+ } } "start" => { ui.heading("Start a new workbook");@@ -430,6 +451,19 @@ pub(super) fn ok_cancel(ui: &mut egui::Ui, confirm: &mut bool, open: &mut bool) }); } +/// Consumes one fresh, unmodified press of `key`. Held-key repeats stay queued.+fn consume_fresh_key(input: &mut egui::InputState, key: egui::Key) -> bool {+ let before = input.events.len();+ input.events.retain(|event| {+ !matches!(+ event,+ egui::Event::Key { key: ev_key, modifiers, pressed: true, repeat: false, .. }+ if *ev_key == key && modifiers.is_none()+ )+ });+ input.events.len() != before+}+ fn form(ui: &mut egui::Ui, d: &mut Dialog, confirm: &mut bool, open: &mut bool) { form_fields(ui, d); ok_cancel(ui, confirm, open);@@ -300,6 +300,159 @@ fn quitting_with_one_of_two_workbooks_unsaved_asks_once() { assert!(h.state().dialog.is_none()); } +fn press_dialog_key(h: &mut egui_kittest::Harness<'static, SheetApp>, key: egui::Key, modifiers: egui::Modifiers) -> Vec<egui::ViewportCommand> {+ h.input_mut().events.push(egui::Event::ModifiersChanged(modifiers));+ h.input_mut().events.push(egui::Event::Key { key, physical_key: Some(key), pressed: true, repeat: false, modifiers });+ h.input_mut().events.push(egui::Event::Key { key, physical_key: Some(key), pressed: false, repeat: false, modifiers });+ h.input_mut().events.push(egui::Event::ModifiersChanged(egui::Modifiers::NONE));+ step_commands(h)+}++#[test]+fn save_changes_dialog_answers_with_single_key_shortcuts() {+ // 1. Single-key 'N' discards changes and closes the window (#2572)+ let mut h = harness(Services::default());+ h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "42"})).unwrap();+ h.run_steps(2);+ request_window_close(&mut h);+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"));++ // Modified shortcuts (Ctrl+N, Alt+Y, Cmd+S) must be ignored and leave dialog open+ let sent = press_dialog_key(&mut h, egui::Key::N, egui::Modifiers::CTRL);+ assert!(!sent.contains(&egui::ViewportCommand::Close));+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"), "Ctrl+N must not answer the dialog");++ let sent = press_dialog_key(&mut h, egui::Key::Y, egui::Modifiers::ALT);+ assert!(!sent.contains(&egui::ViewportCommand::Close));+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"), "Alt+Y must not answer the dialog");++ // Unmodified 'N' answers "Don't Save"+ let sent = press_dialog_key(&mut h, egui::Key::N, egui::Modifiers::NONE);+ assert!(sent.contains(&egui::ViewportCommand::Close), "single-key 'N' closes the window");+ assert!(h.state().dialog.is_none());++ // 2. Single-key 'Y' writes changes to an existing file and closes+ let dir = std::env::temp_dir().join(format!("gridcraft-save-changes-key-{}", std::process::id()));+ std::fs::create_dir_all(&dir).unwrap();+ let path = dir.join("Book.xlsx").to_string_lossy().into_owned();+ let mut h = harness(Services::default());+ h.state_mut().run("file.save", serde_json::json!({"path": &path})).unwrap();+ h.run_steps(2);+ h.state_mut().run("cell.set", serde_json::json!({"cell": "B2", "input": "saved_with_y"})).unwrap();+ h.run_steps(2);++ request_window_close(&mut h);+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"));++ let sent = press_dialog_key(&mut h, egui::Key::Y, egui::Modifiers::NONE);+ assert!(sent.contains(&egui::ViewportCommand::Close), "single-key 'Y' saves and closes");+ assert!(h.state().dialog.is_none());++ let (wb, _) = gridcraft_engine::io::open_bytes("Book.xlsx", &std::fs::read(&path).unwrap()).unwrap();+ assert_eq!(wb.active().unwrap().value(CellRef::new(1, 1)), Value::text("saved_with_y"));+ let _ = std::fs::remove_dir_all(&dir);++ // 3. Quitting multiple workbooks: single-key 'N' answers one workbook per key press+ let mut h = harness(Services::default());+ h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "first"})).unwrap();+ h.state_mut().run("file.new", serde_json::json!({})).unwrap();+ h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "second"})).unwrap();+ h.run_steps(2);++ request_window_close(&mut h);+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"));+ assert_eq!(h.state().session.documents().len(), 2);++ // User presses 'N' to discard the first workbook without releasing the key yet+ h.input_mut().events.push(egui::Event::Key {+ key: egui::Key::N,+ physical_key: Some(egui::Key::N),+ pressed: true,+ repeat: false,+ modifiers: egui::Modifiers::NONE,+ });+ let sent = step_commands(&mut h);+ assert!(!sent.contains(&egui::ViewportCommand::Close));+ assert_eq!(h.state().session.documents().len(), 1);+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"));++ // While 'N' remains held down, OS auto-repeat events must NOT dismiss the second dialog+ h.input_mut().events.push(egui::Event::Key {+ key: egui::Key::N,+ physical_key: Some(egui::Key::N),+ pressed: true,+ repeat: true,+ modifiers: egui::Modifiers::NONE,+ });+ let sent = step_commands(&mut h);+ assert!(!sent.contains(&egui::ViewportCommand::Close));+ assert_eq!(h.state().session.documents().len(), 1, "held-key repeat must not close the workbook");+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"), "dialog must stay open");++ // The user releases 'N'+ h.input_mut().events.push(egui::Event::Key {+ key: egui::Key::N,+ physical_key: Some(egui::Key::N),+ pressed: false,+ repeat: false,+ modifiers: egui::Modifiers::NONE,+ });+ step_commands(&mut h);++ // Fresh 'D' closes the final workbook and closes window+ let sent = press_dialog_key(&mut h, egui::Key::D, egui::Modifiers::NONE);+ assert!(sent.contains(&egui::ViewportCommand::Close));+ assert!(h.state().dialog.is_none());++ // 4. Focused "Don't Save" button does not hijack Enter key; Enter still answers Save+ let dir = std::env::temp_dir().join(format!("gridcraft-enter-focus-test-{}", std::process::id()));+ std::fs::create_dir_all(&dir).unwrap();+ let path = dir.join("FocusTest.xlsx").to_string_lossy().into_owned();+ let mut h = harness(Services::default());+ h.state_mut().run("file.save", serde_json::json!({"path": &path})).unwrap();+ h.run_steps(2);+ h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "keep_this"})).unwrap();+ h.run_steps(2);++ request_window_close(&mut h);+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"));++ h.get_by_label("Don't Save").focus();+ h.run_steps(1);++ let sent = press_dialog_key(&mut h, egui::Key::Enter, egui::Modifiers::NONE);+ assert!(sent.contains(&egui::ViewportCommand::Close));+ assert!(h.state().dialog.is_none());++ let (wb, _) = gridcraft_engine::io::open_bytes("FocusTest.xlsx", &std::fs::read(&path).unwrap()).unwrap();+ assert_eq!(wb.active().unwrap().value(CellRef::new(0, 0)), Value::text("keep_this"));+ let _ = std::fs::remove_dir_all(&dir);++ // 5. Space key on focused "Don't Save" button activates it+ let dir = std::env::temp_dir().join(format!("gridcraft-space-focus-test-{}", std::process::id()));+ std::fs::create_dir_all(&dir).unwrap();+ let path = dir.join("SpaceTest.xlsx").to_string_lossy().into_owned();+ let mut h = harness(Services::default());+ h.state_mut().run("file.save", serde_json::json!({"path": &path})).unwrap();+ h.run_steps(2);+ h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "discard_this"})).unwrap();+ h.run_steps(2);++ request_window_close(&mut h);+ assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"));++ h.get_by_label("Don't Save").focus();+ h.run_steps(1);++ let sent = press_dialog_key(&mut h, egui::Key::Space, egui::Modifiers::NONE);+ assert!(sent.contains(&egui::ViewportCommand::Close));+ assert!(h.state().dialog.is_none());++ let (wb, _) = gridcraft_engine::io::open_bytes("SpaceTest.xlsx", &std::fs::read(&path).unwrap()).unwrap();+ assert_eq!(wb.active().unwrap().value(CellRef::new(0, 0)), Value::Empty);+ let _ = std::fs::remove_dir_all(&dir);+}+ /// Book1 has a cell being typed when File › New runs (the macOS menu runs the command directly). fn typing_in_book1_then_new(h: &mut egui_kittest::Harness<'static, SheetApp>) -> u64 { h.input_mut().events.push(egui::Event::Text("hello".into()));@@ -1,6 +1,6 @@ # UI parity -> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-25 · **Change:** minor (AutoSum over a selected row puts its total to the right) · **Target:** Microsoft Excel (Microsoft 365), Excel for Mac 16.113.4+> **Last reviewed:** 2026-10-10 · **Last updated:** 2026-10-27 · **Change:** minor (Save Changes dialog supports Enter, Y, S for Save, and N, D for Don't Save, #2572) · **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@@ -34,8 +34,8 @@ Sources: `crates/ui-egui/src` (ribbon, grid, editor, formula bar, dialogs, panes | Backstage (File) view: Info, New with templates, Open recent, Export, Options | yes | yes (#174, 10-11): the File tab opens a full-window view (Esc or the back arrow return) with Home, New (blank + the three samples), Open (recent + Browse; the recent list tells when each workbook was last changed, 10-12), Info (location, properties, Protect Workbook), Save, Save As, Print (page preview, active sheet or workbook, fit to page, Page Setup), Export (PDF, CSV, web page), Close and Options (`file.options`: theme, language, formula bar, AutoSave, calculation mode and iterative calculation with Maximum Iterations and Maximum Change, 10-15) | no Account, Share or template gallery beyond the samples; Options applies at once (no OK/Cancel dialog) and holds only the settings GridCraft has | | Dark mode, system appearance | yes | yes | done (10-09) | | Page Layout view (paper view with rulers and header editing) | yes | yes (#329, 10-11): per-sheet view saved in XLSX, pages with margins and gaps on the print pagination, "Click to add data" pages, rulers (inches or centimetres), headers and footers drawn and edited in place with a Header & Footer tab, whose Different First Page and Different Odd & Even Pages give the first page and the even pages their own header and footer (#344), and whose Scale with Document and Align with Page Margins set whether header and footer text follows the print scale and the side margins, on paper and in the view | margins drag on the rulers (#792, with header and footer margins); header/footer pictures missing (1–3 h). Unverified: with Align with Page Margins off, header and footer text sits 0.7 in from the paper edge, a value not yet measured in the target app |-| Multiple windows per workbook | yes | yes (10-22, desktop): each workbook window is a native window of its own (New, Open and New Window open one; titles "Book1", or "Book1:1" / "Book1:2" for a workbook's second window, with " — Edited" while unsaved). Each window keeps its own cell draft, dialog, Backstage view, message and grid state, and its ribbon acts on it; on macOS the menu bar acts on the window focused last. Leaving a window enters its draft as Enter would. A window's close button closes that window: one of several on a workbook closes without asking, a workbook's last asks Save / Don't Save / Cancel when unsaved. Quit asks about each unsaved workbook in its window, and Cancel stops it. The last window closed quits on Windows and Linux; macOS keeps running with the menu bar until New or Open. View › Arrange All (dialog: Tiled, Horizontal, Vertical, Cascade, Windows of active workbook; `view.arrangeAll` tiles by default) lays the visible windows out over the main window's monitor, the active one first. Web: one window | Arrange All uses one monitor; a hidden window (View › Hide) has no native window; every window repaints when one does; see [hardware-parity.md](hardware-parity.md) |-| Switch Windows, New Window, unsaved workbooks on quit | yes | yes (10-22): View › New Window opens another window on the same workbook (no copy: saving from either saves it), each with its own selection, active sheet and scroll (showing different sheets copies nothing and leaves the workbook unchanged; Save records the sheet of the window used last); a workbook with several windows titles them "Book1:1", "Book1:2"; File › Close closes one of them without asking and asks only for the last; View › Switch Windows and the macOS Window menu list the open windows (active one checked, unsaved ones marked •) and bring the chosen one forward (`view.switchWindows {window}`, or `{doc}` for a workbook's most recent window); a cell being typed is entered into its own window and workbook before New, Open, Close, Save, Print and window switches (also between two windows on one workbook), and a refused entry stays open and stops the command; quitting (Cmd+Q, the macOS Quit item, the Dock) asks Save / Don't Save / Cancel for each unsaved workbook | |+| Multiple windows per workbook | yes | yes (10-22, desktop): each workbook window is a native window of its own (New, Open and New Window open one; titles "Book1", or "Book1:1" / "Book1:2" for a workbook's second window, with " — Edited" while unsaved). Each window keeps its own cell draft, dialog, Backstage view, message and grid state, and its ribbon acts on it; on macOS the menu bar acts on the window focused last. Leaving a window enters its draft as Enter would. A window's close button closes that window: one of several on a workbook closes without asking, a workbook's last asks Save / Don't Save / Cancel when unsaved (answering via Enter, Y or S saves, N or D discards, Esc cancels). Quit asks about each unsaved workbook in its window, and Cancel stops it. The last window closed quits on Windows and Linux; macOS keeps running with the menu bar until New or Open. View › Arrange All (dialog: Tiled, Horizontal, Vertical, Cascade, Windows of active workbook; `view.arrangeAll` tiles by default) lays the visible windows out over the main window's monitor, the active one first. Web: one window | Arrange All uses one monitor; a hidden window (View › Hide) has no native window; every window repaints when one does; see [hardware-parity.md](hardware-parity.md) |+| Switch Windows, New Window, unsaved workbooks on quit | yes | yes (10-22): View › New Window opens another window on the same workbook (no copy: saving from either saves it), each with its own selection, active sheet and scroll (showing different sheets copies nothing and leaves the workbook unchanged; Save records the sheet of the window used last); a workbook with several windows titles them "Book1:1", "Book1:2"; File › Close closes one of them without asking and asks only for the last; View › Switch Windows and the macOS Window menu list the open windows (active one checked, unsaved ones marked •) and bring the chosen one forward (`view.switchWindows {window}`, or `{doc}` for a workbook's most recent window); a cell being typed is entered into its own window and workbook before New, Open, Close, Save, Print and window switches (also between two windows on one workbook), and a refused entry stays open and stops the command; quitting (Cmd+Q, the macOS Quit item, the Dock) asks Save / Don't Save / Cancel for each unsaved workbook (Enter, Y, S saves; N, D discards; Esc cancels; held-key repeats ignored across successive prompts) | | ## Grid interaction @@ -187,6 +187,7 @@ scrollbar bugs 2–4 · pixel/feel pass against observed Excel 7–12. | Date | Change | Summary | |---|---|---|+| 2026-10-27 | minor | Save Changes confirmation dialog supports single-key keyboard answers (Enter, Y, S for Save; N, D for Don't Save; Esc for Cancel, #2572). Fresh unmodified key presses are consumed without auto-repeating across successive unsaved workbook prompts on quit; Enter prioritizes Save over focused dialog buttons | | 2026-10-25 | minor | AutoSum (Alt+=, Home › AutoSum) over a selected row puts its total to the right of the row, or in its last cell when that is empty, as in Excel; it wrote a one-cell total under each cell. A selection's empty last column takes each row's total and its empty last row each column's, with the block's total in the corner when both are empty | | 2026-10-25 | minor | Paste Name lays out only the names in view, so a workbook with thousands of names opens it at once; the arrow keys still scroll the chosen name into view. Go To matches a `Sheet2!Name` reference and hides a workbook name behind the active sheet's own name in any case and script (`Größe` = `GRÖSSE`), as formulas do. The two Page Layout view rows are one; the 0.7 in side margin used when Align with Page Margins is off is marked unverified | | 2026-10-24 | minor | Go To lists the named ranges on every sheet, as Excel's does ("The Go to popup window shows named ranges on every worksheet in your workbook"): a name scoped to another sheet appears as `Sheet2!Name`, and hidden names, constants and formulas are left out. It keeps the references entered (newest first, ten at most), and a double-click on an entry goes there. The Reference box and `edit.goTo` take several names or references separated by commas (`Price, Type`, `B14:C22,F19:G30,H21:H29`) and select them all; `edit.goTo {list: true}` returns the list |