From b69613bf8868976a8a2987b99840540a5b45a53e Mon Sep 17 00:00:00 2001
From: Gert Pellin <gert@pellin.be>
Date: Wed, 14 Oct 2026 23:58:43 +0200
Subject: [PATCH] Preferences: Cancel, Esc and the close button undo the
 dialog's changes (#752)

The Preferences dialog applies its controls as they change (appearance,
interface size, the editing checkboxes, the comment author), and its Cancel
result was discarded, so Cancel did nothing and the window stayed open.

The dialog now remembers the settings it opened with. Cancel, Esc and the
window's close button put them back and close it; Done keeps the changes.
---
 crates/ui-egui/src/dialogs.rs | 40 +++++++++++++++++++--
 crates/ui-egui/src/lib.rs     | 65 +++++++++++++++++++++++++++++++++++
 2 files changed, 103 insertions(+), 2 deletions(-)

diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs
index 18d112c8..abe8c87e 100644
--- a/crates/ui-egui/src/dialogs.rs
+++ b/crates/ui-egui/src/dialogs.rs
@@ -108,6 +108,9 @@ pub fn show(app: &mut SlideApp, ctx: &egui::Context) {
     let escape = ctx.input(|i| i.key_pressed(egui::Key::Escape));
     if open && !close && !escape {
         app.dialog = Some(d);
+    } else if !close && d.id == "preferences" {
+        // Esc or the window's close button: the same as Cancel.
+        restore_preferences(app, &d);
     } else if app.dialog.is_none()
         && let Some(ret) = d.params.get("returnParams").filter(|v| v.is_object()).cloned()
     {
@@ -137,6 +140,31 @@ fn buttons(ui: &mut Ui, ok_label: &str) -> (bool, bool) {
     (ok, cancel)
 }
 
+/// What the Preferences dialog can change, as it was when the dialog opened.
+fn preferences_snapshot(app: &SlideApp) -> Value {
+    json!({
+        "dark": app.ui.brightness == theme::Brightness::Dark,
+        "uiScale": app.ui.ui_scale,
+        "prefs": serde_json::to_value(&app.session.prefs).unwrap_or(Value::Null),
+    })
+}
+
+/// Puts back the settings the Preferences dialog opened with (Cancel, Esc, the close button).
+fn restore_preferences(app: &mut SlideApp, d: &Dialog) {
+    let Some(before) = d.params.get("before") else { return };
+    if let Some(dark) = before.get("dark").and_then(Value::as_bool)
+        && dark != (app.ui.brightness == theme::Brightness::Dark)
+    {
+        let _ = app.run("view.dark", json!({ "on": dark }));
+    }
+    if let Some(scale) = before.get("uiScale").and_then(Value::as_f64) {
+        app.ui.ui_scale = scale as f32;
+    }
+    if let Some(prefs) = before.get("prefs").and_then(|p| serde_json::from_value(p.clone()).ok()) {
+        app.session.prefs = prefs;
+    }
+}
+
 fn field(ui: &mut Ui, d: &mut Dialog, label: &str, key: &str, default: &str) {
     let mut v = d.get(key, default);
     ui.horizontal(|ui| {
@@ -2104,6 +2132,11 @@ fn body(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool {
         }
         "gridOptions" => grid_options(app, ui, d),
         "preferences" => {
+            // The controls apply as they change, so the dialog remembers what it opened with and
+            // Cancel (or Esc, or the close button: see `show`) puts that back.
+            if d.params.get("before").is_none() {
+                d.params = json!({ "before": preferences_snapshot(app) });
+            }
             ui.label(egui::RichText::new("General").font(theme::bold(13.0)));
             let mut dark = app.ui.brightness == theme::Brightness::Dark;
             if ui.checkbox(&mut dark, "Dark appearance").changed() {
@@ -2130,8 +2163,11 @@ fn body(app: &mut SlideApp, ui: &mut Ui, d: &mut Dialog) -> bool {
                 ui.label("Your name (comments):");
                 ui.text_edit_singleline(&mut app.session.prefs.author);
             });
-            let (ok, _) = buttons(ui, "Done");
-            ok
+            let (ok, cancel) = buttons(ui, "Done");
+            if cancel {
+                restore_preferences(app, d);
+            }
+            ok || cancel
         }
         "photoAlbum" => photo_album_dialog(app, ui, d),
         "themeColors" => theme_colors_dialog(app, ui, d),
diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs
index f020f5f5..9d4a681b 100644
--- a/crates/ui-egui/src/lib.rs
+++ b/crates/ui-egui/src/lib.rs
@@ -2573,6 +2573,71 @@ mod tests {
         assert!((harness.state().session.prefs.grid_spacing - 6.0).abs() < 1e-9);
     }
 
+    /// Preferences (#752): Cancel, Esc and the window's close button put back what the dialog
+    /// changed while it was open; Done keeps it.
+    #[test]
+    fn preferences_cancel_restores_the_settings_done_keeps_them() {
+        use egui_kittest::kittest::Queryable;
+        let mut harness = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state(
+            |ui, app: &mut SlideApp| {
+                app.logic(ui.ctx());
+                app.ui(ui);
+            },
+            SlideApp::new(Session::with_new(), Services::default()),
+        );
+        harness.run_steps(2);
+        let before = harness.state().session.prefs.clone();
+        let dark_before = harness.state().ui.brightness;
+        let change = |harness: &mut egui_kittest::Harness<'static, SlideApp>| {
+            harness.state_mut().run("app.preferences", json!({})).unwrap();
+            harness.run_steps(2);
+            for label in ["Dark appearance", "Show smart guides", "Snap objects to grid"] {
+                harness
+                    .query_all_by_role_and_label(egui::accesskit::Role::CheckBox, label)
+                    .next()
+                    .unwrap_or_else(|| panic!("no {label} checkbox"))
+                    .click_accesskit();
+                harness.run_steps(2);
+            }
+            assert_ne!(harness.state().ui.brightness, dark_before, "the dialog previews the appearance");
+            assert_ne!(harness.state().session.prefs.smart_guides, before.smart_guides);
+        };
+        let unchanged = |harness: &egui_kittest::Harness<'static, SlideApp>, how: &str| {
+            let s = harness.state();
+            assert!(s.dialog.is_none(), "{how} closes Preferences");
+            assert_eq!(s.ui.brightness, dark_before, "{how} restores the appearance");
+            assert_eq!(s.session.prefs.smart_guides, before.smart_guides, "{how} restores smart guides");
+            assert_eq!(s.session.prefs.snap_to_grid, before.snap_to_grid, "{how} restores snap to grid");
+        };
+
+        change(&mut harness);
+        harness
+            .query_all_by_role_and_label(egui::accesskit::Role::Button, "Cancel")
+            .next()
+            .unwrap_or_else(|| panic!("no Cancel button"))
+            .click_accesskit();
+        harness.run_steps(2);
+        unchanged(&harness, "Cancel");
+
+        change(&mut harness);
+        harness.key_press(egui::Key::Escape);
+        harness.run_steps(2);
+        unchanged(&harness, "Esc");
+
+        change(&mut harness);
+        harness
+            .query_all_by_role_and_label(egui::accesskit::Role::Button, "Done")
+            .next()
+            .unwrap_or_else(|| panic!("no Done button"))
+            .click_accesskit();
+        harness.run_steps(2);
+        let s = harness.state();
+        assert!(s.dialog.is_none(), "Done closes Preferences");
+        assert_ne!(s.ui.brightness, dark_before, "Done keeps the appearance");
+        assert_ne!(s.session.prefs.smart_guides, before.smart_guides, "Done keeps smart guides");
+        assert_ne!(s.session.prefs.snap_to_grid, before.snap_to_grid, "Done keeps snap to grid");
+    }
+
     fn canvas_harness() -> egui_kittest::Harness<'static, SlideApp> {
         let mut harness = egui_kittest::Harness::builder().with_size(egui::vec2(1200.0, 800.0)).build_ui_state(
             |ui, app: &mut SlideApp| {