From 0a8446514bdc516fa204476cb67bce208f89d529 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Tue, 27 Oct 2026 21:43:21 -0700 Subject: [PATCH] Color Wheel: a step past the edge of the square leaves the color as it is An arrow key or a screen reader's step outward at the edge of the square returned the clamped point, which equals the current one, and the wheel picked it again. For a CMYK fill the pick separated the color again, so one step at 100% saturation turned C100 into C71 M16 and added an undo step. color_field now returns no point when the clamped point is unchanged; with no marker a key or an action still sets a color. --- crates/ui-egui/src/panels/tests_color_wheel.rs | 15 +++++++++++++++ crates/ui-egui/src/widgets/color.rs | 11 +++++++---- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/crates/ui-egui/src/panels/tests_color_wheel.rs b/crates/ui-egui/src/panels/tests_color_wheel.rs index 8574a2900..a3dfa3fba 100644 --- a/crates/ui-egui/src/panels/tests_color_wheel.rs +++ b/crates/ui-egui/src/panels/tests_color_wheel.rs @@ -430,6 +430,21 @@ fn screen_readers_read_and_move_the_ring_and_the_square() { assert!(near(hsb(&fill(h.state())), [200.0, 0.19, 0.6], [1e-3, 1e-3, 1e-3]), "{:?}", hsb(&fill(h.state()))); } +/// A step past the edge of the square leaves the fill alone: a CMYK cyan shows at 100% saturation, +/// and a step up from there neither separates it again nor adds an undo step. +#[test] +fn a_step_past_the_edge_of_the_square_changes_nothing() { + let mut h = harness(app(json!({"c": 1.0, "m": 0.0, "y": 0.0, "k": 0.0}))); + let before = fill(h.state()); + assert_eq!(hsb(&before)[1], 1.0, "{:?}", hsb(&before)); + let undo = undo_len(h.state()); + let (target_node, target_tree) = h.get_by_label("Saturation and brightness").accesskit_node().locate(); + h.event(Event::AccessKitActionRequest(ActionRequest { target_node, target_tree, action: Action::Increment, data: None })); + h.run_steps(1); + assert_eq!(fill(h.state()), before); + assert_eq!(undo_len(h.state()), undo, "no undo step"); +} + /// 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 and step the first. #[test] diff --git a/crates/ui-egui/src/widgets/color.rs b/crates/ui-egui/src/widgets/color.rs index 159f33a49..744855cec 100644 --- a/crates/ui-egui/src/widgets/color.rs +++ b/crates/ui-egui/src/widgets/color.rs @@ -378,7 +378,8 @@ fn focus_outline(ui: &Ui, resp: &Response, rect: Rect) { /// 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. +/// point, or the point the keys or actions moved to, and the phase. When `marked`, a step past an +/// edge returns no point. pub fn color_field( ui: &mut Ui, id: impl std::hash::Hash, @@ -435,9 +436,11 @@ pub fn color_field( return (Some((f(p.x, rect.left(), rect.width()), f(p.y, rect.top(), rect.height()))), phase); } 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)); + let moved = (across.clamp(0.0, 1.0), (y - dy / axes[1].units).clamp(0.0, 1.0)); + // A step past an edge leaves the point where it is. Without a marker there is no color yet, so + // a key or an action applies even when the point stays. + let asked = set.is_some() || dx + steps != 0.0 || dy != 0.0; + if moved != (x.clamp(0.0, 1.0), y.clamp(0.0, 1.0)) || (asked && !marked) { return (Some(moved), Live::Released); } (None, Live::Idle)