Closing the window asks to save unsaved workbooks (fixes #838, fixes #841)

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)
         }

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;
                         }

crates/ui-egui/src/lib.rs

@@ -247,6 +247,9 @@ pub struct SheetApp {
     /// Chart selected on the sheet (id).
     pub selected_chart: Option<u32>,
     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<u8>)> = self
             .services

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<egui::ViewportCommand> {
+    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<egui::ViewportCommand> {
+    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<egui::ViewportCommand> {
+    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);
+}