From 7d29b2e6e3b813868d291dca64c96a397ba041c4 Mon Sep 17 00:00:00 2001 From: Jason Lee Date: Fri, 18 Sep 2026 18:13:17 +0800 Subject: [PATCH] sidebar: Make every menu item a tab stop with a focus ring `SidebarMenuItem` rendered a plain `div` with `on_click`, so nothing in the sidebar subtree tracked focus: Tab could not reach a single item, and a keyboard user had no way to open a page. A downstream app had to rebuild the item on `base::Button` for that one reason. The row now owns a keyed focus handle the way `Button` does. It is a tab stop by default, draws the framework focus ring while focused, and activates on Enter or Space through GPUI's keyboard click. A pointer press does not move focus, so the ring only shows during keyboard navigation. Disabled items stay out of the traversal. `tab_stop`, `tab_index` and `FocusableExt::focus_ring` are exposed for the cases that need to opt out. The row's `overflow_x_hidden` goes away because it would clip the ring; the label containers already clip. Co-Authored-By: Claude Opus 5 (1M context) --- crates/component/src/sidebar/menu.rs | 152 ++++++++++++++++++++++++++- website/component/sidebar.md | 23 ++++ website/zh-CN/component/sidebar.md | 21 ++++ 3 files changed, 191 insertions(+), 5 deletions(-) diff --git a/crates/component/src/sidebar/menu.rs b/crates/component/src/sidebar/menu.rs index 7590da8590..3e6d16f00e 100644 --- a/crates/component/src/sidebar/menu.rs +++ b/crates/component/src/sidebar/menu.rs @@ -1,5 +1,6 @@ use crate::{ - ActiveTheme as _, Collapsible, Icon, IconName, Placement, Sizable as _, StyledExt, + ActiveTheme as _, Collapsible, FocusableExt, Icon, IconName, Placement, Sizable as _, + StyledExt, ThemeStyled as _, button::{Button, ButtonVariants as _}, h_flex, menu::{ContextMenuExt, PopupMenu}, @@ -8,9 +9,9 @@ use crate::{ v_flex, }; use gpui::{ - AnyElement, App, ClickEvent, ElementId, InteractiveElement as _, IntoElement, - ParentElement as _, SharedString, StatefulInteractiveElement as _, StyleRefinement, Styled, - Window, div, percentage, prelude::FluentBuilder, + AnyElement, App, ClickEvent, ElementId, InteractiveElement as _, IntoElement, MouseButton, + ParentElement as _, Role, SharedString, StatefulInteractiveElement as _, StyleRefinement, + Styled, Window, div, percentage, prelude::FluentBuilder, }; use gpui_base::TestSupportExt as _; use std::rc::Rc; @@ -106,6 +107,9 @@ pub struct SidebarMenuItem { suffix: Option AnyElement + 'static>>, disabled: bool, context_menu: Option PopupMenu + 'static>>, + focus_ring_enabled: bool, + tab_index: isize, + tab_stop: bool, } impl SidebarMenuItem { @@ -126,6 +130,9 @@ impl SidebarMenuItem { suffix: None, disabled: false, context_menu: None, + focus_ring_enabled: true, + tab_index: 0, + tab_stop: true, } } @@ -213,6 +220,23 @@ impl SidebarMenuItem { self } + /// Set the tab index of the menu item, used to order it in keyboard focus traversal. + /// + /// Default is `0`. + pub fn tab_index(mut self, tab_index: isize) -> Self { + self.tab_index = tab_index; + self + } + + /// Set whether the menu item is a tab stop, so the Tab key can reach it. + /// + /// Default is `true`. A focused item activates on Enter or Space, the same + /// as a [`Button`]. + pub fn tab_stop(mut self, tab_stop: bool) -> Self { + self.tab_stop = tab_stop; + self + } + fn is_submenu(&self) -> bool { self.children.len() > 0 } @@ -233,6 +257,17 @@ impl SidebarMenuItem { impl FluentBuilder for SidebarMenuItem {} +impl FocusableExt for SidebarMenuItem { + fn focus_ring(mut self, enabled: bool) -> Self { + self.focus_ring_enabled = enabled; + self + } + + fn is_focus_ring_enabled(&self) -> bool { + self.focus_ring_enabled + } +} + impl Collapsible for SidebarMenuItem { fn is_collapsed(&self) -> bool { self.collapsed @@ -270,6 +305,16 @@ impl SidebarItem for SidebarMenuItem { let is_open = open_state .as_ref() .map_or(false, |s| !is_collapsed && *s.read(cx)); + // The row owns keyboard focus the way `Button` does: a keyed handle that + // survives re-renders, Enter and Space delivered as a keyboard click. + let focus_handle = window + .use_keyed_state((id.clone(), "focus"), cx, |_, cx| cx.focus_handle()) + .read(cx) + .clone(); + let is_focused = focus_handle.is_focused(window); + let show_focus_ring = is_focused && self.focus_ring_enabled; + let tab_index = self.tab_index; + let tab_stop = self.tab_stop; div() .id(id.clone()) @@ -279,13 +324,22 @@ impl SidebarItem for SidebarMenuItem { h_flex() .size_full() .id("item") - .overflow_x_hidden() .flex_shrink_0() .p_2() .gap_x_2() .rounded(cx.theme().radius) .text_sm() .refine_style(&self.style) + .role(Role::Button) + .aria_label(self.label.clone()) + .when(!is_disabled, |this| { + this.track_focus(&focus_handle.tab_index(tab_index).tab_stop(tab_stop)) + .on_mouse_down(MouseButton::Left, |_, window, _| { + // Keep the focus ring for keyboard navigation, as + // `Button` does: a pointer press does not focus. + window.prevent_default(); + }) + }) .when(is_hoverable, |this| { this.hover(|this| { this.bg(cx.theme().sidebar_accent.opacity(0.8)) @@ -351,6 +405,7 @@ impl SidebarItem for SidebarMenuItem { .when(is_disabled, |this| { this.text_color(cx.theme().muted_foreground) }) + .when(show_focus_ring, |this| this.focus_ring_style(window, cx)) .when(!is_disabled, |this| { this.on_click({ let open_state = open_state.clone(); @@ -421,7 +476,94 @@ impl Styled for SidebarMenuItem { #[cfg(test)] mod tests { + use std::{cell::RefCell, rc::Rc}; + + use gpui::{ + Context, KeyDownEvent, KeyUpEvent, Keystroke, Render, TestAppContext, VisualTestContext, + }; + use super::*; + use crate::sidebar::{Sidebar, SidebarGroup}; + + /// Which items a keyboard user can reach, and which one Enter activates. + struct MenuHarness { + clicks: Rc>>, + } + + impl Render for MenuHarness { + fn render(&mut self, _: &mut Window, _: &mut Context) -> impl IntoElement { + let item = |label: &'static str| { + let clicks = self.clicks.clone(); + SidebarMenuItem::new(label).on_click(move |_, _, _| clicks.borrow_mut().push(label)) + }; + + Sidebar::new("sidebar").child( + SidebarGroup::new("Workspace").child( + SidebarMenu::new() + .child(item("Inbox")) + .child(item("Archive").disable(true)) + .child(item("Drafts").tab_stop(false)) + .child(item("Sent")), + ), + ) + } + } + + fn harness( + cx: &mut TestAppContext, + ) -> (&mut VisualTestContext, Rc>>) { + cx.update(crate::init); + let clicks = Rc::new(RefCell::new(Vec::new())); + let (_, cx) = cx.add_window_view({ + let clicks = clicks.clone(); + move |_, _| MenuHarness { clicks } + }); + cx.update(|window, cx| window.draw(cx).clear(cx)); + (cx, clicks) + } + + fn activate_key(cx: &mut VisualTestContext, key: &str) { + let keystroke = Keystroke::parse(key).unwrap(); + cx.simulate_event(KeyDownEvent { + keystroke: keystroke.clone(), + is_held: false, + prefer_character_input: false, + }); + cx.simulate_event(KeyUpEvent { keystroke }); + } + + fn focus_next_and_activate(cx: &mut VisualTestContext) { + cx.update(|window, cx| window.focus_next(cx)); + cx.update(|window, cx| window.draw(cx).clear(cx)); + activate_key(cx, "enter"); + } + + /// Tab walks the enabled tab stops in order and Enter activates the focused + /// item; a disabled item and a `tab_stop(false)` item are skipped. + #[gpui::test] + fn tab_reaches_each_enabled_item_and_enter_activates_it(cx: &mut TestAppContext) { + let (cx, clicks) = harness(cx); + cx.update(|window, cx| assert!(window.focused(cx).is_none())); + + focus_next_and_activate(cx); + focus_next_and_activate(cx); + // Tab wraps: the third stop is the first item again. + focus_next_and_activate(cx); + + assert_eq!(*clicks.borrow(), ["Inbox", "Sent", "Inbox"]); + } + + /// Space activates a focused item the same way Enter does. + #[gpui::test] + fn space_activates_the_focused_item(cx: &mut TestAppContext) { + let (cx, clicks) = harness(cx); + cx.update(|window, cx| window.focus_next(cx)); + cx.update(|window, cx| window.draw(cx).clear(cx)); + + activate_key(cx, "space"); + + assert_eq!(*clicks.borrow(), ["Inbox"]); + } #[test] fn collapsed_icon_item_uses_label_as_tooltip() { diff --git a/website/component/sidebar.md b/website/component/sidebar.md index 6d1b2225c4..d945710e17 100644 --- a/website/component/sidebar.md +++ b/website/component/sidebar.md @@ -231,6 +231,29 @@ SidebarMenu::new() ) ``` +### Keyboard Access + +Every enabled `SidebarMenuItem` is a tab stop: Tab reaches it, a focus ring +marks it, and Enter or Space fires its `on_click`, the same as a `Button`. +A pointer press does not move focus, so the ring only appears during keyboard +navigation. Disabled items are skipped. + +Turn an item's ring off with `focus_ring(false)` when a container clips it +and draws its own; take an item out of the Tab order with `tab_stop(false)`; +reorder with `tab_index`: + +```rust +use gpui_kit::component::FocusableExt as _; + +SidebarMenuItem::new("Trash") + .icon(IconName::Trash) + .tab_stop(false) + +SidebarMenuItem::new("Inbox") + .icon(IconName::Inbox) + .focus_ring(false) +``` + ### Custom Width and Styling ```rust diff --git a/website/zh-CN/component/sidebar.md b/website/zh-CN/component/sidebar.md index 7fd866b425..546febce74 100644 --- a/website/zh-CN/component/sidebar.md +++ b/website/zh-CN/component/sidebar.md @@ -146,6 +146,27 @@ SidebarMenuItem::new("Project Files") }) ``` +### 键盘可达 + +每个未禁用的 `SidebarMenuItem` 都是一个 tab stop：Tab 能走到它，聚焦时画焦点环， +Enter 或 Space 触发 `on_click`，与 `Button` 一致。指针按下不会移动焦点，所以焦点环只在 +键盘导航时出现。禁用的项会被跳过。 + +容器会裁掉焦点环并自己画时，用 `focus_ring(false)` 关掉；`tab_stop(false)` 把某一项从 +Tab 顺序里拿掉；`tab_index` 调整顺序： + +```rust +use gpui_kit::component::FocusableExt as _; + +SidebarMenuItem::new("Trash") + .icon(IconName::Trash) + .tab_stop(false) + +SidebarMenuItem::new("Inbox") + .icon(IconName::Inbox) + .focus_ring(false) +``` + ### 自定义宽度与样式 ```rust