From 1b8d67839abb95ebf9eb948ab63aa7de45faa6bc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Tue, 13 Oct 2026 21:25:18 -0400 Subject: [PATCH] =?UTF-8?q?Menus=20=C2=B7=20button=20rows:=20a=20reader=20?= =?UTF-8?q?presses=20a=20button=20through=20Click=20and=20hears=20the=20ma?= =?UTF-8?q?rked=20one;=20the=20panel=20is=20a=20menu?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Opus 5.5 (1M context) --- AGENTS.md | 2 +- src/menus/draw.rs | 50 ++++++++++++++++++++++++-------------- src/menus/tests/buttons.rs | 34 ++++++++++++++++++++------ 3 files changed, 59 insertions(+), 27 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index fb1ba46..ae687a5 100644 --- a/AGENTS.md +++ b/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. diff --git a/src/menus/draw.rs b/src/menus/draw.rs index cdf3918..b9cc473 100644 --- a/src/menus/draw.rs +++ b/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
{ /// Button `column` of row `ix`, named by its label. pub(super) fn button( item: &MenuItem, - ix: usize, - column: usize, + (ix, column): (usize, usize), current: bool, + press: Run, cx: &App, ) -> Stateful
{ 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| { - button - .on_mouse_move(move |_, _, cx| { - let open = hover.read(cx); - let marked = open.levels.get(depth).map(|l| (l.at, l.column)); - if marked != Some((Some(ix), column)) { - mark(&hover, &hover_menu, depth, (ix, column), false, cx); - } - }) - .on_mouse_down(MouseButton::Left, move |_, window, cx| { - window.prevent_default(); - cx.stop_propagation(); - choose(&click, &menu, depth, (ix, column), &close, window, cx); - }) + let press: Run = Rc::new(move |window, cx| { + choose(&click, &menu, depth, (ix, column), &close, window, cx) + }); + let drawn = button(item, (ix, column), current, press, cx); + drawn.when(!item.disabled, |button| { + button.on_mouse_move(move |_, _, cx| { + let open = hover.read(cx); + let marked = open.levels.get(depth).map(|l| (l.at, l.column)); + if marked != Some((Some(ix), column)) { + mark(&hover, &hover_menu, depth, (ix, column), false, 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| { diff --git a/src/menus/tests/buttons.rs b/src/menus/tests/buttons.rs index 4e1bf5d..d911e37 100644 --- a/src/menus/tests/buttons.rs +++ b/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 mut node = gpui::accesskit::Node::new(gpui::Role::Button); - gpui::Element::write_a11y_info( - &super::super::draw::button(item, 0, 0, false, cx), - &mut node, - ); - node.description().map(str::to_owned) - }; + let read = + |item: &MenuItem, cx: &mut App| node(item, false, cx).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"); + }); +}