UI: closing a document with unsaved changes asks first (#141) (#146)

File ▸ Close, its shortcut (Ctrl/Cmd+W) and the tab's × closed a document
with unsaved changes at once. Closing also discards the document's
recovery data, so the edits were gone for good.

The three user paths now go through menus::close_document. A document
with unsaved changes opens a dialog with Save, Don't Save and Cancel:

- Save runs the same Save the menu does, then closes. If the document
  is still unsaved afterwards (the file picker was cancelled), it stays
  open.
- Don't Save closes it.
- Cancel (or Escape) does nothing.

A document with nothing to save closes as before. The engine command
file.close is unchanged and never asks, so scripts, MCP and the control
channel's engine.execute keep working unattended. ui.menu.invoke without
params acts like the menu item, as it does for other dialogs.

Quitting the app is not covered here. Recovery data survives a quit, so
unsaved documents come back on the next start.

Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Co-authored-by: Brandon Thomas <[email protected]>

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

@@ -1007,6 +1007,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"),
@@ -1027,6 +1028,7 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) {
         ui.set_min_width(380.0);
         ui.set_max_width(match d.id.as_str() {
             "alert" => 440.0,
+            "closeDocument" => 380.0,
             "newDocument" if crate::i18n::is_rtl(&app.ui.language) => 380.0,
             _ => 640.0,
         });
@@ -1395,6 +1397,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:"));
@@ -1555,11 +1561,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()
@@ -1572,6 +1579,10 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) {
                 if cancel || 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);
+                }
             });
         });
     });
@@ -1871,6 +1882,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.open_file(
             "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

@@ -1001,6 +1001,78 @@ pub fn drop_key_name_text(raw: &mut egui::RawInput, text_field_focused: bool) {
 mod tests {
     use super::*;
 
+    /// 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());
+    }
+
     /// The Control panel's two-row groups were centred as if they were one row tall, so the second
     /// row hung below the panel and was cut off.
     #[test]

crates/ui-egui/src/menus.rs

@@ -1772,9 +1772,28 @@ pub fn menu_enabled(app: &DesignApp, id: &str) -> bool {
     enabled(app, id)
 }
 
+/// 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}));
+        }
+    }
+}
+
 /// 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`]).
 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;