diff --git a/apps/deckcraft-web/src/web.rs b/apps/deckcraft-web/src/web.rs index 67f5157c5..7720b605c 100644 --- a/apps/deckcraft-web/src/web.rs +++ b/apps/deckcraft-web/src/web.rs @@ -58,7 +58,11 @@ pub fn start() { } let mut app = SlideApp::new(Session::new(), services); // In host mode the parent sends the presentation to edit; no sample meanwhile. - if app_bridge.is_none() && !query_has("blank") { + if app_bridge.is_some() { + if let Err(e) = app.start_for_host() { + log::error!("new presentation: {e}"); + } + } else if !query_has("blank") { if let Err(e) = deckcraft_engine::sample::open_sample(&mut app.session) { log::error!("sample deck: {e}"); } diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index 7c9c373ab..178ab24ba 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -106,6 +106,7 @@ impl SlideApp { twisting: false, next_file_id: 1, pending_files: Vec::new(), + host_start: None, ime_tracker: egui::ime_context::ImeContextTracker::new(), ime_pending: None, ime_frame_viewport: None, diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index c1c6198b4..3ec59202e 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -541,13 +541,11 @@ impl SlideApp { /// when the page sent one with a presentation extension and the file is an intact /// presentation, so Save hands it back under that name and format; anything else (no name, an /// outline, a damaged file opened as a repaired copy) is untitled, and its first Save is a new - /// file. An untouched untitled presentation that was the only one open (the blank one the page - /// starts with) gives way to the presentation that opened. + /// file. The first presentation that opens takes the place of the empty one host mode started + /// with ([`Self::start_for_host`]), if that one is still open, untitled and unchanged; later + /// files open beside the others. pub fn open_from_host(&mut self, name: Option<&str>, bytes: &[u8]) { - let blank = match self.session.documents() { - [d] if d.path.is_none() && !d.is_dirty() => Some(d.uid), - _ => None, - }; + let blank = self.host_start.filter(|&uid| self.session.documents().iter().any(|d| d.uid == uid && d.path.is_none() && !d.is_dirty())); let before = self.session.documents().len(); let shown = name.unwrap_or("Presentation"); let named = name.is_some_and(|n| is_presentation_name(&n.to_ascii_lowercase())); @@ -562,8 +560,11 @@ impl SlideApp { self.notify(e.to_string()); } self.drain_requests(); - if self.session.documents().len() > before - && let Some(opened) = self.session.active().map(|d| d.uid) + if self.session.documents().len() <= before { + return; + } + self.host_start = None; + if let Some(opened) = self.session.active().map(|d| d.uid) && let Some(i) = self.session.documents().iter().position(|d| Some(d.uid) == blank) { self.session.close_document(i); @@ -573,6 +574,14 @@ impl SlideApp { } } + /// Host mode (`?host=parent`) starts with an empty presentation instead of the sample, for + /// the first file the page sends to take its place ([`Self::open_from_host`]). + pub fn start_for_host(&mut self) -> Result<(), String> { + self.run("file.new", json!({}))?; + self.host_start = self.session.active().map(|d| d.uid); + Ok(()) + } + /// Act on requests commands left for the UI. pub(super) fn drain_requests(&mut self) { for r in std::mem::take(&mut self.session.ui_requests) { diff --git a/crates/ui-egui/src/backstage/info.rs b/crates/ui-egui/src/backstage/info.rs index 27c0cec6d..3ddbda1a3 100644 --- a/crates/ui-egui/src/backstage/info.rs +++ b/crates/ui-egui/src/backstage/info.rs @@ -182,9 +182,12 @@ pub(super) fn save_as(app: &mut SlideApp, ui: &mut Ui, bs: &mut Backstage, out: { let fmt = bs.file_type; let done = if matches!(fmt, "pptx" | "deckcraft") { - let before = app.session.active().map(|d| (d.path.clone(), d.is_dirty())); + // A save bumps the revision, also one under the same name that leaves the path + // and the dirty flag as they were; a cancelled dialog or a failed save doesn't. + let snapshot = |app: &SlideApp| app.session.active().map(|d| (d.path.clone(), d.is_dirty(), d.revision)); + let before = snapshot(app); app.save_as_dialog(fmt); - web || app.session.active().map(|d| (d.path.clone(), d.is_dirty())) != before + web || snapshot(app) != before } else { app.export_as(fmt, json!({})) }; diff --git a/crates/ui-egui/src/backstage/tests.rs b/crates/ui-egui/src/backstage/tests.rs index 205a5522c..a21cd3cff 100644 --- a/crates/ui-egui/src/backstage/tests.rs +++ b/crates/ui-egui/src/backstage/tests.rs @@ -422,6 +422,35 @@ fn host_close_prompt_keeps_the_presentation_when_the_save_fails() { } } +/// Framed by a page that keeps the files, Save As › Save under the presentation's own name goes +/// back to the slide once the page has the file. The bug this prevents: that save leaves the +/// file and the dirty flag as they were, so the Save As page stayed open and a second click sent +/// the file again. +#[test] +fn host_save_as_under_the_same_name_returns_to_the_slide() { + let got: Rc>> = Default::default(); + let saves = got.clone(); + let services = Services { + host_save: Some(Box::new(move |name: &str, _: &[u8], save_as: bool| { + saves.borrow_mut().push((name.to_string(), save_as)); + Ok(()) + })), + download: Some(Box::new(|_: &str, _: &[u8]| {})), + ..Default::default() + }; + let mut app = SlideApp::new(Session::new(), services); + app.start_for_host().unwrap(); + let deck = deckcraft_engine::cmd::file::save_bytes(&Session::with_new().doc().unwrap().doc, "pptx").unwrap(); + app.open_from_host(Some("deck.pptx"), &deck); + let mut h = harness(app); + h.state_mut().run("app.backstage", json!({"page": "saveAs"})).unwrap(); + h.run_steps(3); + click_on_page(&h, "Save"); + h.run_steps(3); + assert_eq!(*got.borrow(), [("deck.pptx".to_string(), true)]); + assert!(h.state().backstage.is_none(), "the save returns to the slide"); +} + /// Info shows the presentation's counts and properties, and edits them through file.properties. #[test] fn info_shows_and_edits_the_properties() { diff --git a/crates/ui-egui/src/host_save_tests.rs b/crates/ui-egui/src/host_save_tests.rs index 8b9b03c08..78fad53ef 100644 --- a/crates/ui-egui/src/host_save_tests.rs +++ b/crates/ui-egui/src/host_save_tests.rs @@ -29,8 +29,10 @@ fn host_app(fail: bool) -> (SlideApp, Got) { download: Some(Box::new(move |name: &str, bytes: &[u8]| downloads.borrow_mut().push((name.to_string(), false, bytes.to_vec())))), ..Default::default() }; - // The page starts with a blank presentation, as the web shell does in host mode. - (SlideApp::new(Session::with_new(), services), got) + // Host mode starts as the web shell starts it: an empty presentation for the page's file. + let mut app = SlideApp::new(Session::new(), services); + app.start_for_host().unwrap(); + (app, got) } /// A .pptx with `slides` slides, as the page would send it. @@ -138,6 +140,19 @@ fn an_edited_start_presentation_stays_open() { assert_eq!(path(&app).as_deref(), Some("deck.pptx")); } +/// Only the first file from the page takes the start presentation's place. The bug this +/// prevents: a file sent without a name opens untitled and unchanged, as the start presentation +/// is, so the page's next file closed it. +#[test] +fn only_the_first_file_from_the_page_replaces_the_start_presentation() { + let (mut app, _) = host_app(false); + app.open_from_host(None, &pptx(2)); + app.open_from_host(Some("deck.pptx"), &pptx(3)); + let slides: Vec = app.session.documents().iter().map(|d| d.doc.slides.len()).collect(); + assert_eq!(slides, [2, 3], "the file sent without a name stays open beside the next one"); + assert_eq!(path(&app).as_deref(), Some("deck.pptx")); +} + /// The command palette runs `file.save` by id. With a file of its own the engine would write to /// that path, which a browser can't; in host mode it goes to the page like Ctrl+S. #[test] diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 0f0e664f8..08fcc2391 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -335,6 +335,9 @@ pub struct SlideApp { next_file_id: u64, /// Picks and drops whose bytes have not arrived yet. pending_files: Vec, + /// The empty presentation host mode starts with ([`SlideApp::start_for_host`]), until the + /// first file the page sends takes its place. + host_start: Option, /// Owner/generation/cause bookkeeping for the canvas text IME producer. ime_tracker: egui::ime_context::ImeContextTracker, /// Canvas IME geometry for this pass: the caret while editing, or the selected shape. diff --git a/packaging/web/README.md b/packaging/web/README.md index e9622f5aa..1e96279c0 100644 --- a/packaging/web/README.md +++ b/packaging/web/README.md @@ -113,8 +113,9 @@ DeckCraft doesn't learn the name the page finally stores a Save As under, so a p on a conflict should map later Saves of the old name itself. In host mode DeckCraft starts with an empty presentation instead of the sample, and the first -presentation the page sends takes its place. The page guards leaving itself (DeckCraft adds no -guard of its own), and `&author=Name` signs the user's comments. +presentation the page sends takes its place unless the user has changed it; later ones open +beside it. The page guards leaving itself (DeckCraft adds no guard of its own), and +`&author=Name` signs the user's comments. ```html