From d7a78ac358bf3a1fd3e690d0669b834079858806 Mon Sep 17 00:00:00 2001 From: Trevor Brown Date: Fri, 6 Nov 2026 01:07:59 -0500 Subject: [PATCH] Line up the boolean menu's labels: reserve the check and icon columns for every entry menu_icon_item drew the check mark as a text prefix, so a checked entry's label sat about 14 px left of the others, and Flatten (no icon) started further left still. The check now has its own fixed column ahead of the icon slot, and Flatten uses the same row with an empty icon slot. Co-Authored-By: Claude Fable 5.1 --- crates/ui-egui/src/bool_menu_icon_tests.rs | 37 ++++++++++++++++++++-- crates/ui-egui/src/panels/right.rs | 4 +-- crates/ui-egui/src/panels/right/menu.rs | 23 ++++++++++---- 3 files changed, 54 insertions(+), 10 deletions(-) diff --git a/crates/ui-egui/src/bool_menu_icon_tests.rs b/crates/ui-egui/src/bool_menu_icon_tests.rs index 243286f19..8d3ac84c9 100644 --- a/crates/ui-egui/src/bool_menu_icon_tests.rs +++ b/crates/ui-egui/src/bool_menu_icon_tests.rs @@ -63,7 +63,7 @@ fn open_menu(u: &mut Ui) -> Vec { nodes(u) } -/// How many strokes and fills sit in the left 24 points of `row` (where the icon is). +/// How many strokes and fills sit in the left 40 points of `row` (where the icon is). fn marks_in_lead(shapes: &[egui::epaint::ClippedShape], row: Rect) -> usize { fn walk(s: &Shape, area: Rect, n: &mut usize) { match s { @@ -72,7 +72,7 @@ fn marks_in_lead(shapes: &[egui::epaint::ClippedShape], row: Rect) -> usize { _ => {} } } - let area = Rect::from_min_max(row.min, pos2(row.min.x + 24.0, row.max.y)); + let area = Rect::from_min_max(row.min, pos2(row.min.x + 40.0, row.max.y)); let mut n = 0; shapes.iter().for_each(|c| walk(&c.shape, area, &mut n)); n @@ -105,3 +105,36 @@ fn every_boolean_menu_entry_still_runs_its_command() { assert_eq!(crate::layers::node_icon(u.app.session.doc().unwrap(), sel[0]), icon, "{name}'s group shows {icon:?}"); } } + +/// The x every label (checked or not, with an icon or without, Flatten) starts at. +fn label_xs(shapes: &[egui::epaint::ClippedShape]) -> Vec<(String, f32)> { + fn walk(s: &Shape, out: &mut Vec<(String, f32)>) { + match s { + Shape::Vec(v) => v.iter().for_each(|s| walk(s, out)), + Shape::Text(t) => out.push((t.galley.text().to_owned(), t.pos.x)), + _ => {} + } + } + let mut out = Vec::new(); + shapes.iter().for_each(|c| walk(&c.shape, &mut out)); + out +} + +#[test] +fn every_boolean_menu_label_starts_at_the_same_x() { + for checked in [false, true] { + let mut u = two_layers(); + if checked { + u.app.run("bool.union", json!({})); + u.frame(vec![]); + } + open_menu(&mut u); + let out = u.frame_output(vec![]); + let xs = label_xs(&out.shapes); + let at: Vec = ["Union", "Subtract", "Intersect", "Exclude", "Flatten"] + .iter() + .map(|n| xs.iter().rev().find(|(t, _)| t == n).unwrap_or_else(|| panic!("{n} label (checked: {checked})")).1) + .collect(); + assert!(at.iter().all(|x| (x - at[0]).abs() < 0.01), "labels line up (checked: {checked}): {at:?}"); + } +} diff --git a/crates/ui-egui/src/panels/right.rs b/crates/ui-egui/src/panels/right.rs index dca4f12d3..2454e5f67 100644 --- a/crates/ui-egui/src/panels/right.rs +++ b/crates/ui-egui/src/panels/right.rs @@ -486,14 +486,14 @@ fn header(app: &mut FigApp, ui: &mut Ui, doc: &Document, sel: &[NodeId]) { // Each entry leads with its operation's icon (the result it previews). let key = crate::shortcuts::shortcut(c).unwrap_or(""); let icon = crate::layers::op_icon(op); - let picked = menu_icon_item(ui, icon, current.map(|cur| cur == op), tr(op.label()), key); + let picked = menu_icon_item(ui, Some(icon), current.map(|cur| cur == op), tr(op.label()), key); if picked { app.run(c, json!({})); ui.close(); } } let key = crate::shortcuts::shortcut("vector.flatten").unwrap_or(""); - let flatten = if current.is_some() { menu_item_beside_checks(ui, tr("Flatten"), key) } else { menu_item(ui, tr("Flatten"), key) }; + let flatten = menu_icon_item(ui, None, None, tr("Flatten"), key); if flatten { app.run("vector.flatten", json!({})); ui.close(); diff --git a/crates/ui-egui/src/panels/right/menu.rs b/crates/ui-egui/src/panels/right/menu.rs index ed03f5107..9b80ab8d8 100644 --- a/crates/ui-egui/src/panels/right/menu.rs +++ b/crates/ui-egui/src/panels/right/menu.rs @@ -163,14 +163,25 @@ pub(crate) fn block(ui: &mut Ui, h: f32, inset_y: f32, add: impl FnOnce(&mut Ui) r } -/// A menu item led by `icon` (16 px, in the row's text colour), with a check mark when `on` is -/// `Some(true)` (the unchecked ones line up with it): the boolean operations in the Design panel. -pub fn menu_icon_item(ui: &mut Ui, icon: crate::icons::Icon, on: Option, label: &str, shortcut: &str) -> bool { - let shown = format!("{}{label}", if on == Some(true) { "✓ " } else { " " }); +/// Width of the check column of [`menu_icon_item`] (the mark, or nothing, so labels line up). +const CHECK_COL: f32 = 14.0; + +/// A menu item in a menu whose entries lead with a 16 px icon (in the row's text colour): the +/// boolean operations in the Design panel. `icon` is `None` for an entry without one (Flatten): +/// its slot stays empty, so every label starts at the same x. The check mark (`on` is +/// `Some(true)`) is drawn in its own column ahead of the icon, so a checked entry's label does +/// not move against the unchecked ones. +pub fn menu_icon_item(ui: &mut Ui, icon: Option, on: Option, label: &str, shortcut: &str) -> bool { let paint = |p: &egui::Painter, r: Rect, col: Color32| { - crate::icons::paint(p, Rect::from_min_size(pos2(r.min.x + MENU_PAD, r.center().y - 8.0), vec2(16.0, 16.0)), icon, col); + if on == Some(true) { + let mark = p.layout_no_wrap("✓".to_owned(), regular(11.5), col); + p.galley(pos2(r.min.x + MENU_PAD, r.center().y - mark.size().y / 2.0), mark, col); + } + if let Some(icon) = icon { + crate::icons::paint(p, Rect::from_min_size(pos2(r.min.x + MENU_PAD + CHECK_COL, r.center().y - 8.0), vec2(16.0, 16.0)), icon, col); + } }; - let resp = lit_row(ui, &shown, shortcut, 0.0, 22.0, paint, false); + let resp = lit_row(ui, label, shortcut, 0.0, CHECK_COL + 22.0, paint, false); a11y::menu_item(&resp, label, shortcut, on); resp.clicked() }