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

@@ -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),

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| {