From 65ef8a1c705d288ee06e7b366bd2bf1c3d698d8e Mon Sep 17 00:00:00 2001 From: moppeeer <339764117+moppeeer@users.noreply.github.com> Date: Sat, 10 Oct 2026 16:37:21 +0200 Subject: [PATCH] fix(ui): Keep Enter and Escape in the Save Document Preset popup; saved presets in the engine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up. Enter in the Save Document Preset popup created the document and Escape closed the whole dialog: the dialog footer took both keys while a popup was open. The dialog now leaves Enter and Escape to an open popup (read before it draws, as egui closes a popup on Escape that frame): Enter in the preset name saves the preset, Escape closes only the popup. Clicking the name field no longer closes the popup either (menus close on any click; this one only outside), and the field takes focus when the popup opens. Saved presets lived only in the UI state, so `file.new {preset: name}` refused them. They are now engine preferences (saved with prefs.json): `file.savePreset {name, …the file.new settings}` replaces a preset of that name and refuses built-in names, `file.deletePreset {name}` removes one, `file.presets` lists them after the built-in ones (`saved: true`), and `file.new {preset}` makes documents from them by name. The Saved tab reads and writes them through those commands. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/file.rs | 120 ++++++++++++++++++++++++++--- crates/engine/src/lib.rs | 3 + crates/ui-egui/src/dialogs.rs | 7 +- crates/ui-egui/src/i18n.rs | 1 + crates/ui-egui/src/i18n/it.rs | 1 + crates/ui-egui/src/lib.rs | 3 - crates/ui-egui/src/new_document.rs | 105 ++++++++++++++++++++----- 7 files changed, 204 insertions(+), 36 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 612fc4863..47ce669dc 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -13,10 +13,31 @@ use crate::{DocState, EngineError, Result, Session}; pub fn specs() -> Vec { vec![ cmd!(noundo "file.new", "Document…", ["File", "New"], Some("Cmd+N"), - "{preset?: \"Letter\"|\"A4\"|…, intent?: print|web|mobile, units?: points|picas|millimeters|…, width?, height?, pages?, startPage?, facingPages?, columns?, gutter?, margins?: number|{top,bottom,inside,outside}, bleed?, slug?: n | [top, bottom, inside, outside], title?}", + "{preset?: \"Letter\"|\"A4\"|… or a saved preset (file.savePreset), intent?: print|web|mobile, units?: points|picas|millimeters|…, width?, height?, pages?, startPage?, facingPages?, columns?, gutter?, margins?: number|{top,bottom,inside,outside}, bleed?, slug?: n | [top, bottom, inside, outside], title?}", always, file_new), cmd!(noundo "file.newSample", "Sample Document", ["Help"], None, "{} — a multi-page magazine sample", always, file_sample), - cmd!(query "file.presets", "Document Presets", [], None, "{}", always, |_, _| Ok(serde_json::to_value(PRESETS).unwrap_or_default())), + cmd!(query "file.presets", "Document Presets", [], None, "{} → the built-in presets [{name, intent, width, height, units}], then the saved ones (with `saved: true`)", always, |s, _| { + let mut presets: Vec = PRESETS.iter().map(|p| serde_json::to_value(p).unwrap_or_default()).collect(); + presets.extend( + s.prefs + .document_presets + .iter() + .map(|p| json!({"name": p.title, "intent": p.intent, "width": p.width, "height": p.height, "units": p.units, "saved": true})), + ); + Ok(Value::Array(presets)) + }), + cmd!(noundo "file.savePreset", "Save Document Preset", [], None, + "{name, …the settings file.new takes} — kept with the preferences; `file.new {preset: name}` uses it. A saved preset of that name is replaced; built-in names are refused", + always, save_preset), + cmd!(noundo "file.deletePreset", "Delete Document Preset", [], None, "{name} — a saved document preset", always, |s, p| { + let name = str_param(p, "name").unwrap_or_default().trim().to_string(); + let before = s.prefs.document_presets.len(); + s.prefs.document_presets.retain(|x| !x.title.eq_ignore_ascii_case(&name)); + if s.prefs.document_presets.len() == before { + return Err(bad("file.deletePreset", format!("no saved preset `{name}`"))); + } + ok() + }), cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), "{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped (and, for .idml, package parts missing)}", always, file_open), @@ -115,11 +136,59 @@ pub fn from_bytes(b: &[u8]) -> Result { designcraft_format::load(b).map_err(|e| EngineError::Other(e.to_string())) } +/// The longest saved preset name, and how many presets are kept. +const MAX_PRESET_NAME: usize = 100; +const MAX_PRESETS: usize = 500; + +/// A built-in document preset or a saved one, by name (case ignored). +fn find_preset(s: &Session, name: &str) -> Option { + NewDocument::from_preset(name).or_else(|| s.prefs.document_presets.iter().find(|p| p.title.eq_ignore_ascii_case(name.trim())).cloned()) +} + fn file_new(s: &mut Session, p: &Value) -> Result { let mut nd = match str_param(p, "preset") { - Some(name) => NewDocument::from_preset(name).ok_or_else(|| bad("file.new", format!("unknown preset `{name}`")))?, + Some(name) => find_preset(s, name).ok_or_else(|| bad("file.new", format!("unknown preset `{name}`")))?, None => NewDocument::default(), }; + apply_settings(&mut nd, p); + check_size("file.new", &nd)?; + s.untitled += 1; + nd.title = str_param(p, "title").map(str::to_string).unwrap_or_else(|| format!("Untitled-{}", s.untitled)); + let d = Document::new(&nd); + let i = s.add_document(DocState::new(d, None)); + Ok(json!({"index": i})) +} + +fn save_preset(s: &mut Session, p: &Value) -> Result { + let name = str_param(p, "name").map(str::trim).filter(|n| !n.is_empty()).ok_or_else(|| bad("file.savePreset", "missing `name`"))?.to_string(); + if name.chars().count() > MAX_PRESET_NAME { + return Err(bad("file.savePreset", format!("a preset name has at most {MAX_PRESET_NAME} characters"))); + } + if NewDocument::from_preset(&name).is_some() { + return Err(bad("file.savePreset", format!("`{name}` is a built-in preset"))); + } + let mut nd = NewDocument::default(); + apply_settings(&mut nd, p); + check_size("file.savePreset", &nd)?; + nd.title = name.clone(); + let presets = &mut s.prefs.document_presets; + presets.retain(|x| !x.title.eq_ignore_ascii_case(&name)); + if presets.len() >= MAX_PRESETS { + return Err(bad("file.savePreset", format!("at most {MAX_PRESETS} saved presets"))); + } + presets.push(nd); + Ok(json!({"name": name})) +} + +fn check_size(cmd: &str, nd: &NewDocument) -> Result<()> { + if !(nd.width > 0.0 && nd.height > 0.0 && nd.width <= 15552.0 && nd.height <= 15552.0) { + return Err(bad(cmd, "page size out of range (0 < size ≤ 216 in)")); + } + Ok(()) +} + +/// The document settings in `p` (as file.new takes them) over `nd`. +fn apply_settings(nd: &mut NewDocument, p: &Value) { if let Some(intent) = p.get("intent").and_then(|v| serde_json::from_value(v.clone()).ok()) { nd.intent = intent; } @@ -133,7 +202,7 @@ fn file_new(s: &mut Session, p: &Value) -> Result { nd.facing_pages = p.get("facingPages").and_then(Value::as_bool).unwrap_or(nd.facing_pages); nd.columns = p.get("columns").and_then(Value::as_u64).map(|v| v.clamp(1, 216) as u32).unwrap_or(nd.columns); nd.gutter = f64_or(p, "gutter", nd.gutter); - nd.primary_text_frame = p.get("primaryTextFrame").and_then(Value::as_bool).unwrap_or(false); + nd.primary_text_frame = p.get("primaryTextFrame").and_then(Value::as_bool).unwrap_or(nd.primary_text_frame); match p.get("margins") { Some(Value::Number(n)) => nd.margins = designcraft_doc::Margins::uniform(n.as_f64().unwrap_or(36.0)), Some(v @ Value::Object(_)) => { @@ -149,14 +218,6 @@ fn file_new(s: &mut Session, p: &Value) -> Result { if let Some(b) = edges(p, "slug") { nd.slug = b; } - s.untitled += 1; - nd.title = str_param(p, "title").map(str::to_string).unwrap_or_else(|| format!("Untitled-{}", s.untitled)); - if nd.width <= 0.0 || nd.height <= 0.0 || nd.width > 15552.0 || nd.height > 15552.0 { - return Err(bad("file.new", "page size out of range (0 < size ≤ 216 in)")); - } - let d = Document::new(&nd); - let i = s.add_document(DocState::new(d, None)); - Ok(json!({"index": i})) } /// A bleed or slug parameter: one width for every edge, or [top, bottom, inside, outside]. @@ -689,6 +750,41 @@ mod new_document_tests { use crate::Session; + #[test] + fn saved_presets_make_documents_by_name() { + let mut s = Session::new(); + let newsletter = json!({"name": "Newsletter", "units": "millimeters", "width": 595.2755905511812, "height": 841.8897637795277, + "pages": 8, "facingPages": false, "columns": 3, "margins": {"top": 20, "bottom": 30, "inside": 40, "outside": 50}, + "bleed": [9, 9, 0, 9], "primaryTextFrame": true}); + assert_eq!(s.execute("file.savePreset", &newsletter).unwrap()["name"], "Newsletter"); + let listed = s.execute("file.presets", &json!({})).unwrap(); + let saved: Vec<_> = listed.as_array().unwrap().iter().filter(|p| p["saved"] == true).collect(); + assert_eq!(saved.len(), 1); + assert_eq!((&saved[0]["name"], &saved[0]["units"]), (&json!("Newsletter"), &json!("millimeters"))); + // By name, case ignored; parameters still override the preset. + s.execute("file.new", &json!({"preset": "newsletter", "pages": 4})).unwrap(); + let d = &s.doc().unwrap().doc; + assert_eq!(d.page_count(), 4); + assert!(!d.settings.facing_pages && d.settings.primary_text_frame); + assert_eq!(d.settings.horizontal_units, designcraft_geom::Unit::Millimeters); + assert_eq!(d.settings.bleed, [9.0, 9.0, 0.0, 9.0]); + assert_eq!(d.page(0).unwrap().margins.outside, 50.0); + assert!(d.title.starts_with("Untitled-"), "the preset's name isn't the document's"); + // The same name replaces it. + s.execute("file.savePreset", &json!({"name": "NEWSLETTER", "pages": 2})).unwrap(); + assert_eq!(s.prefs.document_presets.len(), 1); + s.execute("file.new", &json!({"preset": "Newsletter"})).unwrap(); + assert_eq!(s.doc().unwrap().doc.page_count(), 2); + // Built-in names, empty and overlong names and impossible sizes are refused. + for bad in [json!({"name": "a4"}), json!({"name": " "}), json!({}), json!({"name": "x".repeat(101)}), json!({"name": "Huge", "width": 1e9})] + { + assert!(s.execute("file.savePreset", &bad).is_err(), "{bad}"); + } + s.execute("file.deletePreset", &json!({"name": "newsletter"})).unwrap(); + assert!(s.execute("file.deletePreset", &json!({"name": "newsletter"})).is_err()); + assert!(s.execute("file.new", &json!({"preset": "Newsletter"})).is_err()); + } + #[test] fn new_document_takes_intent_units_start_page_bleed_and_slug() { let mut s = Session::new(); diff --git a/crates/engine/src/lib.rs b/crates/engine/src/lib.rs index c5835af94..7bab07dad 100644 --- a/crates/engine/src/lib.rs +++ b/crates/engine/src/lib.rs @@ -195,6 +195,8 @@ pub struct Prefs { pub show_note_anchors: bool, /// Preferences › File Handling: minutes between document recovery saves. pub recovery_minutes: f64, + /// File › New › Document › Save Document Preset: saved document presets, by name (`title`). + pub document_presets: Vec, } /// A starter autocorrect list (common English typing slips). @@ -221,6 +223,7 @@ pub fn default_autocorrect() -> Vec<(String, String)> { impl Default for Prefs { fn default() -> Self { Prefs { + document_presets: Vec::new(), show_hidden_characters: false, typographers_quotes: true, polygon_sides: 6, diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index a4e3272c1..162b68733 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1113,6 +1113,9 @@ fn check(ui: &mut egui::Ui, d: &mut Dialog, key: &str, label: &str) { pub fn show(app: &mut DesignApp, ctx: &egui::Context) { let Some(mut d) = app.ui.dialog.clone() else { return }; + // Enter and Escape belong to an open popup (a menu, a name field) rather than the dialog. + // Read before the dialog draws: a popup closed by this frame's Escape is still open here. + let popup_open = egui::Popup::is_any_open(ctx); let mut result: Option = None; let alert_title = d.s("title"); let title = match d.id.as_str() { @@ -1645,13 +1648,13 @@ pub fn show(app: &mut DesignApp, ctx: &egui::Context) { .fill(crate::theme::Tokens::get(ui.ctx()).accent_strong), ) .clicked() - || ui.input(|i| i.key_pressed(egui::Key::Enter)) + || (!popup_open && ui.input(|i| i.key_pressed(egui::Key::Enter))) { result = Some(true); } // An alert only has OK. let cancel = d.id != "alert" && ui.button(crate::rtl::widget(ui, crate::i18n::tr(&app.ui.language, cancel_label))).clicked(); - if cancel || ui.input(|i| i.key_pressed(egui::Key::Escape)) { + if cancel || (!popup_open && ui.input(|i| i.key_pressed(egui::Key::Escape))) { result = Some(false); } if d.id == "closeDocument" && ui.button(crate::rtl::widget(ui, crate::i18n::tr(&app.ui.language, "Don't Save"))).clicked() { diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 6b5bf9dfc..dc63c1db9 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -2867,6 +2867,7 @@ const TABLE: &[(&str, [&str; 5])] = &[ "存储文档预设为：", ], ), + ("Preset Name", ["Vorgabename", "Nom du paramètre prédéfini", "Nombre del ajuste preestablecido", "プリセット名", "预设名称"]), ("Save Preset", ["Vorgabe speichern", "Enregistrer le paramètre prédéfini", "Guardar ajuste preestablecido", "プリセットを保存", "存储预设"]), ("Start #", ["Startseitennr.", "N° de début", "N.º inicial", "開始ページ番号", "起始页码"]), ("Column Gutter", ["Spaltenabstand", "Gouttière", "Medianil", "列間隔", "栏间距"]), diff --git a/crates/ui-egui/src/i18n/it.rs b/crates/ui-egui/src/i18n/it.rs index 52dc962c7..7554a769f 100644 --- a/crates/ui-egui/src/i18n/it.rs +++ b/crates/ui-egui/src/i18n/it.rs @@ -1509,6 +1509,7 @@ pub(super) const TABLE: &[(&str, &str)] = &[ ("Document Name", "Nome documento"), ("Save Document Preset", "Salva predefinito documento"), ("Save Document Preset As:", "Salva predefinito documento come:"), + ("Preset Name", "Nome predefinito"), ("Save Preset", "Salva predefinito"), ("Start #", "N. iniziale"), ("Column Gutter", "Spazio tra colonne"), diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index dcf695c03..386c23790 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -244,8 +244,6 @@ pub struct UiState { pub workspace: String, /// Window › Workspace › New Workspace: saved panel arrangements. pub custom_workspaces: Vec, - /// File › New › Document › Saved: document presets saved by name (`title`). - pub document_presets: Vec, /// File › New › Document › Recent: the settings of the last documents made, newest first. pub recent_new_documents: Vec, /// Transform reference point (0..8, row-major; 0 = top-left). @@ -345,7 +343,6 @@ impl Default for UiState { units: Unit::Picas, workspace: "Essentials".into(), custom_workspaces: Vec::new(), - document_presets: Vec::new(), recent_new_documents: Vec::new(), ref_point: 0, align_to: "selection".into(), diff --git a/crates/ui-egui/src/new_document.rs b/crates/ui-egui/src/new_document.rs index 6e9c1dda5..b27e36b0c 100644 --- a/crates/ui-egui/src/new_document.rs +++ b/crates/ui-egui/src/new_document.rs @@ -139,18 +139,20 @@ pub(crate) fn remember(app: &mut DesignApp, p: &Value) { recent.truncate(MAX_RECENT); } -/// Save the details as the document preset `name` (replacing one of that name) and show it. -fn save_preset(app: &mut DesignApp, d: &mut Dialog, name: &str) { +/// Save the details as the document preset `name` (`file.savePreset`, which replaces one of that +/// name) and show it on the Saved tab. +fn save_preset(app: &mut DesignApp, d: &mut Dialog, name: &str) -> Result<(), String> { let name = name.trim(); - let Some(mut nd) = as_preset(¶ms(d)) else { return }; - if name.is_empty() { - return; + let mut p = params(d); + if let Some(o) = p.as_object_mut() { + o.remove("title"); + o.retain(|_, v| !v.is_null()); + o.insert("name".into(), json!(name)); } - nd.title = name.to_string(); - app.ui.document_presets.retain(|p| p.title != name); - app.ui.document_presets.push(nd); + app.run("file.savePreset", p)?; d.fields.insert("tab".into(), json!("saved")); d.fields.insert("preset".into(), json!(format!("saved:{name}"))); + Ok(()) } /// The built-in preset `nd` is the size of, if any. @@ -184,7 +186,8 @@ fn cards(app: &DesignApp, tab: &str) -> Vec { .collect() } "saved" => app - .ui + .session + .prefs .document_presets .iter() .map(|nd| Card { key: format!("saved:{}", nd.title), label: nd.title.clone(), nd: nd.clone(), custom: preset_name(nd).is_none() }) @@ -366,8 +369,10 @@ fn presets(app: &mut DesignApp, ui: &mut Ui, d: &mut Dialog) -> bool { } } }); - if let Some(name) = delete { - app.ui.document_presets.retain(|p| p.title != name); + if let Some(name) = delete + && let Err(e) = app.run("file.deletePreset", json!({"name": name})) + { + app.status(e); } create } @@ -586,17 +591,30 @@ fn details(app: &mut DesignApp, ui: &mut Ui, d: &mut Dialog) { d.fields.insert("name".into(), json!(name)); } let save = crate::widgets::icon_toggle_sized(ui, "preset-save", false, tr("Save Document Preset"), vec2(30.0, 26.0)); - egui::Popup::menu(&save).show(|ui| { + save.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Button, true, tr("Save Document Preset"))); + // It holds a text field: clicks inside it don't close it. + egui::Popup::menu(&save).close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside).show(|ui| { ui.set_min_width(220.0); crate::rtl::label(ui, tr("Save Document Preset As:")); let mut preset = d.s("presetName"); - if ui.add(egui::TextEdit::singleline(&mut preset).hint_text(tr("Custom")).desired_width(200.0)).changed() { + let field = ui.add(egui::TextEdit::singleline(&mut preset).hint_text(tr("Custom")).desired_width(200.0)); + field.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::TextEdit, true, tr("Preset Name"))); + // Type the name as soon as the popup opens. + if save.clicked() { + field.request_focus(); + } + if field.changed() { d.fields.insert("presetName".into(), json!(preset)); } - if ui.button(crate::rtl::widget(ui, tr("Save Preset"))).clicked() { + // Enter in the name saves the preset (the dialog ignores keys while a popup is open). + let enter = field.lost_focus() && ui.input(|i| i.key_pressed(egui::Key::Enter)); + if ui.button(crate::rtl::widget(ui, tr("Save Preset"))).clicked() || enter { let name = if preset.trim().is_empty() { tr("Custom").to_string() } else { preset }; - save_preset(app, d, &name); - ui.close(); + match save_preset(app, d, &name) { + Ok(()) => ui.close(), + // Refused (a built-in name…): stay open to be corrected. + Err(e) => app.status(e), + } } }); }); @@ -828,14 +846,63 @@ mod tests { assert_eq!((p["gutter"].as_f64(), p["pages"].as_u64()), (Some(12.0), Some(1))); } + /// The app's window (with a document) with the New Document dialog and its Save Document Preset + /// popup open. + fn window_with_preset_popup() -> egui_kittest::Harness<'static, crate::test_window::Window> { + use egui_kittest::kittest::Queryable as _; + let mut app = DesignApp::new(designcraft_engine::Session::new(), crate::Services::default()); + app.run("app.newDocumentDialog", json!({})).unwrap(); + let mut h = crate::test_window::open(app, vec2(1440.0, 900.0)); + // The dialog takes a couple of frames to settle where it opens. + h.run_steps(4); + let button = h.get_by_label("Save Document Preset").rect().center(); + crate::test_window::click_at(&mut h, button); + assert!(egui::Popup::is_any_open(&h.ctx), "the popup opened"); + h + } + + #[test] + fn enter_in_the_preset_popup_saves_the_preset_and_keeps_the_dialog() { + use egui_kittest::kittest::Queryable as _; + let mut h = window_with_preset_popup(); + let field = h.get_by_label("Preset Name").rect().center(); + crate::test_window::click_at(&mut h, field); + h.get_by_label("Preset Name").type_text("Brochure"); + h.run_steps(2); + h.key_press(egui::Key::Enter); + h.run_steps(4); + let app = &mut h.state_mut().app; + let d = app.ui.dialog.as_ref().expect("Enter saves the preset; it doesn't create the document"); + assert_eq!(app.session.documents().len(), 1, "no document was made (the window opens with one)"); + assert_eq!(app.session.prefs.document_presets.iter().map(|p| p.title.as_str()).collect::>(), ["Brochure"]); + assert_eq!((d.s("tab"), d.s("preset")), ("saved".into(), "saved:Brochure".into())); + // The saved preset makes documents by name, as file.new {preset} does from anywhere. + app.run("file.new", json!({"preset": "Brochure"})).unwrap(); + assert!(!egui::Popup::is_any_open(&h.ctx), "the popup closed"); + } + + #[test] + fn escape_in_the_preset_popup_closes_only_the_popup() { + let mut h = window_with_preset_popup(); + h.key_press(egui::Key::Escape); + h.run_steps(4); + assert!(!egui::Popup::is_any_open(&h.ctx), "Escape closed the popup"); + assert!(h.state().app.ui.dialog.is_some(), "and left the dialog open"); + // With no popup open, Escape closes the dialog as before. + h.key_press(egui::Key::Escape); + h.run_steps(4); + assert!(h.state().app.ui.dialog.is_none()); + } + #[test] fn presets_are_saved_by_name_and_recent_documents_once() { let mut app = app(); let mut d = app.ui.dialog.clone().unwrap(); - save_preset(&mut app, &mut d, " Newsletter "); + save_preset(&mut app, &mut d, " Newsletter ").unwrap(); d.fields.insert("pages".into(), json!(8)); - save_preset(&mut app, &mut d, "Newsletter"); - assert_eq!(app.ui.document_presets.len(), 1, "the same name replaces the preset"); + save_preset(&mut app, &mut d, "Newsletter").unwrap(); + assert_eq!(app.session.prefs.document_presets.len(), 1, "the same name replaces the preset"); + assert!(save_preset(&mut app, &mut d, "Letter").is_err(), "a built-in name is refused"); assert_eq!((d.s("tab"), d.s("preset")), ("saved".into(), "saved:Newsletter".into())); let saved = cards(&app, "saved"); assert_eq!((saved.len(), saved[0].label.as_str(), saved[0].nd.pages), (1, "Newsletter", 8));