From ab7d97ad289c2590e9b2f6605bcb59c9a3725548 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Thu, 22 Oct 2026 11:42:46 -0700 Subject: [PATCH] Color Wheel: with no solid color the arrow keys and screen readers set one With None, a pattern or a mixed paint the hue ring and the square drew no marker and also ignored the arrow keys and screen readers' increment, decrement and set-value actions, so only a pointer could set a color. They now step from the last solid color the panel shows; the marker and the values screen readers read still need a solid color. --- crates/ui-egui/src/dialogs/color_picker.rs | 2 +- crates/ui-egui/src/panels/color_wheel.rs | 7 ++- .../ui-egui/src/panels/tests_color_wheel.rs | 26 +++++++++ crates/ui-egui/src/widgets/color.rs | 54 ++++++++++--------- 4 files changed, 58 insertions(+), 31 deletions(-) diff --git a/crates/ui-egui/src/dialogs/color_picker.rs b/crates/ui-egui/src/dialogs/color_picker.rs index 73d9a532de..c4ef1a984d 100644 --- a/crates/ui-egui/src/dialogs/color_picker.rs +++ b/crates/ui-egui/src/dialogs/color_picker.rs @@ -385,7 +385,7 @@ fn body(app: &mut VectorcraftApp, ui: &mut Ui, d: &mut Dialog) -> DialogAction { let (z, x, y) = field_pos(channel, &p); let at = |x: f32, y: f32| c32(&field_pick(channel, z, x, y).snapped(web).color); if let (Some((x, y)), _) = - widgets::color_field(ui, "cp-field", (tl!("Color field"), channel.field_axes()), vec2(side, side), Some((x, y)), &at) + widgets::color_field(ui, "cp-field", (tl!("Color field"), channel.field_axes()), vec2(side, side), (x, y), true, &at) { pick = Some(field_pick(channel, z, x, y)); } diff --git a/crates/ui-egui/src/panels/color_wheel.rs b/crates/ui-egui/src/panels/color_wheel.rs index 6df65411d6..f466584c05 100644 --- a/crates/ui-egui/src/panels/color_wheel.rs +++ b/crates/ui-egui/src/panels/color_wheel.rs @@ -103,7 +103,7 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) { let color = tgt.color(); let cms = cms::active(); // With None, a pattern or a "?" proxy the wheel shows the last solid color without markers, - // and a click applies the picked color. + // and a click, an arrow key or a screen reader's action applies the picked color. let hsb = match (color, pstate::>(ui.ctx(), MEMORY)) { (Some(c), Some((owner, hsb))) if owner == Owner::of(app, &tgt, c) => hsb, (c, _) => display_hsb(&cms, &c.unwrap_or(app.session.last_solid)), @@ -116,15 +116,14 @@ pub fn show(app: &mut VectorcraftApp, ui: &mut Ui) { super::proxy(app, ui, 40.0); }); let size = ui.available_width().min(MAX_WHEEL); - let ring = widgets::hue_ring(ui, "color-wheel-ring", tl!("Hue ring"), size, marked.then_some(hsb[0])); + let ring = widgets::hue_ring(ui, "color-wheel-ring", tl!("Hue ring"), size, hsb[0], marked); let square = widgets::ring_square(ring.rect); let mut field_ui = ui.new_child(UiBuilder::new().id_salt("color-wheel-square").max_rect(square)); let axes = [FieldAxis { name: tl!("Saturation"), units: 100.0, suffix: "%" }, FieldAxis { name: tl!("Brightness"), units: 100.0, suffix: "%" }]; let at = |x: f32, y: f32| widgets::hsb32([hsb[0], x, 1.0 - y]); - let pos = marked.then_some((hsb[1], 1.0 - hsb[2])); let name = tl!("Saturation and brightness"); - let (field, field_phase) = widgets::color_field(&mut field_ui, "field", (name, axes), square.size(), pos, &at); + let (field, field_phase) = widgets::color_field(&mut field_ui, "field", (name, axes), square.size(), (hsb[1], 1.0 - hsb[2]), marked, &at); if let Some((h, phase)) = ring.hue { pick = Some(([h, hsb[1], hsb[2]], phase)); } diff --git a/crates/ui-egui/src/panels/tests_color_wheel.rs b/crates/ui-egui/src/panels/tests_color_wheel.rs index b5498e93d5..61a9530a3f 100644 --- a/crates/ui-egui/src/panels/tests_color_wheel.rs +++ b/crates/ui-egui/src/panels/tests_color_wheel.rs @@ -236,6 +236,32 @@ fn with_a_none_fill_the_wheel_has_no_marker_and_a_click_sets_the_fill() { assert!(near(hsb(&c), [hsb(&h.state().session.last_solid)[0], 0.5, 0.5], [0.5, 0.01, 0.01]), "{:?}", hsb(&c)); } +/// With None the ring and the square still take a screen reader's actions and the arrow keys. They +/// start from the last solid color the wheel shows, and the result sets the fill. +#[test] +fn with_a_none_fill_screen_readers_and_the_arrow_keys_set_the_fill() { + let mut app = app(json!("#ff0000")); + app.run("paint.setFill", json!({"none": true})).unwrap(); + let mut h = harness(app); + let last = hsb(&h.state().session.last_solid); + let (target_node, target_tree) = h.get_by_label("Hue ring").accesskit_node().locate(); + let data = Some(ActionData::NumericValue(200.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, last[1], last[2]], [0.5, 2e-3, 2e-3]), "{:?}", hsb(&fill(h.state()))); + // A click focuses the square; with the fill set to None again the arrows still step it. + let sq = rect_of(&h, "Saturation and brightness"); + click(&mut h, in_square(sq, 0.5, 0.5), Modifiers::NONE); + h.state_mut().run("paint.setFill", json!({"none": true})).unwrap(); + h.run_steps(1); + assert_eq!(h.get_by_label("Saturation and brightness").value(), None, "no marker"); + let last = hsb(&h.state().session.last_solid); + key(&mut h, egui::Key::ArrowLeft, Modifiers::NONE); + key(&mut h, egui::Key::ArrowDown, Modifiers::NONE); + let after = hsb(&fill(h.state())); + assert!(near(after, [last[0], last[1] - 0.01, last[2] - 0.01], [0.5, 2e-3, 2e-3]), "{last:?} → {after:?}"); +} + /// The hue stays while the square passes through a gray: a drag to the left edge (white, whose own /// hue is 0°) and back ends at the color a click at its end gives, for an RGB and for a CMYK fill /// (whose stored color differs from the picked RGB), and an RGB green is green again. diff --git a/crates/ui-egui/src/widgets/color.rs b/crates/ui-egui/src/widgets/color.rs index 5fe8bcbbb7..6d1ac43ab6 100644 --- a/crates/ui-egui/src/widgets/color.rs +++ b/crates/ui-egui/src/widgets/color.rs @@ -372,17 +372,18 @@ fn focus_outline(ui: &Ui, resp: &Response, rect: Rect) { } /// A 2D color field (the Color Picker's, the Color Wheel panel's square): `color_at(x, y)` gives -/// the color at a point (0..1 each, y down) and a ring marks `pos` (no ring for `None`). A press -/// gives it the keyboard focus. While it has the focus, → and ← move the point across by one unit -/// of `axes[0]` and ↑ and ↓ move it up and down by one unit of `axes[1]`. Screen readers read -/// `name` and both components. Returns the clicked or dragged point, or the point the keys moved -/// to, and the phase. +/// 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. pub fn color_field( ui: &mut Ui, id: impl std::hash::Hash, (name, axes): (&str, [FieldAxis<'_>; 2]), size: Vec2, - pos: Option<(f32, f32)>, + pos: (f32, f32), + marked: bool, color_at: &dyn Fn(f32, f32) -> Color32, ) -> (Option<(f32, f32)>, Live) { let t = Tokens::get(ui.ctx()); @@ -393,13 +394,14 @@ pub fn color_field( painter.add(color_mesh(rect, (32, 32), color_at)); painter.rect_stroke(rect, 0.0, Stroke::new(1.0, t.border), StrokeKind::Outside); focus_outline(ui, &resp, rect); - if let Some((x, y)) = pos { + let (x, y) = pos; + if marked { let c = pos2(rect.left() + x.clamp(0.0, 1.0) * rect.width(), rect.top() + y.clamp(0.0, 1.0) * rect.height()); painter.circle_stroke(c, 5.0, Stroke::new(1.0, Color32::BLACK)); painter.circle_stroke(c, 4.0, Stroke::new(1.0, Color32::WHITE)); } let value = |v: f32, a: &FieldAxis| format!("{}{}", (v.clamp(0.0, 1.0) * a.units).round(), a.suffix); - let text = pos.map(|(x, y)| { + let text = marked.then(|| { crate::i18n::fmt( tl!("{name1} {value1}, {name2} {value2}"), &[("name1", axes[0].name), ("value1", &value(x, &axes[0])), ("name2", axes[1].name), ("value2", &value(1.0 - y, &axes[1]))], @@ -414,13 +416,11 @@ pub fn color_field( 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); } - match pos { - Some((x, y)) 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)); - (Some(moved), Live::Released) - } - _ => (None, Live::Idle), + 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)); + return (Some(moved), Live::Released); } + (None, Live::Idle) } /// A smoothly shaded colour area: a `cols` × `rows` grid over `rect` with `color_at(x, y)` (0..1 @@ -743,14 +743,15 @@ pub struct RingResponse { } /// A hue ring `size` across: a band of hues drawn [`RING_MARGIN`] inside its rect, the hue -/// increasing counterclockwise from 3 o'clock as on [`harmony_wheel`], with a marker at `hue` (none -/// for `None`). A press on the band, or up to [`RING_MARGIN`] outside it, picks the hue under the +/// increasing counterclockwise from 3 o'clock as on [`harmony_wheel`], with a marker at `hue` when +/// `marked`. A press on the band, or up to [`RING_MARGIN`] outside it, picks the hue under the /// pointer (with Shift, in 45° steps) and gives the ring the keyboard focus. A press inside the /// band's inner edge comes back as `inner`, and a press in the rect's corners does nothing. The part -/// a press starts on keeps the whole drag. While the ring has the focus, → and ↑ raise the hue by -/// 1° and ← and ↓ lower it ([`STEPS`]), and 360° wraps to 0°. Screen readers read it as a slider -/// named `name` from 0 to 360, which their increment, decrement and set-value actions move. -pub fn hue_ring(ui: &mut Ui, id: impl std::hash::Hash, name: &str, size: f32, hue: Option) -> RingResponse { +/// a press starts on keeps the whole drag. While the ring has the focus, → and ↑ raise `hue` by +/// 1° and ← and ↓ lower it ([`STEPS`]), marked or not, and 360° wraps to 0°. Screen readers read it +/// as a slider named `name` from 0 to 360 (0 when not `marked`), which their increment, decrement +/// and set-value actions move. +pub fn hue_ring(ui: &mut Ui, id: impl std::hash::Hash, name: &str, size: f32, hue: f32, marked: bool) -> RingResponse { use egui::accesskit::{Action, ActionData}; const SEG: u32 = 72; let t = Tokens::get(ui.ctx()); @@ -778,12 +779,12 @@ pub fn hue_ring(ui: &mut Ui, id: impl std::hash::Hash, name: &str, size: f32, hu if ui.memory(|m| m.has_focus(resp.id)) { painter.circle_stroke(c, outer + 2.0, Stroke::new(1.0, t.accent)); } - if let Some(h) = hue { - let m = wheel_pos(c, (inner + outer) / 2.0, h, 1.0); + if marked { + let m = wheel_pos(c, (inner + outer) / 2.0, hue, 1.0); painter.circle_stroke(m, RING_WIDTH / 2.0 - 1.0, Stroke::new(1.5, Color32::WHITE)); painter.circle_stroke(m, RING_WIDTH / 2.0, Stroke::new(1.0, Color32::BLACK)); } - resp.widget_info(|| egui::WidgetInfo::slider(ui.is_enabled(), f64::from(hue.unwrap_or(0.0)), name)); + resp.widget_info(|| egui::WidgetInfo::slider(ui.is_enabled(), if marked { f64::from(hue) } else { 0.0 }, name)); ui.ctx().accesskit_node_builder(resp.id, |node| { node.set_min_numeric_value(0.0); node.set_max_numeric_value(360.0); @@ -839,9 +840,10 @@ pub fn hue_ring(ui: &mut Ui, id: impl std::hash::Hash, name: &str, size: f32, hu RingPart::Inner => out.inner = Some((p, phase)), RingPart::Corner => {} } - } else if let Some(h) = hue { - let new = set.unwrap_or(h + steps); - if new != h { + } else { + let new = set.unwrap_or(hue + steps); + // Without a marker there is no color yet, so a set value applies even when it equals `hue`. + if new != hue || (set.is_some() && !marked) { out.hue = Some((new.rem_euclid(360.0), Live::Released)); } }