From 29506f3e932fa20b87d0d15688dd98cfb0740330 Mon Sep 17 00:00:00 2001 From: Torstein <49699340+torstfugl@users.noreply.github.com> Date: Sun, 4 Oct 2026 18:21:25 +0200 Subject: [PATCH] feat(ui): confirm before discarding unsaved changes (#31) Close, Close All, Close Others, Revert, Exit and the window close button now ask Save / Don't Save / Cancel when a document has unsaved changes. The action is parked until each dirty document has been answered, then re-run. Window close requests are cancelled while the prompt is open. Closes #9 --- crates/ui-egui/src/canvas.rs | 4 +- crates/ui-egui/src/control.rs | 1 + crates/ui-egui/src/discard_ui.rs | 316 ++++++++++++++++++++++++++ crates/ui-egui/src/lib.rs | 9 + crates/ui-egui/src/menus.rs | 8 + crates/ui-egui/src/shortcuts.rs | 2 +- crates/ui-egui/src/view_cmds/tests.rs | 6 +- 7 files changed, 340 insertions(+), 6 deletions(-) create mode 100644 crates/ui-egui/src/discard_ui.rs diff --git a/crates/ui-egui/src/canvas.rs b/crates/ui-egui/src/canvas.rs index 447fcc7b..b910bdc1 100644 --- a/crates/ui-egui/src/canvas.rs +++ b/crates/ui-egui/src/canvas.rs @@ -502,7 +502,7 @@ fn tabs(app: &mut PhotocraftApp, ui: &mut egui::Ui) { app.session.set_active(i); } if let Some(i) = close { - let _ = app.run("file.close", json!({"document": i})); + let _ = crate::menus::invoke(app, ui.ctx(), "file.close", json!({"document": i})); } } @@ -549,7 +549,7 @@ fn pro_tabs(app: &mut PhotocraftApp, ui: &mut egui::Ui) { app.session.set_active(i); } if let Some(i) = close { - let _ = app.run("file.close", json!({"document": i})); + let _ = crate::menus::invoke(app, ui.ctx(), "file.close", json!({"document": i})); } } diff --git a/crates/ui-egui/src/control.rs b/crates/ui-egui/src/control.rs index f3883c38..9d3fc949 100644 --- a/crates/ui-egui/src/control.rs +++ b/crates/ui-egui/src/control.rs @@ -313,6 +313,7 @@ pub fn handle(app: &mut PhotocraftApp, ctx: &egui::Context, req: &ControlRequest }, "app.save" => wrap(app.save_as(s("path").map(str::to_string)).map(|p| json!({"path": p}))), "app.quit" => { + app.allow_close = true; ctx.send_viewport_cmd(egui::ViewportCommand::Close); ok(Value::Null) } diff --git a/crates/ui-egui/src/discard_ui.rs b/crates/ui-egui/src/discard_ui.rs new file mode 100644 index 00000000..cb64cbe5 --- /dev/null +++ b/crates/ui-egui/src/discard_ui.rs @@ -0,0 +1,316 @@ +//! Save / Don't Save / Cancel prompt for actions that would throw away unsaved changes: closing +//! documents, Revert, and leaving the app (File › Exit or the window's close button). +//! +//! egui is immediate-mode, so the action is parked in [`Prompt`] while the modal is up and re-run +//! once every affected document has been answered. Cancel at any point drops it. Documents are +//! tracked by id, not tab index, so closing one elsewhere while the prompt is up can't retarget it. + +use photocraft_doc::DocId; +use serde_json::{Value, json}; + +use crate::PhotocraftApp; + +const EXIT: &str = "file.exit"; + +/// An action waiting on the user, and the dirty documents still to ask about. +pub struct Prompt { + id: String, + params: Value, + /// The document the command was aimed at (`document` param, else the active one), if it has one. + target: Option, + docs: Vec, +} + +fn index_of(app: &PhotocraftApp, id: DocId) -> Option { + app.session.documents().iter().position(|d| d.doc.id == id) +} + +/// Which documents a command discards, relative to its target (the `document` param, else the active one). +#[derive(Clone, Copy)] +enum Reach { + Target, + AllButTarget, + All, +} + +/// Every command that can throw away unsaved changes. A new one only needs a row here. +const DISCARDING: &[(&str, Reach)] = &[ + ("file.close", Reach::Target), + ("file.revert", Reach::Target), + ("file.closeOthers", Reach::AllButTarget), + ("file.closeAll", Reach::All), + (EXIT, Reach::All), +]; + +/// The command's target document and the unsaved documents it would discard (empty for commands +/// that discard nothing). +fn discarded(app: &PhotocraftApp, id: &str, params: &Value) -> (Option, Vec) { + let docs = app.session.documents(); + let target = params.get("document").and_then(Value::as_u64).map(|i| i as usize).or(app.session.active_index()); + let affected: Vec = match DISCARDING.iter().find(|(c, _)| *c == id) { + Some((_, Reach::Target)) => target.into_iter().collect(), + Some((_, Reach::AllButTarget)) => (0..docs.len()).filter(|&i| Some(i) != target).collect(), + Some((_, Reach::All)) => (0..docs.len()).collect(), + None => Vec::new(), + }; + let dirty = affected.into_iter().filter_map(|i| docs.get(i)).filter(|d| d.is_dirty()).map(|d| d.doc.id).collect(); + (target.and_then(|i| docs.get(i)).map(|d| d.doc.id), dirty) +} + +/// Park `id` behind a prompt if it would discard unsaved work. Returns whether it did. +pub fn intercept(app: &mut PhotocraftApp, id: &str, params: &Value) -> bool { + let (target, docs) = discarded(app, id, params); + if docs.is_empty() { + return false; + } + let prompt = Prompt { id: id.to_string(), params: params.clone(), target, docs }; + match &app.discard { + None => app.discard = Some(prompt), + // Quitting overrides whatever is pending: it covers every document, so nothing is lost. + Some(open) if id == EXIT && open.id != EXIT => app.discard = Some(prompt), + // Repeated quit requests (the X pressed again) keep the prompt, and the answers so far. + Some(open) if open.id == id => {} + Some(_) => { + app.ui.status = "Answer the unsaved-changes prompt first".into(); + app.ui.status_error = true; + } + } + true +} + +/// Called once per frame: holds back a window close request while there is unsaved work. +pub fn guard_window_close(app: &mut PhotocraftApp, ctx: &egui::Context) { + if app.allow_close || !ctx.input(|i| i.viewport().close_requested()) { + return; + } + if intercept(app, EXIT, &Value::Null) { + ctx.send_viewport_cmd(egui::ViewportCommand::CancelClose); + } +} + +/// Moves on to the next document, or runs the parked action once none are left. +fn advance(app: &mut PhotocraftApp, ctx: &egui::Context) { + let Some(p) = app.discard.as_mut() else { return }; + if !p.docs.is_empty() { + p.docs.remove(0); + } + if !p.docs.is_empty() { + return; + } + let Some(Prompt { id, mut params, target, .. }) = app.discard.take() else { return }; + if id == EXIT { + app.allow_close = true; + ctx.send_viewport_cmd(egui::ViewportCommand::Close); + return; + } + if let Some(target) = target { + // The tab may have moved since the command was issued; aim it at the same document. + let Some(i) = index_of(app, target) else { return }; + params = json!({"document": i}); + } + if let Err(e) = crate::menus::invoke_unguarded(app, ctx, &id, params) { + app.ui.status = e; + app.ui.status_error = true; + } +} + +/// Saves `doc` in place; false when it is gone, the save failed or its file dialog was cancelled. +fn save(app: &mut PhotocraftApp, ctx: &egui::Context, doc: DocId) -> bool { + let Some(i) = index_of(app, doc) else { return false }; + app.session.set_active(i); + match crate::menus::invoke_unguarded(app, ctx, "file.save", json!({})) { + Ok(_) => true, + // Backing out of the file dialog is the user's choice, not an error. + Err(e) if e == "cancelled" => false, + Err(e) => { + app.ui.status = format!("Couldn't save: {e}"); + app.ui.status_error = true; + false + } + } +} + +pub fn show(app: &mut PhotocraftApp, ctx: &egui::Context) { + let Some(p) = &app.discard else { return }; + let Some(&doc) = p.docs.first() else { return }; + let (exits, reverts) = (p.id == EXIT, p.id == "file.revert"); + let Some(name) = index_of(app, doc).map(|i| app.session.documents()[i].doc.name.clone()) else { + // Closed by something else while the prompt was up: nothing left to ask about. + advance(app, ctx); + return; + }; + let (title, message) = if reverts { + ("Revert", format!("Revert “{name}” to the last saved version? Your changes will be lost.")) + } else { + let verb = if exits { "quitting" } else { "closing" }; + ("Unsaved changes", format!("Do you want to save the changes you made to “{name}” before {verb}?")) + }; + let (mut save_it, mut discard_it, mut cancel) = (false, false, false); + let modal = egui::Modal::new(egui::Id::new("discard-prompt")).show(ctx, |ui| { + ui.set_max_width(420.0); + ui.label(egui::RichText::new(title).font(crate::theme::semibold(15.0))); + ui.add_space(4.0); + crate::widgets::hairline(ui); + ui.add_space(8.0); + ui.label(message); + ui.add_space(12.0); + ui.with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| { + ui.spacing_mut().item_spacing.x = 10.0; + if reverts { + discard_it = crate::widgets::primary_button(ui, "Revert", 84.0).clicked(); + cancel = crate::widgets::secondary_button(ui, "Cancel", 84.0).clicked(); + } else { + save_it = crate::widgets::primary_button(ui, "Save", 84.0).clicked(); + cancel = crate::widgets::secondary_button(ui, "Cancel", 84.0).clicked(); + discard_it = crate::widgets::secondary_button(ui, "Don't Save", 100.0).clicked(); + } + }); + }); + cancel |= modal.should_close(); + if cancel { + app.discard = None; + } else if discard_it || (save_it && save(app, ctx, doc)) { + advance(app, ctx); + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn app_with_docs(n: usize) -> PhotocraftApp { + let mut app = PhotocraftApp::new(photocraft_engine::Session::new(), crate::Services::default()); + for _ in 0..n { + app.run("file.new", json!({"width": 8, "height": 8})).unwrap(); + } + app.sync_views(); + app + } + + fn make_dirty(app: &mut PhotocraftApp, doc: usize) { + app.session.set_active(doc); + app.run("layer.new.layer", json!({})).unwrap(); + } + + fn doc_id(app: &PhotocraftApp, i: usize) -> DocId { + app.session.documents()[i].doc.id + } + + #[test] + fn clean_documents_close_without_asking() { + let mut app = app_with_docs(2); + let ctx = egui::Context::default(); + crate::menus::invoke(&mut app, &ctx, "file.close", json!({})).unwrap(); + assert!(app.discard.is_none()); + assert_eq!(app.session.documents().len(), 1); + } + + #[test] + fn closing_a_dirty_document_waits_for_an_answer() { + let mut app = app_with_docs(2); + let ctx = egui::Context::default(); + make_dirty(&mut app, 1); + crate::menus::invoke(&mut app, &ctx, "file.close", json!({"document": 1})).unwrap(); + assert_eq!(app.session.documents().len(), 2, "nothing closes before the user answers"); + assert!(app.discard.is_some()); + advance(&mut app, &ctx); + assert!(app.discard.is_none()); + assert_eq!(app.session.documents().len(), 1); + } + + #[test] + fn only_documents_that_would_be_discarded_are_asked_about() { + let mut app = app_with_docs(3); + make_dirty(&mut app, 0); + make_dirty(&mut app, 2); + let ids = [doc_id(&app, 0), doc_id(&app, 2)]; + assert_eq!(discarded(&app, "file.closeOthers", &json!({"document": 0})), (Some(ids[0]), vec![ids[1]])); + assert_eq!(discarded(&app, "file.closeAll", &json!({})).1, ids); + assert_eq!(discarded(&app, EXIT, &json!({})).1, ids); + assert!(discarded(&app, "view.zoomIn", &json!({})).1.is_empty()); + } + + #[test] + fn close_all_asks_per_dirty_document_then_runs() { + let mut app = app_with_docs(3); + let ctx = egui::Context::default(); + make_dirty(&mut app, 0); + make_dirty(&mut app, 2); + crate::menus::invoke(&mut app, &ctx, "file.closeAll", json!({})).unwrap(); + advance(&mut app, &ctx); + assert_eq!(app.session.documents().len(), 3, "still waiting on the second document"); + advance(&mut app, &ctx); + assert!(app.session.documents().is_empty()); + } + + #[test] + fn the_parked_close_still_hits_its_document_after_tabs_move() { + let mut app = app_with_docs(3); + let ctx = egui::Context::default(); + make_dirty(&mut app, 2); + let doomed = doc_id(&app, 2); + crate::menus::invoke(&mut app, &ctx, "file.close", json!({"document": 2})).unwrap(); + // Another document closes while the prompt is up, shifting the tab to index 1. + app.run("file.close", json!({"document": 0})).unwrap(); + advance(&mut app, &ctx); + assert_eq!(app.session.documents().len(), 1); + assert!(index_of(&app, doomed).is_none()); + } + + #[test] + fn quitting_replaces_a_pending_close_but_other_requests_are_refused() { + let mut app = app_with_docs(2); + make_dirty(&mut app, 0); + make_dirty(&mut app, 1); + assert!(intercept(&mut app, "file.close", &json!({"document": 0}))); + assert!(intercept(&mut app, "file.closeAll", &Value::Null)); + assert_eq!(app.discard.as_ref().map(|p| (p.id.as_str(), p.docs.len())), Some(("file.close", 1))); + assert!(app.ui.status_error); + assert!(intercept(&mut app, EXIT, &Value::Null)); + assert_eq!(app.discard.as_ref().map(|p| (p.id.as_str(), p.docs.len())), Some((EXIT, 2))); + // Pressing the window's X again must not restart the questions. + app.discard.as_mut().unwrap().docs.remove(0); + assert!(intercept(&mut app, EXIT, &Value::Null)); + assert_eq!(app.discard.as_ref().unwrap().docs.len(), 1); + } + + #[test] + fn exit_is_held_back_until_confirmed() { + let mut app = app_with_docs(1); + let ctx = egui::Context::default(); + make_dirty(&mut app, 0); + assert!(intercept(&mut app, EXIT, &Value::Null)); + assert!(!app.allow_close); + advance(&mut app, &ctx); + assert!(app.allow_close); + } + + /// One frame with the window's close button pressed; whether the guard cancelled the close. + fn press_window_close(app: &mut PhotocraftApp) -> bool { + let mut info = egui::ViewportInfo::default(); + info.events.push(egui::ViewportEvent::Close); + let mut input = egui::RawInput::default(); + input.viewports.insert(egui::ViewportId::ROOT, info); + let mut out = egui::Context::default().run_ui(input, |ui| guard_window_close(app, ui.ctx())); + out.textures_delta.clear(); + out.viewport_output[&egui::ViewportId::ROOT].commands.iter().any(|c| matches!(c, egui::ViewportCommand::CancelClose)) + } + + #[test] + fn window_close_is_cancelled_while_work_is_unsaved() { + let mut app = app_with_docs(1); + make_dirty(&mut app, 0); + assert!(press_window_close(&mut app)); + assert!(app.discard.is_some()); + } + + #[test] + fn window_close_goes_through_when_nothing_is_unsaved_or_it_was_confirmed() { + let mut app = app_with_docs(1); + assert!(!press_window_close(&mut app)); + make_dirty(&mut app, 0); + app.allow_close = true; + assert!(!press_window_close(&mut app)); + assert!(app.discard.is_none()); + } +} diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 12221d3d..d3aa89c6 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -20,6 +20,7 @@ pub mod color_picker_ui; pub mod comps_ui; pub mod control; pub mod dialogs; +pub mod discard_ui; pub mod distort_ui; pub mod doc_props_ui; pub mod enable_rules; @@ -221,6 +222,10 @@ pub struct PhotocraftApp { pub perf: gpu_canvas::Perf, /// Preferences, autosave and snapping runtime state (see `prefs_ui`, `snap_ui`). pub(crate) prefs_rt: prefs_ui::Runtime, + /// Close, Revert or Exit parked behind the unsaved-changes prompt (see `discard_ui`). + pub(crate) discard: Option, + /// Set once the user has agreed to quit, so the resulting close request goes through. + pub(crate) allow_close: bool, #[cfg(all(debug_assertions, not(target_arch = "wasm32")))] live_tokens: theme::live::LiveTokens, } @@ -270,6 +275,8 @@ impl PhotocraftApp { gpu: None, perf: Default::default(), prefs_rt: Default::default(), + discard: None, + allow_close: false, #[cfg(all(debug_assertions, not(target_arch = "wasm32")))] live_tokens: theme::live::LiveTokens::from_env(), }; @@ -493,6 +500,7 @@ impl eframe::App for PhotocraftApp { self.collect_screenshots(ctx); self.issue_screenshots(ctx); prefs_ui::tick(self, ctx); + discard_ui::guard_window_close(self, ctx); shortcuts::handle(self, ctx); let arrived: Vec<(String, Vec)> = self.services.inbox.as_ref().map(|q| std::mem::take(&mut *q.lock().unwrap_or_else(|e| e.into_inner()))).unwrap_or_default(); @@ -568,6 +576,7 @@ impl eframe::App for PhotocraftApp { workspace_ui::windows(self, &ctx); palette::show(self, &ctx); dialogs::show(self, &ctx); + discard_ui::show(self, &ctx); distort_ui::show(self, &ctx); camera_raw_ui::show(self, &ctx); wide_angle_ui::show(self, &ctx); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index a7e4ccb1..7f0eff45 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -98,6 +98,14 @@ fn workspace_name(id: &str) -> Option<&'static str> { /// Run a command id from any source (menu, shortcut, palette, automation). pub fn invoke(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result { + if crate::discard_ui::intercept(app, id, ¶ms) { + return Ok(Value::Null); + } + invoke_unguarded(app, ctx, id, params) +} + +/// [`invoke`] without the unsaved-changes prompt, for once the user has already answered it. +pub(crate) fn invoke_unguarded(app: &mut PhotocraftApp, ctx: &egui::Context, id: &str, params: Value) -> Result { // Help › Discord, website, GitHub, Report an Issue. if let Some(url) = crate::links::url_for(id) { return Ok(crate::links::open(app, ctx, url)); diff --git a/crates/ui-egui/src/shortcuts.rs b/crates/ui-egui/src/shortcuts.rs index 38a0731f..3d2f1799 100644 --- a/crates/ui-egui/src/shortcuts.rs +++ b/crates/ui-egui/src/shortcuts.rs @@ -49,7 +49,7 @@ pub fn pretty(s: &str) -> String { } pub fn handle(app: &mut PhotocraftApp, ctx: &egui::Context) { - if ctx.egui_wants_keyboard_input() || !app.ui.dialogs.is_empty() { + if ctx.egui_wants_keyboard_input() || !app.ui.dialogs.is_empty() || app.discard.is_some() { return; } // Liquify / Puppet Warp / Perspective Warp: ↩ commits, Esc cancels. diff --git a/crates/ui-egui/src/view_cmds/tests.rs b/crates/ui-egui/src/view_cmds/tests.rs index b639243b..eaabdddd 100644 --- a/crates/ui-egui/src/view_cmds/tests.rs +++ b/crates/ui-egui/src/view_cmds/tests.rs @@ -164,11 +164,11 @@ fn engine_commands_open_their_dialogs() { assert!(app.run("type.info", json!({})).unwrap()["warp"]["style"] == "warpArc"); // Batch needs a recorded action. assert!(menu(&mut app, &ctx, "file.automate.batch", json!({})).is_err()); - // Close Others keeps the active document's view. + // Close Others keeps the active document's view (the documents are dirty; the unsaved-changes prompt is covered in discard_ui). app.ui.views[1].zoom = 4.0; - menu(&mut app, &ctx, "file.closeOthers", json!({})).unwrap(); + crate::menus::invoke_unguarded(&mut app, &ctx, "file.closeOthers", json!({})).unwrap(); assert_eq!(app.ui.views.len(), 1); assert_eq!(app.ui.views[0].zoom, 4.0); - menu(&mut app, &ctx, "file.closeAll", json!({})).unwrap(); + crate::menus::invoke_unguarded(&mut app, &ctx, "file.closeAll", json!({})).unwrap(); assert!(app.ui.views.is_empty() && app.session.documents().is_empty()); }