mirror of
https://github.com/storytold/wordcraft.git
synced 2026-11-14 06:22:41 +00:00
Show a failed Save As on the Save As page itself
CodeRabbit caught it on PR #3434: self.status(...) alone is invisible while Backstage is open, since its render loop skips the status bar (only backstage::show runs). A failed Save As (no permission, disk full, a bad path) left the Save As page sitting there with a Browse… button and no explanation, same silent failure as before, just moved. fail_save_as now also stores the message in the egui temp data save_as_page reads and renders inline, cleared once File closes or a later attempt actually saves. Replaced the status_msg-only regression test with one that drives the real UI (Save > Save As page > Browse… > a write that fails) and asserts the same text is rendered on the page, not just sitting in status_msg. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This commit is contained in:
1 parent
85c72c2862
commit
c329a3be0e
3 files changed
+92
-4
No files matched your search
@@ -391,6 +391,19 @@ pub(crate) fn forget_open_error(ctx: &egui::Context) {
|
||||
ctx.data_mut(|d| d.remove::<String>(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::<String>(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::<String>(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<std::cell::RefCell<Vec<(crate::FileDialogRequest, std::sync::mpsc::Sender<crate::FileDialogAnswer>)>>> =
|
||||
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<std::cell::RefCell<Vec<(String, Vec<u8>)>>> = Default::default();
|
||||
|
||||
@@ -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();
|
||||
|
||||
@@ -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);
|
||||
|
||||
Reference in new issue
Block a user