audit menu

crates/core/src/menu.rs

@@ -7,7 +7,7 @@ use gpui::{
     Entity, InteractiveElement as _, IntoElement, KeyDownEvent, MouseButton, MouseDownEvent,
     ParentElement as _, Pixels, Point, RenderOnce, Role, ScrollHandle, SharedString,
     StatefulInteractiveElement as _, Styled as _, Window, anchored, canvas, deferred, div,
-    ease_out_quint, px,
+    ease_out_quint, px, relative,
 };
 use gpui_native_icons::{IconRole, icon as icon_glyph};
 use gpui_native_theme::ActiveTheme as _;
@@ -138,6 +138,9 @@ struct MenuChrome {
     radius_sm: Pixels,
     icon_size: Pixels,
     dense_icon_size: Pixels,
+    menu_item_height: Pixels,
+    spacing_unit: Pixels,
+    text_size: Pixels,
     shadow: Vec<BoxShadow>,
 }
 
@@ -258,6 +261,9 @@ impl RenderOnce for Menu {
                     radius_sm: theme.radius.sm,
                     icon_size: theme.sizes.medium.icon_size,
                     dense_icon_size: theme.sizes.medium.dense_icon_size,
+                    menu_item_height: theme.sizes.medium.menu_item_height,
+                    spacing_unit: theme.spacing.unit,
+                    text_size: theme.sizes.medium.text_size,
                     shadow: theme.elevations.popover.shadows(),
                 },
                 theme.motion.overlay,
@@ -374,7 +380,7 @@ impl RenderOnce for Menu {
                 .aria_label(label)
                 .mt_1()
                 .min_w_56()
-                .p_1()
+                .p(chrome.spacing_unit)
                 .bg(chrome.surface)
                 .text_color(chrome.foreground)
                 .rounded(chrome.radius_overlay)
@@ -454,8 +460,9 @@ fn render_item(
     match item.kind {
         MenuItemKind::Separator => div()
             .h(px(1.))
-            .mx_2()
+            .debug_selector(move || format!("menu-separator-{index}"))
-            .my_1()
+            .mx(chrome.spacing_unit * 2.0)
+            .my(chrome.spacing_unit)
             .bg(chrome.border)
             .into_any_element(),
         MenuItemKind::Action => {
@@ -468,11 +475,14 @@ fn render_item(
 
             div()
                 .id(("menu-item", index))
+                .debug_selector(move || format!("menu-item-{index}"))
                 .flex()
                 .items_center()
                 .gap_2()
-                .px_2()
+                .min_h(chrome.menu_item_height)
-                .py_1()
+                .px(chrome.spacing_unit * 2.0)
+                .text_size(chrome.text_size)
+                .line_height(relative(1.0))
                 .rounded(chrome.radius_sm)
                 .map(|row| {
                     if disabled {

crates/core/tests/menu.rs

@@ -112,6 +112,28 @@ fn click(visual: &mut VisualTestContext, position: gpui::Point<gpui::Pixels>) {
     visual.simulate_mouse_up(position, MouseButton::Left, Modifiers::default());
 }
 
+/// Every action row renders at the platform's stated menu row height,
+/// whatever its content — the metric is the whole row, not a floor under
+/// padding. Item 1 of the harness menu is a separator, so it is skipped.
+#[test]
+fn rows_render_at_the_stated_menu_row_height() {
+    let (cx, mut visual, _window, _changes, _activated) = menu_window(true);
+
+    let stated = cx.update(|cx| Theme::global(cx).sizes.medium.menu_item_height);
+    for index in [0, 2, 3, 4] {
+        let selector: &'static str = Box::leak(format!("menu-item-{index}").into_boxed_str());
+        let row = visual
+            .debug_bounds(selector)
+            .unwrap_or_else(|| panic!("row {index} should have been laid out"));
+        // Painted bounds are snapped to device pixels; allow half a pixel.
+        assert!(
+            (row.size.height - stated).abs() <= px(0.5),
+            "row {index} is {:?} tall, stated menu row height is {stated:?}",
+            row.size.height
+        );
+    }
+}
+
 #[test]
 fn arrow_navigation_skips_separators_and_disabled_items() {
     let (_cx, mut visual, _window, changes, activated) = menu_window(true);

crates/core/tests/select_pointer.rs

@@ -67,25 +67,45 @@ fn click(visual: &mut VisualTestContext, position: gpui::Point<gpui::Pixels>) {
     visual.simulate_mouse_up(position, MouseButton::Left, gpui::Modifiers::default());
 }
 
-/// The trigger band is `16..46`; the panel hangs 4 px below it with a 4 px
+/// The three option rows must answer a pointer press wherever they were laid
-/// inner padding, so the three rows ride `~58..90`, `~90..122`, `~122..154`.
+/// out — a popup that clips its last item never does. Row positions come from
-/// Every option must answer a pointer press — a popup that clips its last
+/// the rendered bounds, so the stated menu row metric can change freely.
-/// item never does.
 #[test]
 fn every_option_row_answers_the_pointer() {
     let (_cx, mut visual, changes) = level_window();
 
-    let expected = [
+    // Open once to measure where the rows were actually drawn.
-        (px(74.), Level::Low),
-        (px(106.), Level::Mid),
-        (px(138.), Level::High),
-    ];
-    for (y, value) in expected {
     click(&mut visual, point(px(100.), px(31.)));
     visual.update(|window, cx| {
         window.draw(cx).clear(cx);
     });
-        click(&mut visual, point(px(100.), y));
+    let centers: Vec<gpui::Point<gpui::Pixels>> = (0..3)
+        .map(|index| {
+            let selector: &'static str = Box::leak(format!("menu-item-{index}").into_boxed_str());
+            let row = visual
+                .debug_bounds(selector)
+                .unwrap_or_else(|| panic!("row {index} should have been laid out"));
+            point(
+                row.left() + row.size.width / 2.0,
+                row.top() + row.size.height / 2.0,
+            )
+        })
+        .collect();
+
+    // Close again; each iteration below opens the popup fresh.
+    click(&mut visual, point(px(100.), px(31.)));
+    visual.update(|window, cx| {
+        window.draw(cx).clear(cx);
+    });
+
+    let expected = [(Level::Low, 0), (Level::Mid, 1), (Level::High, 2)];
+    for (value, index) in expected {
+        // Choosing the previous row closed the popup; reopen it.
+        click(&mut visual, point(px(100.), px(31.)));
+        visual.update(|window, cx| {
+            window.draw(cx).clear(cx);
+        });
+        click(&mut visual, centers[index]);
         visual.update(|window, cx| {
             window.draw(cx).clear(cx);
         });

docs/CHECKLIST.md

@@ -14,9 +14,6 @@ is the first downstream consumer; its screens drive the order.
 Platform metric adoption continues one component at a time; each stop goes
 through the review gate and a gallery pass.
 
-- [ ] **Menu platform chrome** — overlay border gate; paddings, separators,
-      and the row min-height through `Theme::spacing` and the stated menu
-      row height.
 - [ ] **Popover platform chrome** — overlay border gate; the anchor gap
       through `Theme::spacing`.
 - [ ] **Dialog platform chrome** — overlay border gate.
@@ -109,6 +106,11 @@ Gaps surfaced by the signed comparison; ordered by signed's usage.
       macOS / GNOME, so the trigger matches a focused field; the menu
       popup's border sits behind `Theme::borders.overlays`, disappearing
       into the surface where the platform paints none
+- [x] Menu platform chrome — the popup border behind `Theme::borders.overlays`
+      (and dark mode); the panel padding and separator margins come from
+      `Theme::spacing`, and every action row renders at the stated
+      `menu_item_height` with content centered, so the stated metric is the
+      whole row; rows carry debug selectors for headless geometry tests
 - [x] Button, Checkbox, Switch, Radio + RadioGroup, Toggle, Link, Slider,
       Progress, Avatar (image + seeded pixel-art fallback)
 - [x] Spinner — painted ring + rotating-icon variant, indeterminate and