From 0dbb8c747c42701b5bcf7183a2ba9cb153b72d17 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=E9=9B=B7=E7=94=B5=E8=8A=BD=E8=A1=A3?= Date: Sat, 3 Oct 2026 01:35:48 -0400 Subject: [PATCH] =?UTF-8?q?Forms,=20a11y=20=C2=B7=20Checkbox,=20Switch,=20?= =?UTF-8?q?Radio,=20RadioGroup,=20Slider=20and=20Select=20name=20their=20r?= =?UTF-8?q?ole,=20label=20and=20state=20(#3)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A slider's thumb reports its value, the bounds it can reach and its orientation; a select's trigger its value, placeholder and whether its list is open; a disabled control says so in its description. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 1 + src/forms/check.rs | 17 ++++++++++++++--- src/forms/radio.rs | 13 ++++++++++--- src/forms/select.rs | 12 +++++++++++- src/forms/slider.rs | 26 ++++++++++++++++++++++++-- src/forms/switch.rs | 10 ++++++++-- tasks/github.md | 2 +- tasks/reviews.md | 2 ++ 8 files changed, 71 insertions(+), 12 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 8c3b6b6..621030c 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -92,6 +92,7 @@ Ely GPUI Component. A component library for GPUI, in light and dark. - A focus handle a component hands back lives across all its modes: take it with `tab_stop` before any early return, or keyed state drops it while the other mode draws and the handed-back focus lands nowhere. A thread's message keeps one handle folded and open, a Tab stop only while folded (`mail::MailThreadView`). - A button whose words and action change with a mode keeps one handle through `Button::focus_handle`, and the component moves focus to it or back to its field as the mode turns (`project::TimeTracker`). - A button that waits on the owner shows it with `loading`, which takes no press and stays a Tab stop, so focus holds through the wait; `disabled` is for what is not ready, and a disabled button is no stop (`onboarding::ContactSupport`). +- A control names its role, label and state on the element that takes focus, for gpui's AccessKit tree (a slider's thumb, a select's trigger). gpui sets no disabled flag, so a disabled control says `state.unavailable` in its description. A thumb bounded by another reports the bounds it can reach. The test platform builds no tree; a live window read through UI Automation, or `Window::debug_a11y_tree_json` while a client is connected, proves it. - A help mark, ?, is `primitives::HelpTooltip`; FormLabel's help shows it. - A panel that opens from something other than a button goes through `overlays::Popover::with_opener`, which hands the opener its toggle; focus, placement and every way out stay in the popover (`calendar::EventPopover`'s chip). Its content gets a close that hands focus back: a press that ends the panel's work closes it before the owner acts, or a panel whose host leaves the tree strands focus (`onboarding::Hotspot`'s Got it, EventPopover's Edit and Delete). - A flow in steps goes through `navigation::Wizard`: Back, Next, Finish and the step's motion live there. Next and Finish share one focus handle, and focus moves to it when Back reaches the first step or a finished step is picked. `onboarding::OnboardingWizard` leaves out its numbered steps (`headless`) and shows a count over a bar in as many parts. diff --git a/src/forms/check.rs b/src/forms/check.rs index 93a8865..6125f63 100644 --- a/src/forms/check.rs +++ b/src/forms/check.rs @@ -2,8 +2,8 @@ use std::rc::Rc; use gpui::{ Animation, AnimationExt, AnyElement, App, ElementId, InteractiveElement, IntoElement, - MouseButton, ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, - Window, div, prelude::*, + MouseButton, ParentElement, RenderOnce, Role, SharedString, StatefulInteractiveElement, Styled, + Toggled, Window, div, prelude::*, }; use super::{ @@ -11,7 +11,7 @@ use super::{ options::{OnFlag, OnValues}, }; use crate::{ - motion, + i18n, motion, primitives::{Icon, IconName, tab_stop}, theme::{ActiveTheme, IconSize, Mix, Radius, TextSize}, }; @@ -152,8 +152,19 @@ impl RenderOnce for Checkbox { } else { theme.colors.fg }; + let toggled = match self.state { + CheckState::Off => Toggled::False, + CheckState::On => Toggled::True, + CheckState::Mixed => Toggled::Mixed, + }; div() .id(self.id) + .role(Role::CheckBox) + .aria_toggled(toggled) + .when_some(self.label.clone(), |row, label| row.aria_label(label)) + .when(self.disabled, |row| { + row.aria_description(i18n::text(cx, "state.unavailable", &[])) + }) .track_focus(&focus) .flex() .items_center() diff --git a/src/forms/radio.rs b/src/forms/radio.rs index c1535d6..f934b13 100644 --- a/src/forms/radio.rs +++ b/src/forms/radio.rs @@ -2,8 +2,8 @@ use std::rc::Rc; use gpui::{ Animation, AnimationExt, AnyElement, App, ElementId, FocusHandle, InteractiveElement, - IntoElement, MouseButton, ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, - Styled, Window, div, prelude::*, + IntoElement, MouseButton, ParentElement, RenderOnce, Role, SharedString, + StatefulInteractiveElement, Styled, Window, div, prelude::*, }; use super::{ @@ -11,7 +11,7 @@ use super::{ options::{OnValue, Run, step}, }; use crate::{ - motion, + i18n, motion, primitives::tab_stop, theme::{ActiveTheme, TextSize}, }; @@ -127,6 +127,12 @@ impl RenderOnce for Radio { }; div() .id(self.id) + .role(Role::RadioButton) + .aria_toggled(on.into()) + .when_some(self.label.clone(), |row, label| row.aria_label(label)) + .when(self.disabled, |row| { + row.aria_description(i18n::text(cx, "state.unavailable", &[])) + }) .track_focus(&focus) .flex() .items_center() @@ -242,6 +248,7 @@ impl RenderOnce for RadioGroup { .collect(); div() .id(self.id) + .role(Role::RadioGroup) .flex() .when(self.horizontal, |group| group.flex_row().gap_4()) .when(!self.horizontal, |group| group.flex_col().gap_2()) diff --git a/src/forms/select.rs b/src/forms/select.rs index 804eb7e..df0c1ba 100644 --- a/src/forms/select.rs +++ b/src/forms/select.rs @@ -2,7 +2,7 @@ use std::rc::Rc; use gpui::{ App, Bounds, Div, ElementId, Entity, FocusHandle, InteractiveElement, IntoElement, - KeyDownEvent, MouseButton, ParentElement, Pixels, RenderOnce, ScrollHandle, SharedString, + KeyDownEvent, MouseButton, ParentElement, Pixels, RenderOnce, Role, ScrollHandle, SharedString, Stateful, Styled, Window, canvas, div, prelude::*, }; @@ -13,6 +13,7 @@ use super::{ }; use crate::{ data_display::Avatar, + i18n, primitives::{Icon, IconName, tab_stop}, theme::{ActiveTheme, AvatarSize, ControlSize, IconSize, Radius}, typography::Ellipsis, @@ -197,6 +198,7 @@ pub(crate) fn listing( pick.clone(), ); trigger + .aria_expanded(open) .on_mouse_down(MouseButton::Left, move |_, _, cx| { Picker::show(&toggle, !open, start, cx) }) @@ -325,6 +327,14 @@ impl RenderOnce for Select { window, cx, ) + .role(Role::ComboBox) + .aria_placeholder(self.placeholder.clone()) + .when_some(shown.as_ref(), |trigger, choice| { + trigger.aria_value(choice.label.clone()) + }) + .when(self.disabled, |trigger| { + trigger.aria_description(i18n::text(cx, "state.unavailable", &[])) + }) .when_some( shown.as_ref().and_then(|choice| choice.icon), |trigger, icon| { diff --git a/src/forms/slider.rs b/src/forms/slider.rs index 796c0e1..2efdac4 100644 --- a/src/forms/slider.rs +++ b/src/forms/slider.rs @@ -2,12 +2,13 @@ use std::rc::Rc; use gpui::{ AnyElement, App, Bounds, DragMoveEvent, ElementId, EmptyView, EntityId, InteractiveElement, - IntoElement, MouseButton, ParentElement, Pixels, Point, RenderOnce, StatefulInteractiveElement, - Styled, Window, canvas, div, prelude::*, relative, + IntoElement, MouseButton, Orientation, ParentElement, Pixels, Point, RenderOnce, Role, + StatefulInteractiveElement, Styled, Window, canvas, div, prelude::*, relative, }; use super::options::OnNumber; use crate::{ + i18n, primitives::tab_stop, theme::{ActiveTheme, Elevation}, }; @@ -86,6 +87,13 @@ impl Track { ) }) .collect(); + let unavailable = self + .disabled + .then(|| i18n::text(cx, "state.unavailable", &[])); + let orientation = match vertical { + true => Orientation::Vertical, + false => Orientation::Horizontal, + }; let theme = cx.theme(); let colors = &theme.colors; let (thumb, line) = (theme.slider_thumb(), theme.slider_track()); @@ -143,8 +151,22 @@ impl Track { .map(|(k, at)| { let focused = handles[k].is_focused(window); let (commit, value) = (self.commit.clone(), self.values[k]); + let (floor, ceiling) = match (self.values.as_slice(), k) { + ([_, high], 0) => (min, *high), + ([low, _], _) => (*low, max), + _ => (min, max), + }; div() .id(("thumb", k)) + .role(Role::Slider) + .aria_numeric_value(value) + .aria_min_numeric_value(floor) + .aria_max_numeric_value(ceiling) + .aria_numeric_value_step(step) + .aria_orientation(orientation) + .when_some(unavailable.clone(), |thumb, text| { + thumb.aria_description(text) + }) .track_focus(&handles[k]) .absolute() .size(thumb) diff --git a/src/forms/switch.rs b/src/forms/switch.rs index 5da6e96..113933b 100644 --- a/src/forms/switch.rs +++ b/src/forms/switch.rs @@ -2,13 +2,13 @@ use std::rc::Rc; use gpui::{ Animation, AnimationExt, App, ElementId, InteractiveElement, IntoElement, MouseButton, - ParentElement, RenderOnce, SharedString, StatefulInteractiveElement, Styled, Window, div, + ParentElement, RenderOnce, Role, SharedString, StatefulInteractiveElement, Styled, Window, div, prelude::*, transparent_black, }; use super::options::OnFlag; use crate::{ - motion, + i18n, motion, primitives::tab_stop, theme::{ActiveTheme, Elevation, Mix, TextSize}, }; @@ -119,6 +119,12 @@ impl RenderOnce for Switch { let (id, next, on_change) = (self.id.clone(), !self.on, self.on_change); div() .id(self.id) + .role(Role::Switch) + .aria_toggled(self.on.into()) + .when_some(self.label.clone(), |row, label| row.aria_label(label)) + .when(self.disabled, |row| { + row.aria_description(i18n::text(cx, "state.unavailable", &[])) + }) .track_focus(&focus) .flex() .items_center() diff --git a/tasks/github.md b/tasks/github.md index 02513f5..3e3a3ff 100644 --- a/tasks/github.md +++ b/tasks/github.md @@ -2,5 +2,5 @@ - [x] G1 PR #1: build on Windows with `escape_args`; review, verify, merge, answer CodeRabbit - [x] G2 Issue #2: tree guides line up under the chevron and run unbroken across rows -- [ ] G3 Issue #3: Checkbox, Switch, Radio, Slider and Select name their role, label and state +- [x] G3 Issue #3: Checkbox, Switch, Radio, Slider and Select name their role, label and state - [ ] G4 Push, comment on #2 and #3, close #2 diff --git a/tasks/reviews.md b/tasks/reviews.md index f5ffece..ca278d4 100644 --- a/tasks/reviews.md +++ b/tasks/reviews.md @@ -349,3 +349,5 @@ | T48m | 3 | PASS | | | G1 | 1 | PASS | PR #1 merged; CodeRabbit's opt-out declined: Launch is argv | | G2 | 1 | PASS | | +| G3 | 1 | FAIL | RangeSlider thumbs reported the whole track, not the bounds the other thumb leaves | +| G3 | 2 | PASS | |