From 2f893566eeac18af360b0161e48b5a9c6f0c8c8f Mon Sep 17 00:00:00 2001 From: Louise Fox <2503492+KaboomFox@users.noreply.github.com> Date: Tue, 20 Oct 2026 01:56:27 +0000 Subject: [PATCH] Export As: a held Enter doesn't confirm the options; exporters get their file Follow-ups to the review of the exporter plug-ins PR (#3171): - Export As opens its format's options once the save panel has picked the file. The Enter that closed the panel, still held, confirmed them at once: Save As marks such a dialog (dialogs::AFTER_PANEL) so it waits for Enter to be let go, Export As didn't. Every options dialog it opens after the panel is marked now, an exporter plug-in's included, through one helper (dialogs::opened_by_panel) that Save As uses too. - io::export with options that aren't a JSON object dropped the path and format it adds for the encoder: an exporter plug-in was told the document's name and its first extension instead of the file it writes. Such options now count as none. - The exporter UI tests remove the plug-ins they install, so no other test's Export As lists them. tests_export.rs reproduces the held Enter for every options dialog (PNG, SVG, Save PDF, Text, DXF, EPS and an exporter's), and tests_plugin_export.rs the options that aren't an object; both failed before the fix. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HhvnnnD5fTFupK8WWq35un --- crates/ui-egui/src/dialogs/export_as.rs | 17 +++++++ crates/ui-egui/src/dialogs/mod.rs | 8 ++++ crates/ui-egui/src/dialogs/tests_export.rs | 46 ++++++++++++++++++- .../src/dialogs/tests_plugin_export.rs | 24 +++++++++- crates/ui-egui/src/io.rs | 7 +-- 5 files changed, 96 insertions(+), 6 deletions(-) diff --git a/crates/ui-egui/src/dialogs/export_as.rs b/crates/ui-egui/src/dialogs/export_as.rs index 8e55a31bd..bf6fea95a 100644 --- a/crates/ui-egui/src/dialogs/export_as.rs +++ b/crates/ui-egui/src/dialogs/export_as.rs @@ -142,6 +142,23 @@ fn confirm(app: &mut VectorcraftApp, d: &Dialog) -> Result { app.ui.dialog = None; let path = io::target_path(app, None, f.extensions[0])?; let range = if all { "all".to_string() } else { range }; + let r = options_or_write(app, f, plugin, path, use_artboards, all, range); + // The options follow the save panel: the Enter that closed it isn't theirs (as in Save As). + super::opened_by_panel(app); + r +} + +/// After the save panel picked `path`: the options dialog of format `f` (or of its exporter +/// plug-in), else the file written. +fn options_or_write( + app: &mut VectorcraftApp, + f: &'static Format, + plugin: Option>, + path: String, + use_artboards: bool, + all: bool, + range: String, +) -> Result { match f.id { "svg" | "svgz" => { svg_options::open(app, svg_options::Mode::Export, Some(&path)); diff --git a/crates/ui-egui/src/dialogs/mod.rs b/crates/ui-egui/src/dialogs/mod.rs index 8082977cb..f02f75f48 100644 --- a/crates/ui-egui/src/dialogs/mod.rs +++ b/crates/ui-egui/src/dialogs/mod.rs @@ -426,6 +426,14 @@ pub(crate) fn settle(app: &mut VectorcraftApp) { /// until Enter has been let go. pub(crate) const AFTER_PANEL: &str = "__afterPanel"; +/// Marks the open dialog, if any, as one a save panel opened ([`AFTER_PANEL`]): the Enter that +/// closed the panel, still held, doesn't confirm it. +pub(crate) fn opened_by_panel(app: &mut VectorcraftApp) { + if let Some(d) = app.ui.dialog.as_mut() { + d.fields.insert(AFTER_PANEL.into(), json!(true)); + } +} + /// Draws the active dialog and handles shared initial focus, keyboard shortcuts and actions. /// Inline actions and footer buttons use the same confirmation and preview rollback paths. pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { diff --git a/crates/ui-egui/src/dialogs/tests_export.rs b/crates/ui-egui/src/dialogs/tests_export.rs index 90780ad32..80227e776 100644 --- a/crates/ui-egui/src/dialogs/tests_export.rs +++ b/crates/ui-egui/src/dialogs/tests_export.rs @@ -16,10 +16,15 @@ pub(super) type Written = Rc)>>>; /// An app with `artboards` 60×40 pt artboards whose save dialog answers `/out/` and /// whose writer records what it is given. pub(super) fn app(artboards: usize) -> (VectorcraftApp, Written) { + app_saving(artboards, |p| format!("/out/{}", p.name)) +} + +/// [`app`], with a save dialog that answers `pick`. +pub(super) fn app_saving(artboards: usize, pick: fn(&crate::FilePick) -> String) -> (VectorcraftApp, Written) { let written = Written::default(); let w = written.clone(); let services = Services { - pick_save: Some(Box::new(|p: &crate::FilePick| Some(format!("/out/{}", p.name)))), + pick_save: Some(Box::new(move |p: &crate::FilePick| Some(pick(p)))), write: Some(Box::new(move |p: &str, b: &[u8]| { w.borrow_mut().push((p.to_string(), b.to_vec())); Ok(()) @@ -266,3 +271,42 @@ fn export_as_writes_only_the_chosen_artboard_in_every_format() { } } } + +/// The Enter that closed the save panel, still held as the format's options open, doesn't write +/// the file; Enter pressed again after it is let go does. So for every options dialog Export As +/// opens after its panel, an exporter plug-in's included (as Save As's options). +#[test] +fn the_enter_that_closed_the_save_panel_does_not_confirm_the_options() { + let params = r#"{"speed":{"type":"number","min":1,"max":100,"default":40}}"#; + let module = wat::parse_str(vectorcraft_plugins::wat::returning( + &vectorcraft_plugins::wat::exporter("test.ui.held", r#"["tuh"]"#, params), + r#"{"dataBase64":"SGk="}"#, + )) + .unwrap(); + let enter = |pressed| egui::Event::Key { key: egui::Key::Enter, physical_key: None, pressed, repeat: false, modifiers: Default::default() }; + let ctx = egui::Context::default(); + theme::install_fonts(&ctx); + let frame = |app: &mut VectorcraftApp, events: Vec| { + let mut out = ctx.run_ui(egui::RawInput { events, ..Default::default() }, |ui| show(app, ui.ctx())); + out.textures_delta.clear(); + }; + // Each format whose options follow the panel (PDF's with Use Artboards: Save PDF). + let formats = [("png", false), ("svg", false), ("pdf", true), ("txt", false), ("dxf", false), ("eps", false), ("plugin.test.ui.held", false)]; + for (format, artboards) in formats { + let (mut app, written) = app(1); + app.run("plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&module)})).unwrap(); + app.run("file.exportAs", json!({"format": format})).unwrap(); + set(&mut app, "useArtboards", json!(artboards)); + confirm(&mut app).unwrap(); + let options = kind(&app).map(str::to_string); + assert!(options.as_deref().is_some_and(|k| k != "exportAs"), "{format}: its options follow the panel, not {options:?}"); + frame(&mut app, vec![enter(true)]); + assert_eq!(kind(&app).map(str::to_string), options, "{format}: the Enter still held isn't the options'"); + assert!(written.borrow().is_empty(), "{format}: nothing written"); + frame(&mut app, vec![enter(false)]); + frame(&mut app, vec![enter(true)]); + assert!(app.ui.dialog.is_none(), "{format}: Enter pressed again confirms"); + assert!(!written.borrow().is_empty(), "{format}: and writes"); + } + vectorcraft_plugins::registry::remove("test.ui.held"); +} diff --git a/crates/ui-egui/src/dialogs/tests_plugin_export.rs b/crates/ui-egui/src/dialogs/tests_plugin_export.rs index 85156326f..0005a3034 100644 --- a/crates/ui-egui/src/dialogs/tests_plugin_export.rs +++ b/crates/ui-egui/src/dialogs/tests_plugin_export.rs @@ -1,11 +1,12 @@ //! Exporter plug-ins in File › Export › Export As: listed after the built-in formats, written //! whole (Use Artboards is not theirs to apply), and their parameters asked for once the file is -//! picked. Each test installs exporters of its own: plug-ins are process-wide. +//! picked. Each test installs exporters of its own and removes them: plug-ins are process-wide, and +//! a format left installed would show in other tests' Export As. use serde_json::{Value, json}; use vectorcraft_plugins::wat::{self as tpl, Echo}; -use super::tests_export::{app, frame, kind, names, set}; +use super::tests_export::{app, app_saving, frame, kind, names, set}; use super::*; fn install(app: &mut VectorcraftApp, src: &str) { @@ -13,6 +14,8 @@ fn install(app: &mut VectorcraftApp, src: &str) { app.run("plugin.install", json!({"dataBase64": vectorcraft_format::base64_encode(&wasm)})).unwrap(); } +/// Export As lists an exporter after the built-in formats and writes its file of the whole +/// document (Use Artboards greyed out), with the exporter's warnings in the status bar. #[test] fn export_as_lists_exporters_and_writes_their_file_whole() { let (mut app, written) = app(2); @@ -37,8 +40,10 @@ fn export_as_lists_exporters_and_writes_their_file_whole() { assert_eq!(names(&written), ["/out/Untitled-1.tuw"]); assert_eq!(written.borrow()[0].1, b"Hi"); assert!(app.ui.status.contains("a note"), "{}", app.ui.status); + app.run("plugin.remove", json!({"id": "test.ui.whole"})).unwrap(); } +/// An exporter with parameters asks for them once the file is picked, and writes with them. #[test] fn an_exporter_with_parameters_asks_for_them_once_the_file_is_picked() { let (mut app, written) = app(1); @@ -59,6 +64,7 @@ fn an_exporter_with_parameters_asks_for_them_once_the_file_is_picked() { assert_eq!(names(&written), ["/out/Untitled-1.tup"]); let sent: Value = serde_json::from_slice(&written.borrow()[0].1).unwrap(); assert_eq!(sent, json!({"density": 7.0, "_context": {"mode": "export", "extension": "tup", "name": "Untitled-1"}})); + app.run("plugin.remove", json!({"id": "test.ui.params"})).unwrap(); } /// `CRAFT_UI_PLUGIN_EXPORT_SHOT=` saves the pictures of the dialogs to that folder: Export As @@ -113,4 +119,18 @@ fn a_declining_exporter_writes_nothing() { assert!(written.borrow().is_empty()); let said = r.err().unwrap_or_default() + &app.ui.status; assert!(said.contains("Nothing to plot."), "{said}"); + app.run("plugin.remove", json!({"id": "test.ui.declines"})).unwrap(); +} + +/// Export options that aren't an object are none: the exporter is still told the file it writes, +/// by its name and extension (here the second it lists), not the document's name and its first. +#[test] +fn an_exporter_is_told_its_file_whatever_the_options_are() { + let (mut app, written) = app_saving(1, |_| "/out/plotter-job.tnb".into()); + install(&mut app, &tpl::echo_exporter(&tpl::exporter("test.ui.nulloptions", r#"["tna","tnb"]"#, "{}"), Echo::Params)); + crate::io::export(&mut app, Some("plugin.test.ui.nulloptions"), None, &Value::Null).unwrap(); + assert_eq!(names(&written), ["/out/plotter-job.tnb"]); + let sent: Value = serde_json::from_slice(&written.borrow()[0].1).unwrap(); + assert_eq!(sent, json!({"_context": {"mode": "export", "extension": "tnb", "name": "plotter-job"}})); + app.run("plugin.remove", json!({"id": "test.ui.nulloptions"})).unwrap(); } diff --git a/crates/ui-egui/src/io.rs b/crates/ui-egui/src/io.rs index 972b214ad..ae01b0305 100644 --- a/crates/ui-egui/src/io.rs +++ b/crates/ui-egui/src/io.rs @@ -487,9 +487,7 @@ pub fn save(app: &mut VectorcraftApp, mode: SaveMode, p: &Value, ask_options: bo if ask && asks_options(mode, chosen.format) { let path = chosen.path.clone().unwrap_or_default(); let r = ask_format_options(app, mode, chosen.format, &path); - if let Some(d) = app.ui.dialog.as_mut() { - d.fields.insert(crate::dialogs::AFTER_PANEL.into(), json!(true)); - } + crate::dialogs::opened_by_panel(app); return r; } write_plan(app, chosen) @@ -712,6 +710,9 @@ pub fn hide_exports(app: &mut VectorcraftApp) -> Value { /// document keeps its path. → `{path, warnings, files?}` (`files`: every file when there are /// several). pub fn export(app: &mut VectorcraftApp, format: Option<&str>, path: Option, params: &Value) -> Result { + // Options given as anything but an object are none: the path and format still reach the + // encoder (an exporter plug-in is told the file it writes). + let params = &if params.is_object() { params.clone() } else { json!({}) }; let f = fileio::writable_format(format, path.as_deref())?; let st = app.session.active().ok_or("no document")?; let doc = match fileio::export_source(st, params).map_err(|e| e.to_string())? {