From f25bded4ef41bf6fd2123803bbac7f2dcbeb8892 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Tue, 13 Oct 2026 00:12:20 +0200 Subject: [PATCH] Closing the window asks to save unsaved workbooks (fixes #838, fixes #841) --- crates/ui-egui/src/control.rs | 2 + crates/ui-egui/src/dialogs.rs | 10 ++++- crates/ui-egui/src/lib.rs | 44 ++++++++++++++++++ crates/ui-egui/tests/file_menu.rs | 74 +++++++++++++++++++++++++++++++ 4 files changed, 129 insertions(+), 1 deletion(-) diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index b753b00e8..aa0069f53 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -376,6 +376,8 @@ pub fn handle(app: &mut SheetApp, ctx: &egui::Context, method: &str, p: &Json) - } "app.save" => wrap(app.run("file.save", json!({"path": s("path")}))), "app.quit" => { + // Programmatic calls never open dialogs: quitting from here doesn't ask about unsaved work. + app.allow_close = true; ctx.send_viewport_cmd(egui::ViewportCommand::Close); ok(Json::Null) } diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 5e859c68e..d226f4582 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -783,7 +783,7 @@ impl Dialog { } "pickList" => Dialog::custom("pickList", "Pick From List", json!({})), "saveCopy" => Dialog::custom("saveCopy", "Save a Copy", json!({"format": p("format")})), - "saveChanges" => Dialog::custom("saveChanges", "Save Changes?", json!({"title": p("title")})), + "saveChanges" => Dialog::custom("saveChanges", "Save Changes?", json!({"title": p("title"), "quit": p("quit")})), "start" => Dialog::custom("start", "GridCraft", json!({})), "comments" => { app.grid.pane = None; @@ -1038,9 +1038,14 @@ 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"))); + // Asked while quitting: once answered, the next unsaved workbook is asked about. + let quitting = d.values.get("quit").and_then(Json::as_bool) == Some(true); ui.horizontal(|ui| { if ui.button("Don't Save").clicked() { app.run_or_alert("file.close", json!({"force": true})); + if quitting { + app.continue_quit(ui.ctx()); + } open = false; } if ui.button("Cancel").clicked() { @@ -1051,6 +1056,9 @@ pub fn show(app: &mut SheetApp, ctx: &egui::Context) { app.run_or_alert("file.save", json!({})); if app.session.active().is_some_and(|x| !x.is_dirty()) { app.run_or_alert("file.close", json!({"force": true})); + if quitting { + app.continue_quit(ui.ctx()); + } } open = false; } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0b19b2551..d6371b1bc 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -247,6 +247,9 @@ pub struct SheetApp { /// Chart selected on the sheet (id). pub selected_chart: Option, pub started: f64, + /// Every unsaved workbook has been answered for quitting (or `app.quit` asked): the next + /// window close goes through. + pub allow_close: bool, } impl SheetApp { @@ -288,6 +291,7 @@ impl SheetApp { shots: control::Shots::default(), selected_chart: None, started: now_ms(), + allow_close: false, } } @@ -877,6 +881,45 @@ impl SheetApp { } } + /// Holds back a window close (the close button, Alt+F4, the OS quitting the app) while a + /// workbook has unsaved changes, and asks about them first. + fn guard_window_close(&mut self, ctx: &egui::Context) { + if self.allow_close || !ctx.input(|i| i.viewport().close_requested()) { + return; + } + // A cell still being edited is part of the workbook, as for File › Close. + if !self.commit_edit(0, 0, false, false) { + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + return; + } + if !self.session.documents().iter().any(|d| d.is_dirty()) { + return; + } + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + match self.dialog.as_mut() { + // Already asking about a workbook (File › Close): answering it now goes on to quit. + Some(d) if d.name() == "saveChanges" => { + d.values.insert("quit".into(), json!(true)); + } + _ => self.continue_quit(ctx), + } + } + + /// Asks about the next unsaved workbook before quitting, or closes the window once none is left. + pub(crate) fn continue_quit(&mut self, ctx: &egui::Context) { + match self.session.documents().iter().position(|d| d.is_dirty()) { + Some(i) => { + self.session.set_active(i); + let title = self.session.active().map(|d| d.display_title()).unwrap_or_default(); + self.open_dialog("saveChanges", json!({"title": title, "quit": true})); + } + None => { + self.allow_close = true; + ctx.send_viewport_cmd(egui::ViewportCommand::Close); + } + } + } + /// Per-frame logic (control channel, screenshots). Call before `ui`. pub fn logic(&mut self, ctx: &egui::Context) { // A keytip control request lives one frame: the ribbon consumes it during `ui`, and @@ -945,6 +988,7 @@ impl SheetApp { ctx.request_repaint(); } control::poll(self, ctx); + self.guard_window_close(ctx); // Files read asynchronously (web file picker, dropped files). let arrived: Vec<(String, Vec)> = self .services diff --git a/crates/ui-egui/tests/file_menu.rs b/crates/ui-egui/tests/file_menu.rs index c6ef2b212..195c43c37 100644 --- a/crates/ui-egui/tests/file_menu.rs +++ b/crates/ui-egui/tests/file_menu.rs @@ -171,3 +171,77 @@ fn save_as_offers_pdf_and_leaves_the_workbook_file_alone() { assert!(d.path.is_none() && d.is_dirty(), "a PDF is a copy, not the workbook's file"); let _ = std::fs::remove_dir_all(&dir); } + +/// The window's close button (or Alt+F4, or the OS quitting the app): steps until the request +/// has been handled and returns the viewport commands the app answered with. +fn request_window_close(h: &mut egui_kittest::Harness<'static, SheetApp>) -> Vec { + h.input_mut().viewports.entry(egui::ViewportId::ROOT).or_default().events.push(egui::ViewportEvent::Close); + step_commands(h) +} + +/// Runs a few frames and gathers the commands sent to the window. +fn step_commands(h: &mut egui_kittest::Harness<'static, SheetApp>) -> Vec { + let mut out = Vec::new(); + for _ in 0..3 { + h.step(); + out.extend(h.output().viewport_output.get(&egui::ViewportId::ROOT).map(|v| v.commands.clone()).unwrap_or_default()); + } + out +} + +fn click_commands(h: &mut egui_kittest::Harness<'static, SheetApp>, label: &str) -> Vec { + h.get_by_role_and_label(egui::accesskit::Role::Button, label).click(); + step_commands(h) +} + +#[test] +fn closing_the_window_asks_about_every_unsaved_workbook() { + let mut h = harness(Services::default()); + h.state_mut().run("cell.set", serde_json::json!({"cell": "A1", "input": "42"})).unwrap(); + h.state_mut().run("file.new", serde_json::json!({})).unwrap(); + h.state_mut().run("cell.set", serde_json::json!({"cell": "B2", "input": "7"})).unwrap(); + h.run_steps(2); + + let sent = request_window_close(&mut h); + assert!(sent.contains(&egui::ViewportCommand::CancelClose), "unsaved changes hold the window open: {sent:?}"); + assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges")); + let sent = click_commands(&mut h, "Cancel"); + assert!(!sent.contains(&egui::ViewportCommand::Close), "Cancel keeps the app open"); + assert!(h.state().dialog.is_none()); + assert_eq!(h.state().session.documents().len(), 2, "Cancel keeps both workbooks"); + + request_window_close(&mut h); + let sent = click_commands(&mut h, "Don't Save"); + assert!(!sent.contains(&egui::ViewportCommand::Close), "the other workbook is still unsaved"); + assert_eq!(h.state().session.documents().len(), 1); + assert_eq!(h.state().dialog.as_ref().map(|d| d.name()), Some("saveChanges"), "then it asks about the next one"); + let sent = click_commands(&mut h, "Don't Save"); + assert!(sent.contains(&egui::ViewportCommand::Close), "once every workbook is answered the window closes: {sent:?}"); + let sent = request_window_close(&mut h); + assert!(!sent.contains(&egui::ViewportCommand::CancelClose), "the close that follows is let through"); +} + +#[test] +fn closing_the_window_can_save_first_and_a_saved_workbook_closes_at_once() { + let dir = std::env::temp_dir().join(format!("gridcraft-close-window-{}", 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); + let sent = request_window_close(&mut h); + assert!(!sent.contains(&egui::ViewportCommand::CancelClose), "nothing unsaved: the window just closes"); + assert!(h.state().dialog.is_none()); + + // A cell still being typed is an unsaved change too. + h.input_mut().events.push(egui::Event::Text("123".into())); + h.run_steps(2); + assert!(h.state().editor.is_some()); + let sent = request_window_close(&mut h); + assert!(sent.contains(&egui::ViewportCommand::CancelClose), "{sent:?}"); + let sent = click_commands(&mut h, "Save"); + assert!(sent.contains(&egui::ViewportCommand::Close), "{sent:?}"); + let (wb, _) = gridcraft_engine::io::open_bytes("Book.xlsx", &std::fs::read(&path).unwrap()).unwrap(); + assert_eq!(wb.active().unwrap().value(CellRef::new(0, 0)), Value::Number(123.0), "Save wrote the change before closing"); + let _ = std::fs::remove_dir_all(&dir); +}