diff --git a/crates/ui-egui/src/backstage.rs b/crates/ui-egui/src/backstage.rs index 4d794d9078..3044bdc223 100644 --- a/crates/ui-egui/src/backstage.rs +++ b/crates/ui-egui/src/backstage.rs @@ -391,6 +391,19 @@ pub(crate) fn forget_open_error(ctx: &egui::Context) { ctx.data_mut(|d| d.remove::(open_error_id())); } +/// The Save As page's write-failure message ([`WordApp::fail_save_as`]): this page's own +/// render loop skips the status bar, so a failed write (no permission, disk full, a bad path) +/// needs its own place to say why, or the page just sits there with a Browse… button and no clue. +pub(crate) fn save_as_error_id() -> egui::Id { + egui::Id::new("backstage_save_as_error") +} + +/// Drop the Save As page's error once File is closed or a later save succeeds, so it doesn't +/// come back next time. +pub(crate) fn forget_save_as_error(ctx: &egui::Context) { + ctx.data_mut(|d| d.remove::(save_as_error_id())); +} + /// Open a file from the Recent list. A file that can't be opened (deleted, moved, unreadable) /// leaves File open so the list can say why, rather than closing it on a greyed-out window. fn open_recent(app: &mut WordApp, path: &str) -> Result<(), String> { @@ -423,6 +436,11 @@ fn open_page(app: &mut WordApp, ui: &mut Ui) { /// opens it, which is not what Save As is for. fn save_as_page(app: &mut WordApp, ui: &mut Ui) { heading(ui, "Save As"); + let error = ui.ctx().data(|d| d.get_temp::(save_as_error_id())).unwrap_or_default(); + if !error.is_empty() { + ui.label(egui::RichText::new(&error).color(ui.visuals().error_fg_color)); + ui.add_space(6.0); + } if ui.button(egui::RichText::new(tl!("Browse…")).font(medium(14.0))).clicked() { app.save_as_dialog(); } @@ -1047,6 +1065,55 @@ mod tests { let _ = std::fs::remove_dir_all(&dir); } + /// The Save As page's own render loop skips the status bar while Backstage is open, so a + /// write failure (no permission, disk full, a bad path) needs its own place to say why, or + /// the page just sits there with a Browse… button and no clue (CodeRabbit catch on #3434). + #[test] + fn backstage_save_browse_write_failure_shows_on_the_save_as_page() { + use std::sync::mpsc::channel; + let shown: std::rc::Rc)>>> = + Default::default(); + let log = shown.clone(); + let services = Services { + file_dialog: Some(Box::new(move |req| { + let (tx, rx) = channel(); + log.borrow_mut().push((req, tx)); + rx + })), + ..Default::default() + }; + let dir = std::env::temp_dir().join(format!("wordcraft-backstage-save-error-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + // `dir` is deliberately never created, so the write fails: its parent doesn't exist. + let bogus = dir.join("missing").join("out.docx"); + + let mut a = WordApp::new(wordcraft_engine::Session::new(wordcraft_doc::Document::new()), services); + a.run("text.insert", json!({"text": "unsaved text"})).unwrap(); + a.run("ui.backstage", json!({"value": true, "page": "home"})).unwrap(); + let mut h = egui_kittest::Harness::builder().with_size(egui::vec2(1440.0, 900.0)).build_ui_state( + |ui, app: &mut WordApp| { + let ctx = ui.ctx().clone(); + app.logic(&ctx); + app.ui(ui); + }, + a, + ); + h.run_steps(4); + h.get_by_label("Save").click(); + h.run_steps(4); + h.get_by_label("Browse…").click(); + h.run_steps(4); + let (_, tx) = shown.borrow_mut().pop().unwrap(); + tx.send(Ok(vec![bogus.to_string_lossy().to_string()])).unwrap(); + h.run_steps(6); + assert!(h.state().ui.backstage && h.state().ui.backstage_page == "saveAs", "nothing was actually saved"); + let msg = h.state().status_msg.as_ref().map(|(m, _)| m.clone()).expect("the failure set a status message"); + assert!(!msg.is_empty()); + // The same text is rendered on the Save As page itself, not just the (invisible, while + // Backstage is open) status bar. + h.get_by_label(msg.as_str()); + } + #[test] fn backstage_save_closes_when_saved() { let got: std::rc::Rc)>>> = Default::default(); diff --git a/crates/ui-egui/src/file_dialogs.rs b/crates/ui-egui/src/file_dialogs.rs index df6ec95e53..ec2e63671d 100644 --- a/crates/ui-egui/src/file_dialogs.rs +++ b/crates/ui-egui/src/file_dialogs.rs @@ -320,17 +320,19 @@ impl WordApp { // The save's error, kept for the Save Changes prompt to explain (#1445). A plain // Save As (Backstage's Browse…, AutoSave) has no prompt to carry it to, so it was // otherwise dropped: a failed write (no permission, disk full, a bad path) left the - // Save As page sitting there with nothing to say why. Put it on the status bar instead. + // Save As page sitting there with nothing to say why, since its own render loop + // skips the status bar while Backstage is open. `fail_save_as` reaches both. let result = picked.map(|path| self.run("file.save", json!({"path": path}))); let saved = matches!(&result, Some(Ok(v)) if v.get("saved").and_then(Value::as_bool) == Some(true) || v.get("downloaded").is_some()); if saved { self.ui.backstage = false; self.canvas.want_focus = true; + self.clear_save_as_error(); } match after { AfterSave::Nothing => { if let Some(Err(error)) = &result { - self.status(error.clone()); + self.fail_save_as(error); } Ok(json!({"saved": saved})) } @@ -338,7 +340,7 @@ impl WordApp { if saved { self.autosave = true; } else if let Some(Err(error)) = &result { - self.status(error.clone()); + self.fail_save_as(error); } Ok(json!({"saved": saved})) } @@ -361,6 +363,24 @@ impl WordApp { let _ = self.save_as(AfterSave::Nothing); } + /// A plain Save As (Backstage's Browse…, AutoSave) failed to write: say so on the status bar, + /// and, if the Save As page is what's showing, there too — its own render loop skips the + /// status bar while Backstage is open, so that page was otherwise left with nothing to explain + /// a failed write (no permission, disk full, a bad path). + fn fail_save_as(&mut self, error: &str) { + self.status(error.to_string()); + if let Some(ctx) = self.ctx.clone() { + ctx.data_mut(|d| d.insert_temp(crate::backstage::save_as_error_id(), error.to_string())); + } + } + + /// Drop a Save As page error once a later attempt has actually saved. + fn clear_save_as_error(&mut self) { + if let Some(ctx) = self.ctx.clone() { + crate::backstage::forget_save_as_error(&ctx); + } + } + /// Ask where to save, save there, then do `after`. pub(crate) fn save_as(&mut self, after: AfterSave) -> Asked { let name = self.default_save_name(); diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c7f687a4e8..eaab7dd74c 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -2208,9 +2208,10 @@ impl WordApp { if self.session.view.focus_mode && self.dialog.is_none() && ctx.input_mut(|i| i.consume_key(egui::Modifiers::NONE, egui::Key::Escape)) { let _ = self.run("view.focus", json!({"value": false})); } - // A failed Recent open is news only until File closes (#2938). + // A failed Recent open, or a failed Save As, is news only until File closes (#2938). if !self.ui.backstage { backstage::forget_open_error(&ctx); + backstage::forget_save_as_error(&ctx); } if self.ui.backstage { backstage::show(self, ui);