mirror of
https://github.com/storytold/figcraft.git
synced 2026-10-26 19:36:46 -06:00
M1: Save, Save as and Export arrive as downloads in the browser (#969)
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.
This commit is contained in:
@@ -51,7 +51,7 @@ pub fn specs() -> Vec<CommandSpec> {
|
||||
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<Value> {
|
||||
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<Value> {
|
||||
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<String>) -> Result<Value> {
|
||||
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<Value> {
|
||||
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<Value> {
|
||||
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<Value> {
|
||||
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()));
|
||||
|
||||
@@ -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}"
|
||||
|
||||
@@ -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)"
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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() {
|
||||
|
||||
@@ -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);
|
||||
|
||||
@@ -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())]));
|
||||
|
||||
@@ -9,12 +9,12 @@ use serde_json::{Value, json};
|
||||
use crate::variables_panel::export_dialog;
|
||||
use crate::{FigApp, NoServices, Services};
|
||||
|
||||
type Downloads = Arc<Mutex<Vec<(String, Vec<u8>)>>>;
|
||||
pub(crate) type Downloads = Arc<Mutex<Vec<(String, Vec<u8>)>>>;
|
||||
|
||||
/// 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 {
|
||||
|
||||
@@ -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");
|
||||
}
|
||||
Reference in New Issue
Block a user