Sketch Palette: a right-aligned control column, one Look At icon, a disclosure triangle, plain-noun labels (#497)

Every Options row puts its control in one right-aligned column, Look At is a single label with an icon button, Options folds with a disclosure triangle, and the toggles use plain nouns (with dedicated ids for Profile and Points so they don't collide with other labels).

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}");

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();

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",

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",

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": "単一",

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": "단일",

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");