Merge #146: File › Close, ⌘W and the tab × ask Save / Don't Save / Cancel for unsaved documents

Conflict: crates/ui-egui/src/lib.rs: kept both sides' tests in mod tests (#142/#138 and #146)

crates/ui-egui/src/chrome.rs

@@ -514,7 +514,7 @@ pub fn doc_tabs(app: &mut DesignApp, ui: &mut egui::Ui) {
         x += w;
     }
     if let Some(i) = close {
-        let _ = app.run("file.close", json!({"index": i}));
+        crate::menus::close_document(app, Some(i));
     } else if let Some(i) = activate {
         let _ = app.run("file.activate", json!({"index": i}));
     }

crates/ui-egui/src/dialogs.rs

@@ -819,6 +819,7 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) {
         "polygonSettings" => crate::i18n::tr(&app.ui.language, "Polygon Settings"),
         "userDictionary" => crate::i18n::tr(&app.ui.language, "User Dictionary"),
         "newWorkspace" => crate::i18n::tr(&app.ui.language, "New Workspace"),
+        "closeDocument" => crate::i18n::tr(&app.ui.language, "Unsaved Changes"),
         "menus" => crate::i18n::tr(&app.ui.language, "Menu Customization"),
         "importOptions" => crate::i18n::tr(&app.ui.language, "Import Options"),
         "fittingOptions" => crate::i18n::tr(&app.ui.language, "Frame Fitting Options"),
@@ -837,7 +838,7 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) {
     };
     egui::Modal::new(egui::Id::new("dialog")).show(ctx, |ui| {
         ui.set_min_width(380.0);
-        ui.set_max_width(if d.id == "newDocument" && crate::i18n::is_rtl(&app.ui.language) { 380.0 } else { 640.0 });
+        ui.set_max_width(if d.id == "closeDocument" || (d.id == "newDocument" && crate::i18n::is_rtl(&app.ui.language)) { 380.0 } else { 640.0 });
         if crate::i18n::is_rtl(&app.ui.language) {
             ui.horizontal(|ui| {
                 ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
@@ -1198,6 +1199,10 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) {
                     });
                 }
             }
+            "closeDocument" => {
+                let text = crate::i18n::tr(&app.ui.language, "“{}” has changes that are not saved. Save them before closing?").replace("{}", &d.s("title"));
+                crate::rtl::label(ui, text);
+            }
             "newWorkspace" => {
                 ui.horizontal(|ui| {
                     crate::rtl::label(ui, crate::i18n::tr(&app.ui.language, "Name:"));
@@ -1369,11 +1374,12 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) {
             _ => {}
         }
         ui.add_space(12.0);
+        let ok_label = if d.id == "closeDocument" { "  Save  " } else { "  OK  " };
         ui.horizontal(|ui| {
             ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
                 if ui
                     .add(
-                        egui::Button::new(crate::rtl::widget(ui, egui::RichText::new(crate::i18n::tr(&app.ui.language, "  OK  ")).color(egui::Color32::WHITE)))
+                        egui::Button::new(crate::rtl::widget(ui, egui::RichText::new(crate::i18n::tr(&app.ui.language, ok_label)).color(egui::Color32::WHITE)))
                             .fill(crate::theme::Tokens::get(ui.ctx()).accent_strong),
                     )
                     .clicked()
@@ -1384,6 +1390,10 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) {
                 if ui.button(crate::rtl::widget(ui, crate::i18n::tr(&app.ui.language, "Cancel"))).clicked() || ui.input(|i| i.key_pressed(egui::Key::Escape)) {
                     result = Some(false);
                 }
+                if d.id == "closeDocument" && ui.button(crate::rtl::widget(ui, crate::i18n::tr(&app.ui.language, "Don't Save"))).clicked() {
+                    d.fields.insert("discard".into(), json!(true));
+                    result = Some(true);
+                }
             });
         });
     });
@@ -1601,6 +1611,24 @@ pub fn confirm(app: &mut DesignApp) -> Result<Value, String> {
             app.run("document.preferences", doc)
         }
         "newWorkspace" => app.run("window.newWorkspace", json!({"name": d.s("name")})),
+        "closeDocument" => {
+            // The document may have moved in the tab row (or gone) while the dialog was open.
+            let uid = d.fields.get("uid").and_then(Value::as_u64);
+            let index = |app: &DesignApp| app.session.documents().iter().position(|doc| Some(doc.uid) == uid);
+            let Some(i) = index(app) else { return Ok(Value::Null) };
+            if !d.b("discard") {
+                app.run("file.activate", json!({"index": i}))?;
+                app.run("app.save", json!({}))?;
+                // Still unsaved: the save was cancelled (no file chosen). Keep the document open.
+                if app.session.documents().get(i).is_none_or(|doc| doc.is_dirty()) {
+                    return Ok(Value::Null);
+                }
+            }
+            match index(app) {
+                Some(i) => app.run("file.close", json!({"index": i})),
+                None => Ok(Value::Null),
+            }
+        }
         "importOptions" => app.run(
             "file.place",
             json!({"path": d.s("path"), "removeStyles": d.b("removeStyles"), "styleConflicts": d.s("styleConflicts"), "styleMap": d.fields.get("map").cloned().unwrap_or(json!({}))}),

crates/ui-egui/src/lib.rs

@@ -928,6 +928,78 @@ mod tests {
         assert_eq!(raw.events, events());
     }
 
+    /// An app with one document that has an unsaved edit.
+    fn app_with_unsaved_document() -> DesignApp {
+        let mut app = DesignApp::new(designcraft_engine::Session::new(), Services::default());
+        app.run("file.new", json!({})).unwrap();
+        assert!(!app.session.documents()[0].is_dirty(), "a new document has nothing to save");
+        app.run("frame.create", json!({"rect": [36, 36, 136, 136]})).unwrap();
+        assert!(app.session.documents()[0].is_dirty());
+        app
+    }
+
+    /// File ▸ Close (and its shortcut) closed a document with unsaved changes without asking, and
+    /// closing discards the document's recovery data too.
+    #[test]
+    fn closing_an_unsaved_document_asks_first() {
+        let mut app = app_with_unsaved_document();
+        menus::activate(&mut app, "file.close", &Value::Null);
+        assert_eq!(app.session.documents().len(), 1, "the document stays open until the user answers");
+        assert_eq!(app.ui.dialog.as_ref().map(|d| d.id.as_str()), Some("closeDocument"));
+        // Cancel: nothing happens.
+        app.ui.dialog = None;
+        assert!(app.session.documents()[0].is_dirty());
+        // The tab's × asks too.
+        menus::close_document(&mut app, Some(0));
+        assert_eq!(app.ui.dialog.as_ref().map(|d| d.id.as_str()), Some("closeDocument"));
+        assert_eq!(app.session.documents().len(), 1);
+    }
+
+    #[test]
+    fn closing_without_saving_discards_the_document() {
+        let mut app = app_with_unsaved_document();
+        menus::close_document(&mut app, None);
+        app.ui.dialog.as_mut().unwrap().fields.insert("discard".into(), json!(true));
+        dialogs::confirm(&mut app).unwrap();
+        assert!(app.session.documents().is_empty());
+        assert!(app.ui.dialog.is_none());
+    }
+
+    #[test]
+    fn closing_with_save_writes_the_file_then_closes() {
+        let mut app = app_with_unsaved_document();
+        let path = std::env::temp_dir().join(format!("designcraft-close-test-{}.designcraft", std::process::id()));
+        app.run("file.saveAs", json!({"path": path})).unwrap();
+        let saved = std::fs::metadata(&path).unwrap().len();
+        app.run("frame.create", json!({"rect": [200, 200, 300, 300]})).unwrap();
+        menus::close_document(&mut app, None);
+        dialogs::confirm(&mut app).unwrap();
+        assert!(app.session.documents().is_empty());
+        let written = std::fs::metadata(&path).unwrap().len();
+        std::fs::remove_file(&path).unwrap();
+        assert!(written > saved, "the second frame was saved ({saved} → {written} bytes)");
+    }
+
+    /// Save on a document that has no file opens the file picker; cancelling it must not close the
+    /// document. (No picker service here: the same as cancelling.)
+    #[test]
+    fn a_cancelled_save_keeps_the_document_open() {
+        let mut app = app_with_unsaved_document();
+        menus::close_document(&mut app, None);
+        dialogs::confirm(&mut app).unwrap();
+        assert_eq!(app.session.documents().len(), 1);
+        assert!(app.session.documents()[0].is_dirty());
+    }
+
+    #[test]
+    fn closing_a_saved_document_does_not_ask() {
+        let mut app = DesignApp::new(designcraft_engine::Session::new(), Services::default());
+        app.run("file.new", json!({})).unwrap();
+        menus::activate(&mut app, "file.close", &Value::Null);
+        assert!(app.session.documents().is_empty());
+        assert!(app.ui.dialog.is_none());
+    }
+
     #[test]
     fn snap_view_uses_the_saved_switches() {
         let mut ui = UiState::default();

crates/ui-egui/src/menus.rs

@@ -1718,7 +1718,26 @@ pub fn menu_enabled(app: &DesignApp, id: &str) -> bool {
 
 /// A menu item was chosen. An engine command whose label ends in "…" and that takes parameters
 /// opens a dialog built from its parameter documentation (see [`crate::dialogs::command_fields`]).
+/// Close document `index` (the active one when `None`) the way the user asks for it: with the
+/// tab's ×, File ▸ Close or its shortcut. A document with unsaved changes asks first, because
+/// closing also discards its recovery data. Scripts and agents use `file.close`, which never asks.
+pub fn close_document(app: &mut DesignApp, index: Option<usize>) {
+    let Some(i) = index.or(app.session.active_index()) else { return };
+    match app.session.documents().get(i) {
+        Some(d) if d.is_dirty() => {
+            app.ui.dialog = Some(crate::dialogs::Dialog::new("closeDocument", json!({"uid": d.uid, "title": d.title(), "discard": false})));
+        }
+        _ => {
+            let _ = app.run("file.close", json!({"index": i}));
+        }
+    }
+}
+
 pub fn activate(app: &mut DesignApp, id: &str, params: &Value) {
+    if params.is_null() && id == "file.close" {
+        close_document(app, None);
+        return;
+    }
     if params.is_null() && id == "layout.documentSetup" {
         crate::dialogs::open_document_setup(app);
         return;