From 29c014deef9abe85f9a23721490be9a5345b5332 Mon Sep 17 00:00:00 2001 From: Marco Arnulfo Date: Fri, 16 Oct 2026 02:01:15 +0200 Subject: [PATCH] Ask before a session opened in the browser replaces a modified one (fixes #432) Picking a session with Open Session or dropping one on the page in the browser replaced the document as soon as its bytes were read, without the Save Changes question the native Open goes through, so unsaved edits and their undo history were lost. The bytes now wait behind that question, asked when the read finishes: Don't Save opens them, Save saves first and then opens them, Cancel drops them and leaves the document alone. --- apps/soundcraft-web/src/web.rs | 20 +++------- crates/ui-egui/src/dialogs.rs | 46 ++++++++++++++++++++-- crates/ui-egui/tests/save_changes.rs | 58 +++++++++++++++++++++++++++- 3 files changed, 106 insertions(+), 18 deletions(-) diff --git a/apps/soundcraft-web/src/web.rs b/apps/soundcraft-web/src/web.rs index 3682fab7..0d9b6485 100644 --- a/apps/soundcraft-web/src/web.rs +++ b/apps/soundcraft-web/src/web.rs @@ -1,6 +1,5 @@ //! The browser shell: the eframe web runner around [`SoundApp`]. -use serde_json::json; use soundcraft_engine::{Engine, io}; use soundcraft_ui_egui::dialogs::Dialog; use soundcraft_ui_egui::sampler_window::SampleRead; @@ -166,30 +165,23 @@ impl WebShell { } } - fn handle(&mut self, purpose: Purpose, name: &str, bytes: &[u8]) { - if matches!(purpose, Purpose::Session) && soundcraft_ui_egui::bounce::refuses_session_switch(&mut self.app) { - return; - } + fn handle(&mut self, purpose: Purpose, name: String, bytes: Vec) { let e = &mut self.app.engine; let at = e.session().edit.selection.start.max(0); let result = match purpose { - Purpose::Session => { - let _ = self.app.run("transport.stop", json!({})); - io::open_session_bytes(&mut self.app.engine, name, bytes).map(|missing| { - if missing.is_empty() { format!("Opened {name}") } else { format!("Opened {name} ({} missing media)", missing.len()) } - }) - } + // Asks first when the open session has changes: the bytes wait for the answer. + Purpose::Session => return soundcraft_ui_egui::dialogs::open_session_bytes(&mut self.app, name, bytes), // Imports are edits: one undo step each, like the import commands. Purpose::Audio => { let before = e.session_arc(); - io::import_audio_bytes(e, name, bytes, None, None, at).map(|_| { + io::import_audio_bytes(e, &name, &bytes, None, None, at).map(|_| { e.push_undo("Import Audio", before); format!("Imported {name}") }) } Purpose::Midi => { let before = e.session_arc(); - io::import_midi_bytes(e, bytes, at, true).map(|_| { + io::import_midi_bytes(e, &bytes, at, true).map(|_| { e.push_undo("Import MIDI", before); format!("Imported {name}") }) @@ -205,7 +197,7 @@ impl eframe::App for WebShell { self.take_path_prompt(ctx); let pending = std::mem::take(&mut *self.inbox.borrow_mut()); for (purpose, name, bytes) in pending { - self.handle(purpose, &name, &bytes); + self.handle(purpose, name, bytes); } let samples = std::mem::take(&mut *self.samples.borrow_mut()); for (read, bytes) in samples { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 713e94b7..7b315ad5 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -34,6 +34,8 @@ pub struct Dialogs { pub open: Option, /// Runs once the session has been saved from `SaveChanges`. after_save: Option<(String, Value)>, + /// A session the browser read while the open one had changes, waiting for `SaveChanges`. + pending_session: Option<(String, Vec)>, } impl Dialogs { @@ -155,7 +157,7 @@ impl Dialogs { /// When `id` would close a session with unsaved changes, ask first; true when the question is /// open instead. A take in progress is stopped first, so it counts as a change. pub fn ask_to_save(app: &mut SoundApp, id: &str, params: &Value) -> bool { - if !matches!(id, "session.new" | "session.open" | "session.close" | "session.revert" | "app.quit") + if !matches!(id, "session.new" | "session.open" | "session.close" | "session.revert" | "app.quit" | OPEN_PENDING) || soundcraft_engine::find_command(id).is_some_and(|c| (c.enabled)(&app.engine).is_err()) { return false; @@ -178,6 +180,34 @@ pub fn run_or_ask(app: &mut SoundApp, id: &str, params: Value) -> Result) { + if crate::bounce::refuses_session_switch(app) { + return; + } + app.dialogs.pending_session = Some((name, bytes)); + if !ask_to_save(app, OPEN_PENDING, &json!({})) { + open_pending_session(app); + } +} + +fn open_pending_session(app: &mut SoundApp) { + let Some((name, bytes)) = app.dialogs.pending_session.take() else { return }; + if crate::bounce::refuses_session_switch(app) { + return; + } + let _ = app.run("transport.stop", json!({})); + app.ui.status = match soundcraft_engine::io::open_session_bytes(&mut app.engine, &name, &bytes) { + Ok(missing) if missing.is_empty() => format!("Opened {name}"), + Ok(missing) => format!("Opened {name} ({} missing media)", missing.len()), + Err(e) => e.to_string(), + }; +} + /// A path prompt for any command whose parameters start with a required `path` or `dir` (plus /// Print Score, whose path is optional), with a default file name from the command. pub fn takes_path(id: &str) -> bool { @@ -594,6 +624,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { if esc || ctx.memory(|m| m.data.get_temp::(egui::Id::new("dlg_cancel")).unwrap_or(false)) { keep = false; app.dialogs.after_save = None; + app.dialogs.pending_session = None; ctx.memory_mut(|m| m.data.remove::(egui::Id::new("dlg_cancel"))); } if let Some(then) = save_first { @@ -613,7 +644,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { let saving = action.as_ref().is_some_and(|(cmd, _)| cmd.starts_with("session.save")); if let Some((cmd, params)) = action { keep = false; - match app.run(&cmd, params) { + match run_then(app, &cmd, params) { Ok(v) if cmd == "file.export_clips" => { app.ui.status = format!("Exported {} clip(s) to {}", v["written"], v["dir"].as_str().unwrap_or_default()); } @@ -624,7 +655,7 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { if saving && let Some((then, params)) = app.dialogs.after_save.take() && !app.engine.is_dirty() - && let Err(e) = app.run(&then, params) + && let Err(e) = run_then(app, &then, params) { app.ui.status = e; } @@ -633,6 +664,15 @@ pub fn show(app: &mut SoundApp, ctx: &egui::Context) { } } +/// Runs a dialog's command, or what `SaveChanges` carries on with. +fn run_then(app: &mut SoundApp, cmd: &str, params: Value) -> Result { + if cmd == OPEN_PENDING { + open_pending_session(app); + return Ok(Value::Null); + } + app.run(cmd, params) +} + /// Cancel/OK row; returns true when OK was pressed (or Enter). fn buttons(ui: &mut egui::Ui, ok: &str, enter: bool) -> bool { let mut pressed = enter; diff --git a/crates/ui-egui/tests/save_changes.rs b/crates/ui-egui/tests/save_changes.rs index e500a8d5..caa0561d 100644 --- a/crates/ui-egui/tests/save_changes.rs +++ b/crates/ui-egui/tests/save_changes.rs @@ -2,7 +2,7 @@ use egui_kittest::Harness; use egui_kittest::kittest::Queryable; use serde_json::json; use soundcraft_ui_egui::dialogs::Dialog; -use soundcraft_ui_egui::{Services, SoundApp, menus}; +use soundcraft_ui_egui::{Services, SoundApp, dialogs, menus}; fn harness() -> Harness<'static, SoundApp> { let app = SoundApp::new(soundcraft_engine::demo::demo_engine(), None, Services::default()); @@ -95,3 +95,59 @@ fn save_in_the_question_saves_before_closing() { assert!(!h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute); let _ = std::fs::remove_dir_all(&dir); } + +/// A session the browser read: an empty one, so opening it is easy to see. +fn session_bytes() -> Vec { + soundcraft_engine::Engine::default().session().to_json().unwrap().into_bytes() +} + +fn open_bytes(h: &mut Harness<'_, SoundApp>) { + dialogs::open_session_bytes(h.state_mut(), "Other.scraft".into(), session_bytes()); + steps(h); +} + +#[test] +fn a_session_read_in_the_browser_asks_before_replacing_a_modified_one() { + let mut h = harness(); + open_bytes(&mut h); + assert!(h.state().dialogs.open.is_none(), "an unmodified session is replaced without asking"); + assert!(h.state().engine.session().tracks.is_empty()); + + let mut h = harness(); + let tracks = h.state().engine.session().tracks.len(); + mute_kick(&mut h); + open_bytes(&mut h); + assert_eq!(h.state().dialogs.open_name(), Some("save_changes")); + click(&mut h, "Cancel"); + assert!(h.state().dialogs.open.is_none()); + assert_eq!(h.state().engine.session().tracks.len(), tracks); + h.state_mut().run("edit.undo", json!({})).unwrap(); + assert!(!h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute, "Cancel keeps the undo history"); + + mute_kick(&mut h); + open_bytes(&mut h); + click(&mut h, "Don't Save"); + assert!(h.state().engine.session().tracks.is_empty()); +} + +#[test] +fn save_in_the_question_saves_before_opening_a_session_read_in_the_browser() { + let dir = std::env::temp_dir().join(format!("soundcraft-save-changes-bytes-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + let path = dir.join("Song.scraft").to_string_lossy().into_owned(); + + let mut h = harness(); + mute_kick(&mut h); + open_bytes(&mut h); + click(&mut h, "Save"); + match &mut h.state_mut().dialogs.open { + Some(Dialog::PathPrompt { cmd, path: p, .. }) if cmd == "session.save_as" => p.clone_from(&path), + other => panic!("expected Save As, got {other:?}"), + } + steps(&mut h); + click(&mut h, "OK"); + assert!(h.state().engine.session().tracks.is_empty()); + h.state_mut().run("session.open", json!({"path": path})).unwrap(); + assert!(h.state().engine.session().track_by_name("Kick").unwrap().mixer.mute); + let _ = std::fs::remove_dir_all(&dir); +}