From 22a439edb7a8c33a2b0392e3365deaca2ccdd36e Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 22 Oct 2026 14:31:18 -0700 Subject: [PATCH] Color Wheel: screen readers' actions move the square's saturation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The saturation and brightness square, and the Color Picker's color field it shares `widgets::color_field` with, gave screen readers its name and both values but took none of their actions, so an increment, decrement or set-value on it left the color as it was. The field is now a slider over its first component (saturation on the Color Wheel), from 0 to that component's units with a step of 1, as the hue ring is from 0 to 360. Increment and decrement move it one unit as → and ← do, and set-value sets it; the second component stays where it is. Without a marker the value reads 0 and a set value still applies, as on the ring. `screen_readers_read_and_move_the_ring_and_the_square` checks the square's range, step and actions, an increment, a set value and a decrement, and `the_color_pickers_field_takes_the_arrow_keys` checks an increment on the Color Picker's field. --- .../ui-egui/src/panels/tests_color_wheel.rs | 25 ++++++++++++-- crates/ui-egui/src/widgets/color.rs | 34 +++++++++++++++---- 2 files changed, 50 insertions(+), 9 deletions(-) diff --git a/crates/ui-egui/src/panels/tests_color_wheel.rs b/crates/ui-egui/src/panels/tests_color_wheel.rs index 61a9530a3..8574a2900 100644 --- a/crates/ui-egui/src/panels/tests_color_wheel.rs +++ b/crates/ui-egui/src/panels/tests_color_wheel.rs @@ -394,7 +394,7 @@ fn the_arrow_keys_step_the_focused_ring_or_square() { } /// Screen readers find the ring as a slider from 0 to 360 that their actions move, and the square -/// with both of its components. +/// with both of its components, whose actions move its saturation as → and ← do. #[test] fn screen_readers_read_and_move_the_ring_and_the_square() { let mut h = harness(app(json!([0.6, 0.3, 0.3]))); @@ -412,10 +412,26 @@ fn screen_readers_read_and_move_the_ring_and_the_square() { h.event(Event::AccessKitActionRequest(ActionRequest { target_node, target_tree, action: Action::SetValue, data })); h.run_steps(1); assert!(near(hsb(&fill(h.state())), [200.0, 0.5, 0.6], [1e-3, 1e-3, 1e-3]), "{:?}", hsb(&fill(h.state()))); + let square = h.get_by_label("Saturation and brightness"); + let node = square.accesskit_node(); + assert_eq!((node.min_numeric_value(), node.max_numeric_value(), node.numeric_value_step()), (Some(0.0), Some(100.0), Some(1.0))); + assert!([Action::Increment, Action::Decrement, Action::SetValue].iter().all(|a| node.data().supports_action(*a))); + assert!(node.numeric_value().is_some_and(|v| (v - 50.0).abs() < 1e-3), "{:?}", node.numeric_value()); + let (target_node, target_tree) = node.locate(); + h.event(Event::AccessKitActionRequest(ActionRequest { target_node, target_tree, action: Action::Increment, data: None })); + h.run_steps(1); + assert!(near(hsb(&fill(h.state())), [200.0, 0.51, 0.6], [1e-3, 1e-3, 1e-3]), "{:?}", hsb(&fill(h.state()))); + let data = Some(ActionData::NumericValue(20.0)); + h.event(Event::AccessKitActionRequest(ActionRequest { target_node, target_tree, action: Action::SetValue, data })); + h.run_steps(1); + assert!(near(hsb(&fill(h.state())), [200.0, 0.2, 0.6], [1e-3, 1e-3, 1e-3]), "{:?}", hsb(&fill(h.state()))); + h.event(Event::AccessKitActionRequest(ActionRequest { target_node, target_tree, action: Action::Decrement, data: None })); + h.run_steps(1); + assert!(near(hsb(&fill(h.state())), [200.0, 0.19, 0.6], [1e-3, 1e-3, 1e-3]), "{:?}", hsb(&fill(h.state()))); } /// The Color Picker's color field is the same widget: a press focuses it, the arrows step the -/// components it shows, and screen readers read its name and both components. +/// components it shows, and screen readers read its name and both components and step the first. #[test] fn the_color_pickers_field_takes_the_arrow_keys() { let mut app = app(json!([0.6, 0.3, 0.3])); @@ -445,5 +461,10 @@ fn the_color_pickers_field_takes_the_arrow_keys() { key(&mut h, egui::Key::ArrowDown, Modifiers::SHIFT); let after = picked(&h); assert!(near(after, [before[0], before[1] + 0.01, before[2] - 0.1], [1e-3, 1e-4, 1e-4]), "{before:?} → {after:?}"); + let (target_node, target_tree) = h.get_by_label("Color field").accesskit_node().locate(); + h.event(Event::AccessKitActionRequest(ActionRequest { target_node, target_tree, action: Action::Increment, data: None })); + h.run_steps(1); + let stepped = picked(&h); + assert!(near(stepped, [after[0], after[1] + 0.01, after[2]], [1e-3, 1e-4, 1e-4]), "{after:?} → {stepped:?}"); assert!(h.state().ui.dialog.is_some(), "the dialog stays open"); } diff --git a/crates/ui-egui/src/widgets/color.rs b/crates/ui-egui/src/widgets/color.rs index 6d1ac43ab..159f33a49 100644 --- a/crates/ui-egui/src/widgets/color.rs +++ b/crates/ui-egui/src/widgets/color.rs @@ -375,8 +375,10 @@ fn focus_outline(ui: &Ui, resp: &Response, rect: Rect) { /// the color at a point (0..1 each, y down) and, when `marked`, a ring marks `pos`. A press gives /// it the keyboard focus. While it has the focus, → and ← move `pos` across by one unit of /// `axes[0]` and ↑ and ↓ move it up and down by one unit of `axes[1]`, marked or not. Screen -/// readers read `name` and, when `marked`, both components. Returns the clicked or dragged point, -/// or the point the keys moved to, and the phase. +/// readers read `name` and, when `marked`, both components. They read it as a slider over +/// `axes[0]`'s units (0 when not `marked`), which their increment and decrement actions move as → +/// and ← do and their set-value action sets, as for a [`hue_ring`]. Returns the clicked or dragged +/// point, or the point the keys or actions moved to, and the phase. pub fn color_field( ui: &mut Ui, id: impl std::hash::Hash, @@ -386,6 +388,7 @@ pub fn color_field( marked: bool, color_at: &dyn Fn(f32, f32) -> Color32, ) -> (Option<(f32, f32)>, Live) { + use egui::accesskit::{Action, ActionData}; let t = Tokens::get(ui.ctx()); let (rect, _) = ui.allocate_exact_size(size, Sense::hover()); let resp = ui.interact(rect, id_with(ui.scope_id(), id), Sense::click_and_drag()); @@ -407,17 +410,34 @@ pub fn color_field( &[("name1", axes[0].name), ("value1", &value(x, &axes[0])), ("name2", axes[1].name), ("value2", &value(1.0 - y, &axes[1]))], ) }); - resp.widget_info(|| egui::WidgetInfo { - current_text_value: text.clone(), - ..egui::WidgetInfo::labeled(egui::WidgetType::Slider, ui.is_enabled(), name) + let shown = if marked { f64::from(x.clamp(0.0, 1.0) * axes[0].units) } else { 0.0 }; + resp.widget_info(|| egui::WidgetInfo { current_text_value: text.clone(), ..egui::WidgetInfo::slider(ui.is_enabled(), shown, name) }); + ui.ctx().accesskit_node_builder(resp.id, |node| { + node.set_min_numeric_value(0.0); + node.set_max_numeric_value(f64::from(axes[0].units)); + node.set_numeric_value_step(1.0); + for action in [Action::SetValue, Action::Increment, Action::Decrement] { + node.add_action(action); + } }); let (dx, dy) = arrow_steps(ui, resp.id, true, true); + let (steps, set) = ui.input(|i| { + let steps = + i.num_accesskit_action_requests(resp.id, Action::Increment) as f32 - i.num_accesskit_action_requests(resp.id, Action::Decrement) as f32; + let set = i.accesskit_action_requests(resp.id, Action::SetValue).find_map(|r| match r.data { + Some(ActionData::NumericValue(v)) if v.is_finite() => Some(v as f32), + _ => None, + }); + (steps, set) + }); if let Some((p, phase)) = pointer_phase(&resp) { let f = |v: f32, lo: f32, len: f32| ((v - lo) / len).clamp(0.0, 1.0); return (Some((f(p.x, rect.left(), rect.width()), f(p.y, rect.top(), rect.height()))), phase); } - if dx != 0.0 || dy != 0.0 { - let moved = ((x + dx / axes[0].units).clamp(0.0, 1.0), (y - dy / axes[1].units).clamp(0.0, 1.0)); + let across = set.map_or(x + (dx + steps) / axes[0].units, |v| v / axes[0].units); + // Without a marker there is no color yet, so a set value applies even when it equals `x`. + if across != x || dy != 0.0 || (set.is_some() && !marked) { + let moved = (across.clamp(0.0, 1.0), (y - dy / axes[1].units).clamp(0.0, 1.0)); return (Some(moved), Live::Released); } (None, Live::Idle)