Menus · button rows: a reader presses a button through Click and hears the marked one; the panel is a menu

Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>

AGENTS.md

@@ -88,7 +88,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark.
 - Floating lists go below their anchor, or above when only above has room (`forms::options::float`). gpui's own switch keeps the anchor point, so a flipped list would cover its trigger. An option list is at least its trigger's width and at most the window's less `window_margin` each side, so gpui can snap it inside and its rows end in an ellipsis; wider, gpui pins it left and the rest runs off.
 - A trigger that opens a list goes through `forms::select::listing`: toggle, keys, blur and popup live there, for Select, TabOverflowMenu and Breadcrumb alike. A field that suggests as you type floats its rows through `forms::select::Picker` and `forms::options::Popup`, as Combobox and TagInput do.
 - A palette goes through `navigation::palette::Palette`: query field, grouped rows, cursor, keys and focus live there. CommandPalette, QuickOpen, QuickSwitcher, SearchPalette and QuickLauncher only build rows.
-- A menu goes through `menus::menu`: rows, submenus, keys, focus and presses outside live there, for DropdownMenu, OverflowMenu, SplitButton, ContextMenu, SearchableMenu and MenuBar. A press on the host's own box is the host's to handle, so a trigger toggles and a right click moves a context menu without a close and reopen in one event.
+- A menu goes through `menus::menu`: rows, submenus, keys, focus and presses outside live there, for DropdownMenu, OverflowMenu, SplitButton, ContextMenu, SearchableMenu and MenuBar. A press on the host's own box is the host's to handle, so a trigger toggles and a right click moves a context menu without a close and reopen in one event. A row of icon buttons (`Menu::buttons`) is one entry: Up and Down reach it, Left and Right cross it, a reader presses a button through Click and hears the marked one, and a mark holds its item by label, so a row that changes under it runs nothing unasked.
 - gpui focuses the nearest focusable ancestor on mouse down, and apps wrap everything in a `FocusScope`. A press that must leave focus alone calls `prevent_default`, as buttons and context regions do.
 - gpui runs the click of every pressed element under the pointer, a row's with its button's. A pressable row that holds buttons stops their press at their box on mouse down (`files::transfer`), as `overlays::media_button` stops its own.
 - gpui draws a window's first frame inside `open_window`. State the first frame reads, such as `cx.set_menus`, is set before it opens.

src/menus/draw.rs

@@ -1,6 +1,9 @@
+use std::rc::Rc;
+
 use gpui::{
     AnyElement, App, Div, ElementId, Entity, FontWeight, InteractiveElement, IntoElement,
-    MouseButton, ParentElement, Role, SharedString, Stateful, Styled, div, prelude::*,
+    MouseButton, ParentElement, Role, SharedString, Stateful, Styled, accesskit::Action, div,
+    prelude::*,
 };
 
 use super::{
@@ -86,9 +89,9 @@ fn row(item: &MenuItem, ix: usize, current: bool, cx: &App) -> Stateful<Div> {
 /// Button `column` of row `ix`, named by its label.
 pub(super) fn button(
     item: &MenuItem,
-    ix: usize,
+    (ix, column): (usize, usize),
-    column: usize,
     current: bool,
+    press: Run,
     cx: &App,
 ) -> Stateful<Div> {
     let theme = cx.theme();
@@ -113,11 +116,23 @@ pub(super) fn button(
         .rounded(theme.radius(Radius::Md))
         .border_1()
         .border_color(colors.border)
+        .aria_selected(current)
+        .when(current, |button| button.aria_active_descendant())
         .when(current && !item.disabled, |button| button.bg(colors.hover))
-        .when(!item.disabled, |button| button.cursor_pointer())
         .when(item.disabled, |button| {
             button.aria_description(i18n::text(cx, "state.unavailable", &[]))
         })
+        .when(!item.disabled, |button| {
+            let read = press.clone();
+            button
+                .cursor_pointer()
+                .on_mouse_down(MouseButton::Left, move |_, window, cx| {
+                    window.prevent_default();
+                    cx.stop_propagation();
+                    press(window, cx);
+                })
+                .on_a11y_action(Action::Click, move |_, window, cx| read(window, cx))
+        })
         .child(Icon::new(icon).size(IconSize::Sm).color(fg))
         .tooltip(Tooltip::rich(move |_, cx| {
             div()
@@ -174,20 +189,18 @@ pub(super) fn panel(
                     let (hover, click, close, menu) =
                         (state.clone(), state.clone(), close.clone(), menu.clone());
                     let hover_menu = menu.clone();
-                    button(item, ix, column, current, cx).when(!item.disabled, |button| {
+                    let press: Run = Rc::new(move |window, cx| {
-                        button
+                        choose(&click, &menu, depth, (ix, column), &close, window, cx)
-                            .on_mouse_move(move |_, _, cx| {
+                    });
-                                let open = hover.read(cx);
+                    let drawn = button(item, (ix, column), current, press, cx);
-                                let marked = open.levels.get(depth).map(|l| (l.at, l.column));
+                    drawn.when(!item.disabled, |button| {
-                                if marked != Some((Some(ix), column)) {
+                        button.on_mouse_move(move |_, _, cx| {
-                                    mark(&hover, &hover_menu, depth, (ix, column), false, cx);
+                            let open = hover.read(cx);
-                                }
+                            let marked = open.levels.get(depth).map(|l| (l.at, l.column));
-                            })
+                            if marked != Some((Some(ix), column)) {
-                            .on_mouse_down(MouseButton::Left, move |_, window, cx| {
+                                mark(&hover, &hover_menu, depth, (ix, column), false, cx);
-                                window.prevent_default();
+                            }
-                                cx.stop_propagation();
+                        })
-                                choose(&click, &menu, depth, (ix, column), &close, window, cx);
-                            })
                     })
                 }))
                 .into_any_element(),
@@ -230,6 +243,7 @@ pub(super) fn panel(
             }
         });
     surface((id.clone(), format!("panel-{depth}")), cx)
+        .role(Role::Menu)
         .relative()
         // Presses here are the menu's, not its host's.
         .on_mouse_down(MouseButton::Left, |_, window, cx| {

src/menus/tests/buttons.rs

@@ -214,16 +214,34 @@ fn a_disabled_button_says_it_is_unavailable(cx: &mut TestAppContext) {
         Theme::init(cx);
         let off = MenuItem::new("Copy").icon(IconName::Copy).disabled(true);
         let on = MenuItem::new("Cut").icon(IconName::Scissors);
-        let read = |item: &MenuItem, cx: &mut App| {
+        let read =
-            let mut node = gpui::accesskit::Node::new(gpui::Role::Button);
+            |item: &MenuItem, cx: &mut App| node(item, false, cx).description().map(str::to_owned);
-            gpui::Element::write_a11y_info(
-                &super::super::draw::button(item, 0, 0, false, cx),
-                &mut node,
-            );
-            node.description().map(str::to_owned)
-        };
         let expected = crate::i18n::text(cx, "state.unavailable", &[]).to_string();
         assert_eq!(read(&off, cx), Some(expected));
         assert_eq!(read(&on, cx), None);
     });
 }
+
+/// The accessibility node `item` draws as a menu button.
+fn node(item: &MenuItem, current: bool, cx: &mut App) -> gpui::accesskit::Node {
+    let mut node = gpui::accesskit::Node::new(gpui::Role::Button);
+    let press: crate::forms::Run = std::rc::Rc::new(|_, _| {});
+    let drawn = super::super::draw::button(item, (0, 0), current, press, cx);
+    gpui::Element::write_a11y_info(&drawn, &mut node);
+    node
+}
+
+#[gpui::test]
+fn a_button_offers_its_press_and_says_it_is_marked(cx: &mut TestAppContext) {
+    cx.update(|cx| {
+        Theme::init(cx);
+        let cut = MenuItem::new("Cut").icon(IconName::Scissors);
+        let off = MenuItem::new("Copy").icon(IconName::Copy).disabled(true);
+        let click = gpui::accesskit::Action::Click;
+        let marked = node(&cut, true, cx);
+        assert!(marked.supports_action(click), "a reader can press it");
+        assert_eq!(marked.is_selected(), Some(true));
+        assert_eq!(node(&cut, false, cx).is_selected(), Some(false));
+        assert!(!node(&off, false, cx).supports_action(click), "off");
+    });
+}