From 5e2957a9011d71ff8fc3469830eb28ca17a2d3ad Mon Sep 17 00:00:00 2001 From: Ridzzz0Alam Date: Wed, 21 Oct 2026 19:15:58 +0200 Subject: [PATCH] M1: Save, Save as and Export arrive as downloads in the browser (#969) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cause: in the web build no file has a path, so Save goes to Save as, and Save as and every Export go through Services::pick_save / pick_folder. WebServices keeps the trait's defaults there, which return no path, so the callback never ran: no download, no message. Only Variables ▸ Export used the platform's downloads (#368). Fix: where services.downloads() is true, those routes run their command without a path and hand the bytes to services.download(): - engine: file.saveAs takes `data: true` and returns the .figcraft file as base64 `data` with its `name`, marking it saved (no path, nothing remembered). Programmatic Save as without either still asks for a path. - ui: FigApp::download decodes a result's `data` and says when nothing arrived; Save as, one-off export (PNG/JPG/WebP/SVG/PDF), .fig export, frames to PDF and multi-layer export (one download per file) use it. Tests: engine save_as_data_hands_back_the_file_and_marks_it_saved; ui-egui web_save_tests (save, export, refused download) with the fake browser Services from variables_export_ui_tests. The UI tests fail without the fix. --- crates/engine/src/cmd/file.rs | 58 +++++++++++++++---- crates/ui-egui/locales/en.toml | 1 + crates/ui-egui/locales/es.toml | 1 + crates/ui-egui/src/dialogs.rs | 15 ++++- crates/ui-egui/src/export_panel.rs | 17 ++++++ crates/ui-egui/src/lib.rs | 22 +++++++ crates/ui-egui/src/menus.rs | 9 +++ .../ui-egui/src/variables_export_ui_tests.rs | 8 +-- crates/ui-egui/src/web_save_tests.rs | 55 ++++++++++++++++++ 9 files changed, 169 insertions(+), 17 deletions(-) create mode 100644 crates/ui-egui/src/web_save_tests.rs diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 7380711f..8a75efb7 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -51,7 +51,7 @@ pub fn specs() -> Vec { spec!( "file.saveAs", "Save as", - "{path} (no extension adds .figcraft); a .fig path writes an experimental export copy of the shown document with `warnings`, keeping the native path, unsaved changes and version preview", + "{path | data: true (no path: returns the file as base64 `data` with its `name`, for a download)} (no extension adds .figcraft); a .fig path writes an experimental export copy of the shown document with `warnings`, keeping the native path, unsaved changes and version preview", false, save_as ), @@ -843,37 +843,53 @@ fn color_profile(s: &mut Session, p: &Value) -> Result { pub(crate) fn save_ends_preview(s: &Session, cmd: &str, p: &Value) -> bool { let path = string(p, "path").or_else(|| if cmd == "file.save" { s.docs.get(crate::branches::file_tab(s)).and_then(|ds| ds.path.clone()) } else { None }); - path.is_some_and(|path| ext(&path) != "fig") + path.is_some_and(|path| ext(&path) != "fig") || (cmd == "file.saveAs" && wants_data(p)) +} + +/// `file.saveAs` asked for the file's bytes instead of a path to write (the browser's download). +fn wants_data(p: &Value) -> bool { + p.get("data").and_then(Value::as_bool) == Some(true) } /// Save the file of the active tab (the main file for a branch tab, with its branches), or export /// the active document as an experimental `.fig` copy without changing its native save state. -fn save_to(s: &mut Session, path: String) -> Result { - if ext(&path) == "fig" { +/// Without a path the file comes back as base64 `data` for the caller to deliver (#969). +fn save_to(s: &mut Session, path: Option) -> Result { + if let Some(path) = path.as_deref().filter(|p| ext(p) == "fig") { return export_fig(s, &json!({"path": path})); } let i = crate::branches::file_tab(s); let ds = s.docs.get_mut(i).ok_or(EngineError::NoDocument)?; let mut snapshot = FileSnapshot::of(ds); let follows_path = ds.path.as_deref().map_or(ds.doc.name == "Untitled", |old| ds.doc.name == file_stem(old)); - if follows_path && ds.path.as_deref() != Some(path.as_str()) { - Arc::make_mut(&mut snapshot.doc).name = file_stem(&path); + if let Some(path) = path.as_deref() + && follows_path + && ds.path.as_deref() != Some(path) + { + Arc::make_mut(&mut snapshot.doc).name = file_stem(path); } let bytes = snapshot.encode()?; - write(&path, &bytes)?; + if let Some(path) = &path { + write(path, &bytes)?; + } ds.doc = snapshot.doc; - ds.path = Some(path.clone()); + if path.is_some() { + ds.path.clone_from(&path); + } ds.mark_saved(); ds.revision += 1; - let uid = ds.uid; + let (uid, name) = (ds.uid, format!("{}.figcraft", ds.doc.name)); branch_tabs_saved(s, uid); + let Some(path) = path else { + return Ok(json!({"name": name, "bytes": bytes.len(), "data": base64::engine::general_purpose::STANDARD.encode(&bytes)})); + }; remember(s, &path); Ok(json!({"path": path, "bytes": bytes.len()})) } fn save(s: &mut Session, p: &Value) -> Result { if let Some(path) = string(p, "path") { - return save_to(s, path); + return save_to(s, Some(path)); } match s.docs.get(crate::branches::file_tab(s)).ok_or(EngineError::NoDocument)?.path.clone() { // Earlier builds wrote native data under a .fig name: never overwrite that file with an export. @@ -881,7 +897,7 @@ fn save(s: &mut Session, p: &Value) -> Result { s.request_ui("dialog.saveAs", json!({})); Err(EngineError::Disabled("file.save".into(), "the saved path ends in .fig: choose Save as or pass `path`".into())) } - Some(path) => save_to(s, path), + Some(path) => save_to(s, Some(path)), None => { s.request_ui("dialog.saveAs", json!({})); Err(EngineError::Disabled("file.save".into(), "the file has no path yet: pass `path`".into())) @@ -895,8 +911,9 @@ fn save_as(s: &mut Session, p: &Value) -> Result { if ext(&path).is_empty() { path.push_str(".figcraft"); } - save_to(s, path) + save_to(s, Some(path)) } + None if wants_data(p) => save_to(s, None), None => { s.request_ui("dialog.saveAs", json!({})); Err(EngineError::Disabled("file.saveAs".into(), "pass `path`".into())) @@ -1563,6 +1580,23 @@ mod rest_import_tests { assert_eq!(s.docs.len(), open, "a refused open adds nothing"); } + #[test] + fn save_as_data_hands_back_the_file_and_marks_it_saved() { + // The browser has no paths: Save as returns the file for a download (#969). + let mut s = Session::new(); + s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + assert!(s.execute("file.saveAs", &json!({})).is_err(), "no path and no data: still asks for a path"); + let recent = s.prefs.recent_files.clone(); + let r = s.execute("file.saveAs", &json!({"data": true})).unwrap(); + assert_eq!(r["name"], "Untitled.figcraft"); + assert!(!s.ds().unwrap().is_dirty()); + assert_eq!(s.ds().unwrap().path, None, "still no path to save over"); + assert_eq!(s.prefs.recent_files, recent, "nothing on disk to remember"); + let mut t = Session::new(); + t.execute("file.open", &json!({"data": r["data"], "name": r["name"]})).unwrap(); + assert!(t.doc().unwrap().nodes().any(|n| matches!(n.kind, NodeKind::Rectangle)), "the bytes reopen"); + } + #[test] fn open_and_import_rest_json() { let dir = std::env::temp_dir().join(format!("figcraft-rest-{}", std::process::id())); diff --git a/crates/ui-egui/locales/en.toml b/crates/ui-egui/locales/en.toml index ae566fc8..3ce0659d 100644 --- a/crates/ui-egui/locales/en.toml +++ b/crates/ui-egui/locales/en.toml @@ -319,6 +319,7 @@ name = "English" "Corner radius" = "Corner radius" "Corner smoothing" = "Corner smoothing" "Couldn't download the variables" = "Couldn't download the variables" +"Couldn't download {name}" = "Couldn't download {name}" "Couldn't reach {server}: showing what this computer remembers." = "Couldn't reach {server}: showing what this computer remembers." "Couldn't read {name}." = "Couldn't read {name}." "Couldn't save automatically: {error}" = "Couldn't save automatically: {error}" diff --git a/crates/ui-egui/locales/es.toml b/crates/ui-egui/locales/es.toml index 596feda6..bd572dbe 100644 --- a/crates/ui-egui/locales/es.toml +++ b/crates/ui-egui/locales/es.toml @@ -10,6 +10,7 @@ status = "draft — needs native review" [ui] "Compatibility notes" = "Notas de compatibilidad" "Couldn't download the variables" = "No se pudieron descargar las variables" +"Couldn't download {name}" = "No se pudo descargar {name}" "% of all commits" = "% del total de commits" "(no longer published)" = "(ya no publicado)" diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index bf9f1b29..9695faee 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -665,7 +665,11 @@ pub(crate) fn file_saved(app: &mut FigApp, result: &Value) { } fn result_file_name(result: &Value) -> String { - result["path"].as_str().and_then(|p| Path::new(p).file_name()).map(|f| f.to_string_lossy().into_owned()).unwrap_or_default() + match result["path"].as_str() { + Some(p) => Path::new(p).file_name().map(|f| f.to_string_lossy().into_owned()).unwrap_or_default(), + // A download (#969). + None => result["name"].as_str().unwrap_or_default().to_owned(), + } } /// The `.fig` result stays an export: native save state is unchanged, and omissions are reported. @@ -681,6 +685,15 @@ pub(crate) fn fig_exported(app: &mut FigApp, result: &Value) { /// Save the whole file as `.fig`; the toast says how much was left out. fn save_fig(app: &mut FigApp, name: &str) { let filename = format!("{name}.fig"); + if app.services.downloads() { + if let Some(mut r) = app.run("file.export", json!({"format": "fig"})) + && app.download(&filename, &r) + { + r["name"] = json!(filename); + fig_exported(app, &r); + } + return; + } app.pick_save(tr("Export"), &filename, &["fig"], |app, p| { if let Some(r) = app.run("file.export", json!({"format": "fig", "path": p.to_string_lossy()})) { fig_exported(app, &r); diff --git a/crates/ui-egui/src/export_panel.rs b/crates/ui-egui/src/export_panel.rs index 8617c35a..6a4ca8ee 100644 --- a/crates/ui-egui/src/export_panel.rs +++ b/crates/ui-egui/src/export_panel.rs @@ -320,6 +320,15 @@ pub(crate) fn export_layers(app: &mut FigApp, ids: &[NodeId], assets: bool) { [] => {} // The suggested name keeps only the file's own name (a layer's folders apply to folders). [one] => save_one(app, &[one.id], &one.setting, one.file.rsplit('/').next().unwrap_or(&one.file)), + // No folders in the browser: each file arrives as its own download (#969). + _ if app.services.downloads() => { + let Some(r) = app.run("file.exportLayers", json!({"ids": ids, "assets": assets})) else { return }; + let files = r["files"].as_array().map_or(&[][..], Vec::as_slice); + let n = files.iter().filter(|f| app.download(f["name"].as_str().unwrap_or_default(), f)).count(); + if n > 0 { + app.toast(trn("Exported {n} files", n, &[])); + } + } _ => { let ids = ids.to_vec(); app.pick_folder(tr("Export"), move |app, dir| { @@ -337,6 +346,14 @@ pub(crate) fn save_one(app: &mut FigApp, ids: &[NodeId], setting: &ExportSetting let ext = setting.format.ext(); let mut params = setting_params(setting); params["ids"] = json!(ids); + if app.services.downloads() { + if let Some(r) = app.run("file.export", params) + && app.download(file, &r) + { + app.toast(trf("Exported {name}", &[("name", &file)])); + } + return; + } app.pick_save(tr("Export"), file, &[ext], move |app, p| { params["path"] = json!(p.to_string_lossy()); if app.run("file.export", params).is_some() { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c93333b3..788bbf4f 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -223,6 +223,8 @@ mod vector_tail_ui_tests; #[cfg(test)] mod web_clipboard_tests; #[cfg(test)] +mod web_save_tests; +#[cfg(test)] mod windows_qa_tests; #[cfg(test)] mod windows_tests; @@ -1140,6 +1142,14 @@ impl FigApp { } pub fn save_as_dialog(&mut self) { + if self.services.downloads() { + if let Some(r) = self.run("file.saveAs", json!({"data": true})) + && self.download(r["name"].as_str().unwrap_or_default(), &r) + { + dialogs::file_saved(self, &r); + } + return; + } let Ok(name) = self.session.doc().map(|d| d.name.clone()) else { return }; self.pick_save(i18n::tr("Save as"), &format!("{name}.figcraft"), &["figcraft", "fig"], |app, mut p| { if p.extension().is_none_or(|e| e.is_empty()) { @@ -1244,6 +1254,18 @@ impl FigApp { self.start_dialog(|services| services.pick_save(title, name, exts), true, done); } + /// Hand a command result's base64 `data` over as a download named `name`, where the platform + /// saves by downloads rather than save dialogs (the browser, #969). Says so when nothing arrived. + pub(crate) fn download(&mut self, name: &str, result: &Value) -> bool { + use base64::Engine as _; + let bytes = result["data"].as_str().and_then(|d| base64::engine::general_purpose::STANDARD.decode(d).ok()); + if bytes.is_some_and(|b| self.services.download(name, &b)) { + return true; + } + self.toast(i18n::trf("Couldn't download {name}", &[("name", &name)])); + false + } + /// Pick a folder; `done` runs as [`FigApp::pick_open`]'s does. pub fn pick_folder(&mut self, title: &str, done: impl FnOnce(&mut FigApp, PathBuf) + 'static) { self.start_dialog(|services| services.pick_folder(title), true, done); diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index 9659c6b3..afa34873 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -248,6 +248,15 @@ pub fn invoke(app: &mut FigApp, id: &str) { /// Every visible top-level frame of the page as one PDF page each, through a save dialog. fn export_frames_pdf(app: &mut FigApp) { let name = app.session.doc().map(|d| d.name.clone()).unwrap_or_else(|_| "Untitled".into()); + if app.services.downloads() { + let file = format!("{name}.pdf"); + if let Some(r) = app.run("file.exportFramesPdf", json!({})) + && app.download(&file, &r) + { + app.toast(trf("Exported {name}", &[("name", &file)])); + } + return; + } app.pick_save(tr("Export"), &format!("{name}.pdf"), &["pdf"], |app, p| { if app.run("file.exportFramesPdf", json!({"path": p.to_string_lossy()})).is_some() { app.toast(trf("Exported {name}", &[("name", &p.file_name().map(|f| f.to_string_lossy().to_string()).unwrap_or_default())])); diff --git a/crates/ui-egui/src/variables_export_ui_tests.rs b/crates/ui-egui/src/variables_export_ui_tests.rs index 40a84393..ce51a096 100644 --- a/crates/ui-egui/src/variables_export_ui_tests.rs +++ b/crates/ui-egui/src/variables_export_ui_tests.rs @@ -9,12 +9,12 @@ use serde_json::{Value, json}; use crate::variables_panel::export_dialog; use crate::{FigApp, NoServices, Services}; -type Downloads = Arc)>>>; +pub(crate) type Downloads = Arc)>>>; /// Services that save by downloads and keep them (`deliver` false: the browser refused). -struct Browser { - got: Downloads, - deliver: bool, +pub(crate) struct Browser { + pub(crate) got: Downloads, + pub(crate) deliver: bool, } impl Services for Browser { diff --git a/crates/ui-egui/src/web_save_tests.rs b/crates/ui-egui/src/web_save_tests.rs new file mode 100644 index 00000000..7d1d75d2 --- /dev/null +++ b/crates/ui-egui/src/web_save_tests.rs @@ -0,0 +1,55 @@ +//! Save, Save as and Export where the platform saves by downloads (the browser, which has no save +//! dialogs or paths): the file arrives as a download instead of nothing happening (#969). + +use base64::Engine as _; +use figcraft_doc::{ExportSetting, NodeId}; +use figcraft_engine::Session; +use serde_json::json; + +use crate::FigApp; +use crate::variables_export_ui_tests::{Browser, Downloads}; + +fn browser(deliver: bool) -> (FigApp, Downloads, NodeId) { + let mut s = Session::new(); + let r = s.execute("shape.rectangle", &json!({"x": 0, "y": 0, "width": 10, "height": 10})).unwrap(); + let id = serde_json::from_value(r["id"].clone()).unwrap(); + let got = Downloads::default(); + (FigApp::new(s, Box::new(Browser { got: got.clone(), deliver })), got, id) +} + +fn toast(app: &FigApp) -> String { + app.ui.toast.as_ref().map(|t| t.0.clone()).unwrap_or_default() +} + +#[test] +fn save_downloads_the_figcraft_file() { + let (mut app, got, _) = browser(true); + app.save(); + let got = got.lock().unwrap(); + let [(name, bytes)] = got.as_slice() else { panic!("one download, got {}", got.len()) }; + assert_eq!(name, "Untitled.figcraft"); + let mut reopened = Session::new(); + let data = base64::engine::general_purpose::STANDARD.encode(bytes); + reopened.execute("file.open", &json!({"data": data, "name": name})).unwrap(); + assert!(!app.session.ds().unwrap().is_dirty()); + assert_eq!(toast(&app), "Saved Untitled.figcraft"); +} + +#[test] +fn export_downloads_the_exported_file() { + let (mut app, got, id) = browser(true); + crate::export_panel::save_one(&mut app, &[id], &ExportSetting::default(), "Rectangle.png"); + let got = got.lock().unwrap(); + let [(name, bytes)] = got.as_slice() else { panic!("one download, got {}", got.len()) }; + assert_eq!(name, "Rectangle.png"); + assert!(bytes.starts_with(b"\x89PNG")); + assert_eq!(toast(&app), "Exported Rectangle.png"); +} + +#[test] +fn a_refused_download_says_so() { + let (mut app, got, _) = browser(false); + app.save_as_dialog(); + assert_eq!(got.lock().unwrap().len(), 1); + assert_eq!(toast(&app), "Couldn't download Untitled.figcraft"); +}