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

@@ -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));
                             }

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::<Option<(Owner, [f32; 3])>>(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));
         }

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.

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<f32>) -> 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));
         }
     }