diff --git a/crates/ui-egui/src/i18n.rs b/crates/ui-egui/src/i18n.rs index 40f60270..2830021a 100644 --- a/crates/ui-egui/src/i18n.rs +++ b/crates/ui-egui/src/i18n.rs @@ -229,6 +229,28 @@ mod tests { assert_eq!(Locale(&PARTIAL).text("ui.extrude"), "Extrude"); } + /// `label()`'s id fallback (an argument that matches no other entry's English text is + /// looked up as an id directly) is how the Sketch Palette reaches `ui.palette_profile` and + /// `ui.palette_points` without colliding with "Profile" / "Points" elsewhere (see the + /// comment on `sketch_palette::TOGGLES`). Prove it resolves to each locale's own + /// translation, not merely to something: a broken id would fall back to the English catalog + /// silently (`Locale::text`'s own fallback chain), so a test that only checks "resolves to a + /// non-empty string" would not catch that. + #[test] + fn id_labels_resolve_to_their_own_translation_not_an_english_fallback() { + for (lang, profile, points) in [("de", "Profil", "Punkte"), ("ja", "プロファイル", "点"), ("ko", "프로파일", "점")] { + let locale = Locale::for_language(lang); + assert_eq!(locale.label("ui.palette_profile"), profile, "{lang}"); + assert_eq!(locale.label("ui.palette_points"), points, "{lang}"); + } + // The literal English text "Profile" / "Points" must keep naming the pre-existing, + // unrelated entries (the SelInput row labels in dialogs.rs, and "Show Profile" / + // "Hide Profile" in context_menu.rs), not the palette's own ids: that separation is the + // entire reason these two rows pass an id instead of their English text. + assert_eq!(Locale::for_language("de").label("Profile"), Locale::for_language("de").text("ui.profile")); + assert_eq!(Locale::for_language("de").label("Points"), Locale::for_language("de").text("ui.points")); + } + #[test] fn templates_allow_reordering_and_literal_braces_in_values() { assert_eq!(interpolate("{second} then {first}", &[("first", "{second}"), ("second", "B")]), "B then {second}"); diff --git a/crates/ui-egui/src/i18n_ui_tests.rs b/crates/ui-egui/src/i18n_ui_tests.rs index 9e501bed..213835c0 100644 --- a/crates/ui-egui/src/i18n_ui_tests.rs +++ b/crates/ui-egui/src/i18n_ui_tests.rs @@ -21,6 +21,26 @@ fn japanese_dialog_keeps_english_scenario_handles() { assert!(h.widget("Cancel").is_some()); } +#[test] +fn sketch_palette_profile_and_points_rows_show_their_own_translation() { + // ui.palette_profile / ui.palette_points are reached by id, not by the usual English-text + // lookup (see the comment on sketch_palette::TOGGLES, and + // i18n::tests::id_labels_resolve_to_their_own_translation_not_an_english_fallback for the + // lookup itself). A broken id falls back to English silently rather than failing loudly, so + // check the word actually on screen in each locale, not just that something renders. + for (lang, profile, points) in [("de", "Profil", "Punkte"), ("ja", "プロファイル", "点"), ("ko", "프로파일", "점")] { + let mut h = Harness::new(); + h.app.preferences.language = lang.into(); + h.app.home.open = false; + h.app.run("sketch.create", json!({"plane":"XY"})).unwrap(); + h.frames(3); + assert!(h.widget(profile).is_some(), "{lang}: the Profile row must show {profile:?}"); + assert!(h.widget(points).is_some(), "{lang}: the Points row must show {points:?}"); + assert!(h.widget("Profile").is_none(), "{lang}: must not silently fall back to English"); + assert!(h.widget("Points").is_none(), "{lang}: must not silently fall back to English"); + } +} + #[test] fn language_preference_cancel_restores_previous_locale() { let mut h = Harness::new(); diff --git a/crates/ui-egui/src/locales/de.json b/crates/ui-egui/src/locales/de.json index 8dfff0e5..333fca23 100644 --- a/crates/ui-egui/src/locales/de.json +++ b/crates/ui-egui/src/locales/de.json @@ -210,6 +210,8 @@ "ui.options": "Optionen", "ui.orbit_around": "Umkreisen um", "ui.origin": "Ursprung", + "ui.palette_points": "Punkte", + "ui.palette_profile": "Profil", "ui.pan_zoom_orbit": "Verschieben, zoomen, umkreisen", "ui.parallel": "Parallel", "ui.parameters": "Parameter", @@ -251,11 +253,11 @@ "ui.selection": "Auswahl", "ui.shell": "Hülle", "ui.show": "Einblenden", - "ui.show_constraints": "Bedingungen anzeigen", - "ui.show_dimensions": "Maße anzeigen", + "ui.show_constraints": "Bedingungen", + "ui.show_dimensions": "Maße", "ui.show_points": "Punkte anzeigen", "ui.show_profile": "Profil anzeigen", - "ui.show_projected_geometries": "Projizierte Geometrien anzeigen", + "ui.show_projected_geometries": "Projizierte Geometrien", "ui.simple": "Einfach", "ui.simulation": "Simulation", "ui.single": "Einzel", diff --git a/crates/ui-egui/src/locales/en.json b/crates/ui-egui/src/locales/en.json index 660fea7b..482d371f 100644 --- a/crates/ui-egui/src/locales/en.json +++ b/crates/ui-egui/src/locales/en.json @@ -210,6 +210,8 @@ "ui.options": "Options", "ui.orbit_around": "Orbit around", "ui.origin": "Origin", + "ui.palette_points": "Points", + "ui.palette_profile": "Profile", "ui.pan_zoom_orbit": "Pan, zoom, orbit", "ui.parallel": "Parallel", "ui.parameters": "Parameters", @@ -251,11 +253,11 @@ "ui.selection": "Selection", "ui.shell": "Shell", "ui.show": "Show", - "ui.show_constraints": "Show Constraints", - "ui.show_dimensions": "Show Dimensions", + "ui.show_constraints": "Constraints", + "ui.show_dimensions": "Dimensions", "ui.show_points": "Show Points", "ui.show_profile": "Show Profile", - "ui.show_projected_geometries": "Show Projected Geometries", + "ui.show_projected_geometries": "Projected Geometries", "ui.simple": "Simple", "ui.simulation": "Simulation", "ui.single": "Single", diff --git a/crates/ui-egui/src/locales/ja.json b/crates/ui-egui/src/locales/ja.json index 5928d2d4..f837c0b0 100644 --- a/crates/ui-egui/src/locales/ja.json +++ b/crates/ui-egui/src/locales/ja.json @@ -210,6 +210,8 @@ "ui.options": "オプション", "ui.orbit_around": "回転の中心", "ui.origin": "原点", + "ui.palette_points": "点", + "ui.palette_profile": "プロファイル", "ui.pan_zoom_orbit": "パン、ズーム、オービット", "ui.parallel": "平行", "ui.parameters": "パラメータ", @@ -251,11 +253,11 @@ "ui.selection": "選択", "ui.shell": "シェル", "ui.show": "表示", - "ui.show_constraints": "拘束を表示", - "ui.show_dimensions": "寸法を表示", + "ui.show_constraints": "拘束", + "ui.show_dimensions": "寸法", "ui.show_points": "点を表示", "ui.show_profile": "プロファイルを表示", - "ui.show_projected_geometries": "投影ジオメトリを表示", + "ui.show_projected_geometries": "投影ジオメトリ", "ui.simple": "単純", "ui.simulation": "シミュレーション", "ui.single": "単一", diff --git a/crates/ui-egui/src/locales/ko.json b/crates/ui-egui/src/locales/ko.json index 46088c4e..5785375c 100644 --- a/crates/ui-egui/src/locales/ko.json +++ b/crates/ui-egui/src/locales/ko.json @@ -210,6 +210,8 @@ "ui.options": "옵션", "ui.orbit_around": "궤도 회전 기준", "ui.origin": "원점", + "ui.palette_points": "점", + "ui.palette_profile": "프로파일", "ui.pan_zoom_orbit": "이동, 확대/축소, 궤도 회전", "ui.parallel": "평행", "ui.parameters": "매개변수", @@ -251,11 +253,11 @@ "ui.selection": "선택", "ui.shell": "쉘", "ui.show": "표시", - "ui.show_constraints": "구속 조건 표시", - "ui.show_dimensions": "치수 표시", + "ui.show_constraints": "구속 조건", + "ui.show_dimensions": "치수", "ui.show_points": "점 표시", "ui.show_profile": "프로파일 표시", - "ui.show_projected_geometries": "투영된 형상 표시", + "ui.show_projected_geometries": "투영된 형상", "ui.simple": "단순", "ui.simulation": "시뮬레이션", "ui.single": "단일", diff --git a/crates/ui-egui/src/sketch_palette.rs b/crates/ui-egui/src/sketch_palette.rs index 97259456..0efc746a 100644 --- a/crates/ui-egui/src/sketch_palette.rs +++ b/crates/ui-egui/src/sketch_palette.rs @@ -28,16 +28,41 @@ thread_local! { /// Palette width (logical px). const WIDTH: f32 = 236.0; -/// The display toggles: option key and label. +/// The display toggles: option key and label. Plain nouns throughout, no "Show " prefix: the +/// checkbox itself already says whether each one is on. +/// +/// Seven of these pass their English source text directly, like the rest of this file's +/// `app.tr_label(...)` calls: `Locale::label` looks up a literal English string against every +/// other English catalog entry to find its id. "Profile" and "Points" cannot do that safely: +/// both already name something else under that same lookup (`SelInput::new("Profile", ...)` / +/// `SelInput::new("Points", ...)` in `dialogs.rs`, the Extrude/Sweep/Shell/Loft and +/// point-selection dialogs' own row labels, and "Profile" is also `context_menu.rs`'s "Show +/// Profile" / "Hide Profile" action by way of the existing `ui.show_profile` key). Giving the +/// palette row the same bare text would silently repoint one of those unrelated lookups to +/// whichever translation this file happens to carry. `ui.palette_profile` and +/// `ui.palette_points` are dedicated keys instead, carrying exactly "Profile" and "Points" in +/// English; passing the id itself (`Locale::label` falls back to a direct id lookup when the +/// text it was given does not match any other entry) reaches them without going through the +/// shared text lookup at all. See `i18n::tests::id_labels_resolve_to_their_own_translation_not_an_english_fallback` +/// and `i18n_ui_tests::sketch_palette_profile_and_points_rows_show_their_own_translation` for +/// proof they resolve to the real translation in German, Japanese and Korean rather than +/// silently falling back to English. +/// +/// The id-fallback check still runs the text lookup first: a future English catalog entry +/// whose value happened to be the literal string "ui.palette_profile" or "ui.palette_points" +/// would silently steal this lookup (`Locale::label`'s `ids.get(label)` would then match +/// before the id fallback ever runs). Nobody writes UI copy that looks like a dotted id, so +/// this is a remote hazard, not a likely one, but it is why these two ids are spelled out here +/// rather than reused anywhere else. pub const TOGGLES: [(&str, &str); 9] = [ ("grid", "Sketch Grid"), ("snap", "Snap"), ("slice", "Slice"), - ("show_profile", "Show Profile"), - ("show_points", "Show Points"), - ("show_dimensions", "Show Dimensions"), - ("show_constraints", "Show Constraints"), - ("show_projected", "Show Projected Geometries"), + ("show_profile", "ui.palette_profile"), + ("show_points", "ui.palette_points"), + ("show_dimensions", "Dimensions"), + ("show_constraints", "Constraints"), + ("show_projected", "Projected Geometries"), ("sketch_3d", "3D Sketch"), ]; @@ -191,6 +216,17 @@ pub fn set_linetype(app: &mut SolveApp, kind: u8) { } } +/// A small disclosure triangle for the Options section: pointing right when collapsed, down +/// when open. Kept separate from the palette's own "−"/"+" fold, which stays a plain glyph. +fn disclosure_triangle(p: &egui::Painter, c: egui::Pos2, open: bool, col: Color32) { + let pts = if open { + vec![egui::pos2(c.x - 4.0, c.y - 2.0), egui::pos2(c.x + 4.0, c.y - 2.0), egui::pos2(c.x, c.y + 3.0)] + } else { + vec![egui::pos2(c.x - 2.5, c.y - 4.0), egui::pos2(c.x - 2.5, c.y + 4.0), egui::pos2(c.x + 3.0, c.y)] + }; + p.add(egui::Shape::convex_polygon(pts, col, Stroke::NONE)); +} + /// Draw the palette (while sketching and no command dialog is open). pub fn show(app: &mut SolveApp, ctx: &egui::Context) { app.viewport.palette_rect = None; @@ -259,10 +295,11 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { } let ofold = OPTIONS_FOLDED.with(Cell::get); crate::frame::heading(ui, Level::Section, width, |ui| { - if ui - .add(egui::Button::new(RichText::new(format!("{} {}", if ofold { "+" } else { "−" }, app.tr("ui.options"))).size(12.0)).frame(false)) - .clicked() - { + let (tri_rect, tri_resp) = ui.allocate_exact_size(vec2(14.0, 14.0), egui::Sense::click()); + disclosure_triangle(ui.painter(), tri_rect.center(), !ofold, t.text); + crate::scenario::publish_handle("sketch_palette:options_fold", tri_rect.center()); + let label_resp = ui.add(egui::Label::new(RichText::new(app.tr("ui.options")).size(12.0)).sense(egui::Sense::click())); + if tri_resp.clicked() || label_resp.clicked() { OPTIONS_FOLDED.with(|f| f.set(!ofold)); } }); @@ -272,36 +309,72 @@ pub fn show(app: &mut SolveApp, ctx: &egui::Context) { let options_height = (bottom - ui.cursor().top() - footer).max(24.0); let options = egui::ScrollArea::vertical().id_salt("sc_palette_options").max_height(options_height).auto_shrink([false, true]).show(ui, |ui| { + // Every row below: its label at the left, its control right-aligned at the + // same column (`frame::right_aligned`), so every row lines up instead of + // each control sitting wherever its label happened to end. ui.vertical(|ui| { - ui.label(app.tr("ui.linetype")); ui.horizontal(|ui| { - // With a selection: its line type, and a click converts it; without: - // the mode new curves are drawn in. - let current = mode(app); - for (i, (label, tip)) in [("—", "Normal"), ("- -", "Construction"), ("-·-", "Centerline")].iter().enumerate() { - let on = if has_curves { selected_types.get(i).copied().unwrap_or(false) } else { current == i as u8 }; - let tr_tip = app.tr_label(tip); - let tip = if has_curves { - crate::i18n::format("sketch.make_selection", &[("linetype", tr_tip)]) - } else { - crate::i18n::format("sketch.draw_curves", &[("linetype", tr_tip)]) - }; - if ui.add(egui::Button::new(*label).selected(on).min_size(vec2(30.0, 20.0))).on_hover_text(tip).clicked() { - linetype = Some(i as u8); - } + ui.label(app.tr("ui.linetype")); + let btn_w = 30.0; + let group_w = 3.0 * btn_w + 2.0 * ui.spacing().item_spacing.x; + crate::frame::right_aligned(ui, width, group_w, |ui| { + ui.horizontal(|ui| { + // With a selection: its line type, and a click converts it; + // without: the mode new curves are drawn in. + let current = mode(app); + for (i, (label, tip)) in [("—", "Normal"), ("- -", "Construction"), ("-·-", "Centerline")].iter().enumerate() { + let on = if has_curves { selected_types.get(i).copied().unwrap_or(false) } else { current == i as u8 }; + let tr_tip = app.tr_label(tip); + let tip = if has_curves { + crate::i18n::format("sketch.make_selection", &[("linetype", tr_tip)]) + } else { + crate::i18n::format("sketch.draw_curves", &[("linetype", tr_tip)]) + }; + if ui.add(egui::Button::new(*label).selected(on).min_size(vec2(btn_w, 20.0))).on_hover_text(tip).clicked() { + linetype = Some(i as u8); + } + } + }); + }); + }); + ui.horizontal(|ui| { + // One label, one icon button (the "lookat" glyph already used on the + // viewport's own navigation overlay): not the word "Look At" twice. + // The button carries no visible text of its own; `labelled_by` gives + // it the row's own label as its accessible name, so a screen reader + // still hears "Look At" the way the old text button said it. + let label_resp = ui.label(app.tr("ui.look_at")); + let icon_w = 28.0; + let resp = crate::frame::right_aligned(ui, width, icon_w, |ui| { + let r = ui.add(egui::Button::new("").min_size(vec2(icon_w, 20.0))); + let side = (r.rect.width().min(r.rect.height()) - 4.0).max(12.0); + let icon_rect = egui::Rect::from_center_size(r.rect.center(), vec2(side, side)); + crate::icons::paint(ui.painter(), icon_rect, "lookat", t.icon, t.icon_fill, t.accent); + crate::scenario::publish_handle("sketch_palette:look_at", r.rect.center()); + r.labelled_by(label_resp.id) + }); + let resp = resp.on_hover_text(app.tr("ui.look_at")); + if resp.clicked() { + look = true; } }); - ui.label(app.tr("ui.look_at")); - if ui.button(app.tr("ui.look_at")).clicked() { - look = true; - } }); for (k, label) in TOGGLES { - // The label is part of the checkbox: clicking it toggles too. - let mut on = shown(k); - if ui.checkbox(&mut on, app.tr_label(label)).changed() { - set = Some((k.to_string(), on)); - } + ui.horizontal(|ui| { + // The label is still part of the toggle: clicking it flips the + // checkbox too, same as before the checkbox moved to its own column. + let label_resp = ui.add(egui::Label::new(app.tr_label(label)).sense(egui::Sense::click())); + let mut on = shown(k); + let box_w = ui.spacing().interact_size.y; + let cb = crate::frame::right_aligned(ui, width, box_w, |ui| { + ui.add(egui::Checkbox::without_text(&mut on)).labelled_by(label_resp.id) + }); + if cb.changed() { + set = Some((k.to_string(), on)); + } else if label_resp.clicked() { + set = Some((k.to_string(), !on)); + } + }); } }); crate::scenario::publish_rect("sketch_palette:options", options.inner_rect); @@ -426,6 +499,63 @@ mod tests { assert_eq!(pressed(&mut h, &[]), [false, true, false]); } + #[test] + fn look_at_is_one_label_and_an_icon_button_that_animates_the_camera() { + let mut h = crate::scenario::Harness::new(); + h.app.run("sketch.create", json!({"plane":"XY"})).unwrap(); + h.settle(); + // Point the camera somewhere clearly not looking straight down the sketch plane, so the + // click's effect is unambiguous regardless of where `sketch.create` happened to leave + // it. `Harness::call` settles any camera animation to completion before it returns, so + // checking `cam_anim` right after the click (as an earlier version of this test did) + // never sees it: the animation is always finished and cleared by then. Check where the + // camera actually ended up instead. + h.app.cam.yaw = 1.0; + h.app.cam.pitch = 0.2; + h.settle(); + // The row's own label is the only place the words "Look At" are drawn; the control + // next to it is an icon button with no visible text of its own. + assert!(h.widget("Look At").is_some(), "the row label must still be reachable by name"); + let at = crate::scenario::handle_at("sketch_palette:look_at").unwrap(); + h.call("ui.click", json!({"x": at.x, "y": at.y})); + let (yaw, pitch) = (h.app.cam.yaw, h.app.cam.pitch); + assert!((pitch - std::f64::consts::FRAC_PI_2).abs() < 0.01, "must look straight down the XY sketch plane: pitch={pitch}"); + assert!(yaw.abs() < 0.01, "must align with the sketch's own x axis: yaw={yaw}"); + } + + #[test] + fn options_disclosure_triangle_folds_and_unfolds_the_section() { + OPTIONS_FOLDED.with(|v| v.set(false)); + let mut h = crate::scenario::Harness::new(); + h.app.run("sketch.create", json!({"plane":"XY"})).unwrap(); + h.settle(); + assert!(h.widget("Sketch Grid").is_some(), "options are open by default"); + let at = crate::scenario::handle_at("sketch_palette:options_fold").unwrap(); + h.call("ui.click", json!({"x": at.x, "y": at.y})); + h.settle(); + assert!(h.widget("Sketch Grid").is_none(), "the triangle must fold the options section"); + h.call("ui.click", json!({"x": at.x, "y": at.y})); + h.settle(); + assert!(h.widget("Sketch Grid").is_some(), "the triangle must unfold it again"); + OPTIONS_FOLDED.with(|v| v.set(false)); + } + + #[test] + fn toggle_row_label_and_checkbox_both_flip_the_option() { + let mut h = crate::scenario::Harness::new(); + h.app.run("sketch.create", json!({"plane":"XY"})).unwrap(); + h.settle(); + let active = h.app.session.active_sketch.unwrap(); + let slice_on = |h: &mut crate::scenario::Harness| h.app.session.world_state().sketch(active).unwrap().sketch.view.slice; + assert!(!slice_on(&mut h)); + // Clicking the row's label (not just the checkbox) toggles the option, same as before + // the checkbox and the label became separate widgets in their own column. + let [x, y] = h.widget("Slice").unwrap(); + h.call("ui.click", json!({"x": x, "y": y})); + h.settle(); + assert!(slice_on(&mut h), "clicking the \"Slice\" label must toggle it on"); + } + #[test] fn dof_readout() { assert_eq!(dof_text(0, true), "Fully constrained");