From f4181890bdeb1bc9d6a26d8f1a2140d9b3006953 Mon Sep 17 00:00:00 2001 From: bflatastic Date: Wed, 7 Oct 2026 08:20:35 +0700 Subject: [PATCH] i18n review follow-ups: names are never translated, palette cache, locale probe off the UI thread Follow-ups from the review of #205. - Names that are user, file or system data are shown as they are: menu items for recent files, saved views, recent fonts and Type > Font, user swatch and graphic style libraries, saved perspective presets, custom workspaces (and Reset ) and plug-ins no longer go through the catalog (menus::shows_a_name). widgets::dropdown_names, menu_item_name and dim_name are the non-translating forms of the shared widgets, for the panels and dialogs that list such names. - The command palette builds its entries (shown label, search text) once per language and plug-in set instead of translating ~700 labels every frame while a query is typed. - The locale probe (reg.exe / defaults) runs on a background thread at startup instead of on the first frame. Co-Authored-By: Claude Opus 5.5 --- apps/vectorcraft/src/main.rs | 1 + crates/ui-egui/src/i18n/mod.rs | 13 +++++ crates/ui-egui/src/menus.rs | 50 ++++++++++++++++-- crates/ui-egui/src/palette.rs | 92 ++++++++++++++++++++++++++-------- crates/ui-egui/src/widgets.rs | 31 ++++++++++-- 5 files changed, 160 insertions(+), 27 deletions(-) diff --git a/apps/vectorcraft/src/main.rs b/apps/vectorcraft/src/main.rs index c203b22d..a1e7bcf1 100644 --- a/apps/vectorcraft/src/main.rs +++ b/apps/vectorcraft/src/main.rs @@ -246,6 +246,7 @@ fn app_icon() -> egui::IconData { const CUSTOM_TITLEBAR: bool = !cfg!(target_os = "macos"); fn main() -> eframe::Result { + vectorcraft_ui_egui::i18n::detect_system_lang_in_background(); let mut control_port: Option = std::env::var("VECTORCRAFT_CONTROL_PORT").ok().and_then(|p| p.parse().ok()); let mut files = Vec::new(); let mut args = std::env::args().skip(1); diff --git a/crates/ui-egui/src/i18n/mod.rs b/crates/ui-egui/src/i18n/mod.rs index ebacd8b9..5e7add93 100644 --- a/crates/ui-egui/src/i18n/mod.rs +++ b/crates/ui-egui/src/i18n/mod.rs @@ -168,6 +168,19 @@ pub fn lang_from_tag(tag: &str) -> Option { cands.iter().find_map(|c| Lang::from_code(c)) } +/// Work out the system language on a background thread, so the first frame doesn't wait for the +/// locale probe (which runs `reg.exe` on Windows and `defaults` on macOS). Call it at startup; a +/// frame that needs the language before the probe finishes waits for it. A no-op on wasm. +pub fn detect_system_lang_in_background() { + #[cfg(not(target_arch = "wasm32"))] + { + // A failed spawn leaves the probe to the first frame that needs it. + let _ = std::thread::Builder::new().name("locale-probe".into()).spawn(|| { + system_lang(); + }); + } +} + /// The system language (cached). English when it can't be determined. pub fn system_lang() -> Lang { // Tests drive the UI by its English labels whatever the developer's locale is. diff --git a/crates/ui-egui/src/menus.rs b/crates/ui-egui/src/menus.rs index d58b2e38..125113ee 100644 --- a/crates/ui-egui/src/menus.rs +++ b/crates/ui-egui/src/menus.rs @@ -1258,9 +1258,29 @@ pub fn dynamic_label(app: &VectorcraftApp, id: &str, label: &str) -> String { } } +/// Does the menu item `id` show a name that is user, file or system data rather than an interface +/// label: a recent file, a saved view, a font, a user library or preset, a custom workspace, a +/// plug-in? Such names are shown as they are, never translated (a workspace the user calls +/// "Layers" stays "Layers"). +fn shows_a_name(id: &str, label: &str) -> bool { + const SLOTS: [&str; 6] = [ + "file.openRecent", + "view.goto", + "type.recentFont", + crate::panels::swatches::USER_SLOT, + crate::panels::graphic_styles::USER_SLOT, + crate::dialogs::perspective_presets::SLOT, + ]; + SLOTS.iter().any(|s| id.starts_with(s)) + || matches!(id, "text.setStyle" | "plugin.dialog") + || (id == "window.workspace" && !crate::workspaces::is_builtin(label)) + || (matches!(id, "effect.apply" | "effect.dialog") && vectorcraft_effects::plugin_effects().iter().any(|e| e.label == label)) +} + /// The label of a menu item as drawn: [`dynamic_label`] in the UI language. Labels assembled /// around a name (Undo *Move*, Reset *Essentials*, Last Effect: *Drop Shadow*) are translated as -/// templates so the name can move; names that are user data (files, views, fonts) pass through. +/// templates so the name can move; names that are user data (files, views, fonts, custom +/// workspaces, plug-ins: [`shows_a_name`]) pass through. pub fn display_label(app: &VectorcraftApp, id: &str, label: &str) -> String { let lang = crate::i18n::current(); let fmt = crate::i18n::fmt; @@ -1273,7 +1293,10 @@ pub fn display_label(app: &VectorcraftApp, id: &str, label: &str) -> String { None => tl!(if id == "edit.undo" { "Undo" } else { "Redo" }).to_string(), } } - "window.workspace.reset" => fmt(tl!("Reset {name}"), &[("name", tl!(&app.ui.workspace))]), + "window.workspace.reset" => { + let name = &app.ui.workspace; + fmt(tl!("Reset {name}"), &[("name", if crate::workspaces::is_builtin(name) { tl!(name) } else { name })]) + } "effect.last" => match &app.last_effect { Some((e, _)) => { let name = vectorcraft_effects::effect_info(e).map(|i| i.label).unwrap_or(e.as_str()); @@ -1281,7 +1304,10 @@ pub fn display_label(app: &VectorcraftApp, id: &str, label: &str) -> String { } None => crate::i18n::tr_id(lang, id, label).to_string(), }, - _ => crate::i18n::tr_id(lang, id, &dynamic_label(app, id, label)).to_string(), + _ => { + let shown = dynamic_label(app, id, label); + if shows_a_name(id, &shown) { shown } else { crate::i18n::tr_id(lang, id, &shown).to_string() } + } } } @@ -2896,6 +2922,24 @@ pub fn menu_strings() -> std::collections::BTreeSet { mod tests { use super::*; + #[test] + fn names_in_menus_are_not_interface_labels() { + for (id, label) in [ + ("file.openRecent1", "Layers.svg"), + ("view.goto2", "Layers"), + ("type.recentFont1", "Regular"), + ("text.setStyle", "Black"), + ("window.workspace", "Layers"), + ("plugin.dialog", "Group"), + ("window.userSwatchLibrary3", "Default"), + ] { + assert!(shows_a_name(id, label), "{id} {label}"); + } + for (id, label) in [("object.group", "Group"), ("window.workspace", "Essentials"), ("effect.apply", "Drop Shadow")] { + assert!(!shows_a_name(id, label), "{id} {label}"); + } + } + #[test] fn last_effect_dialog_and_view_toggles() { let mut app = VectorcraftApp::new(vectorcraft_engine::Session::new(), crate::Services::default()); diff --git a/crates/ui-egui/src/palette.rs b/crates/ui-egui/src/palette.rs index aa7699c3..9378828a 100644 --- a/crates/ui-egui/src/palette.rs +++ b/crates/ui-egui/src/palette.rs @@ -1,5 +1,7 @@ //! The command palette: fuzzy search over every command and tool. +use std::sync::Arc; + use serde_json::json; use crate::theme::Tokens; @@ -23,10 +25,46 @@ pub fn items() -> Vec<(String, String, String)> { items } -/// A palette label in the UI language: each `›`-separated menu segment and the command label -/// translate on their own. -fn shown_label(label: &str) -> String { - label.split(" › ").map(crate::i18n::t).collect::>().join(" › ") +/// A palette label in `lang`: each `›`-separated menu segment and the command label translate on +/// their own. +fn shown_label(lang: crate::i18n::Lang, label: &str) -> String { + label.split(" › ").map(|s| crate::i18n::tr(lang, s)).collect::>().join(" › ") +} + +/// One palette entry, with what it shows and what a query matches worked out once. +struct Entry { + id: String, + shortcut: String, + /// The label in the UI language. + shown: String, + /// The English label (what agents document), the shown label and the id, lowercased, one per + /// line: a query word (which has no line break) matches within one of them. + haystack: String, +} + +/// The palette's entries in `lang`. They are built when the language or the installed plug-ins +/// change, not on every frame while a query is typed. +fn entries(ctx: &egui::Context, lang: crate::i18n::Lang) -> Arc> { + type Cached = ((&'static str, u64), Arc>); + let key = (lang.code(), menus::plugin_revision()); + let id = egui::Id::new("palette-entries"); + if let Some((k, v)) = ctx.data(|d| d.get_temp::(id)) + && k == key + { + return v; + } + let built: Arc> = Arc::new( + items() + .into_iter() + .map(|(label, id, shortcut)| { + let shown = shown_label(lang, &label); + let haystack = format!("{}\n{}\n{}", label.to_lowercase(), shown.to_lowercase(), id.to_lowercase()); + Entry { id, shortcut, shown, haystack } + }) + .collect(), + ); + ctx.data_mut(|d| d.insert_temp::(id, (key, built.clone()))); + built } pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { @@ -35,18 +73,8 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { } let t = Tokens::get(ctx); let q = app.ui.palette_query.to_lowercase(); - let items = items(); - let matches: Vec<&(String, String, String)> = items - .iter() - .filter(|(l, id, _)| { - q.is_empty() || { - // Match the English text (what agents document), the shown text and the id. - let shown = shown_label(l).to_lowercase(); - q.split_whitespace().all(|w| l.to_lowercase().contains(w) || shown.contains(w) || id.to_lowercase().contains(w)) - } - }) - .take(14) - .collect(); + let entries = entries(ctx, crate::i18n::current()); + let matches: Vec<&Entry> = entries.iter().filter(|e| q.split_whitespace().all(|w| e.haystack.contains(w))).take(14).collect(); let mut run: Option = None; egui::Area::new(egui::Id::new("palette")).order(egui::Order::Foreground).anchor(egui::Align2::CENTER_TOP, [0.0, 90.0]).show(ctx, |ui| { egui::Frame::popup(ui.style()).fill(t.panel).inner_margin(egui::Margin::same(10)).show(ui, |ui| { @@ -54,21 +82,21 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { let r = ui.add(egui::TextEdit::singleline(&mut app.ui.palette_query).hint_text(tl!("Search commands and tools…")).desired_width(500.0)); r.request_focus(); ui.add_space(6.0); - for (i, (label, id, sc)) in matches.iter().enumerate() { + for (i, e) in matches.iter().enumerate() { let resp = ui.add( - egui::Button::new(shown_label(label)) - .shortcut_text(menus::pretty_shortcut(sc)) + egui::Button::new(e.shown.as_str()) + .shortcut_text(menus::pretty_shortcut(&e.shortcut)) .min_size(egui::vec2(500.0, 24.0)) .selected(i == 0), ); if resp.clicked() { - run = Some(id.clone()); + run = Some(e.id.clone()); } } if ui.input(|i| i.key_pressed(egui::Key::Enter)) && let Some(first) = matches.first() { - run = Some(first.1.clone()); + run = Some(first.id.clone()); } }); }); @@ -81,3 +109,25 @@ pub fn show(app: &mut VectorcraftApp, ctx: &egui::Context) { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::i18n::Lang; + + #[test] + fn entries_are_built_once_per_language_and_match_english_shown_text_and_ids() { + let ctx = egui::Context::default(); + let zh = Lang::from_code("zh-hant").unwrap(); + let en = entries(&ctx, Lang::EN); + assert!(Arc::ptr_eq(&en, &entries(&ctx, Lang::EN)), "kept while the language stays"); + let zh_entries = entries(&ctx, zh); + assert!(!Arc::ptr_eq(&en, &zh_entries), "rebuilt for another language"); + let group = zh_entries.iter().find(|e| e.id == "object.group").unwrap(); + assert!(group.shown.ends_with(crate::i18n::tr(zh, "Group")), "{}", group.shown); + // A query matches the English label, the shown label or the id. + for word in ["group", crate::i18n::tr(zh, "Group"), "object.group"] { + assert!(group.haystack.contains(&word.to_lowercase()), "{word}"); + } + } +} diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 0a7d9ef7..b547671a 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -88,8 +88,14 @@ pub fn divider(ui: &mut Ui) { } pub fn dim_label(ui: &mut Ui, text: &str) -> Response { + dim_name(ui, tl!(text)) +} + +/// [`dim_label`] for a name that is user or file data (a swatch, style, artboard or font name): +/// shown as it is, never translated. +pub fn dim_name(ui: &mut Ui, text: &str) -> Response { let t = Tokens::get(ui.ctx()); - ui.label(egui::RichText::new(tl!(text)).color(t.text).size(12.5)) + ui.label(egui::RichText::new(text).color(t.text).size(12.5)) } /// A recessed numeric field showing `value` (points) in `unit`. Returns the new value (points) @@ -395,11 +401,26 @@ pub fn fill_stroke_proxy(ui: &mut Ui, fill: &Paint, stroke: &Paint, mixed: (bool } } -/// A compact dropdown. Returns the chosen index. +/// A compact dropdown of interface labels (shown in the UI language). Returns the chosen index. pub fn dropdown(ui: &mut Ui, id: impl std::hash::Hash + std::fmt::Debug, current: &str, options: &[&str], width: f32) -> Option { dropdown_with(ui, id, current, options, width, |_| true) } +/// [`dropdown`] of names that are user, file or system data (font styles, artboards, presets the +/// user saved, printers, profiles), shown as they are, never translated. For a list that mixes +/// built-in labels with such names, translate the built-in ones (`tl!`) before passing them. +pub fn dropdown_names(ui: &mut Ui, id: impl std::hash::Hash + std::fmt::Debug, current: &str, options: &[&str], width: f32) -> Option { + combo(ui, id, current, width, false, |ui| { + let mut chosen = None; + for (i, o) in options.iter().enumerate() { + if ui.add(egui::Button::selectable(*o == current, *o)).clicked() { + chosen = Some(i); + } + } + chosen + }) +} + /// [`dropdown`] whose options can be disabled (`enabled(index)`: greyed and not choosable). pub fn dropdown_with( ui: &mut Ui, @@ -1057,7 +1078,11 @@ pub fn bottom_bar(ui: &mut Ui, add: impl FnOnce(&mut Ui)) { /// A menu row for panel (≡) menus: label, optional check mark, disabled when not implemented. pub fn menu_item(ui: &mut Ui, label: &str, enabled: bool, checked: bool) -> bool { - let label = tl!(label); + menu_item_name(ui, tl!(label), enabled, checked) +} + +/// [`menu_item`] for a name that is user or file data (a library, a recent URL), never translated. +pub fn menu_item_name(ui: &mut Ui, label: &str, enabled: bool, checked: bool) -> bool { let text = if checked { format!("✓ {label}") } else { format!(" {label}") }; ui.add_enabled(enabled, egui::Button::new(egui::RichText::new(text).size(12.5)).frame(false)).clicked() }