Colour picker: dragging a gradient handle on the canvas keeps the picker and the handles up (#1852) (#1985)

* M1: dragging a gradient handle on the canvas keeps the colour picker open (#1852)

A press on one of the open gradient's handles or stops closed the picker: the canvas took the press
and began the drag, then the picker, drawn later in the same frame, saw a press outside its card and
closed, committing the drag's interaction at once. The handles vanished with the picker and the drag
went on without an undo step of its own. The handles are the picker's controls: while a gradient
handle or stop drags (`handles::gradient_dragging`, the press included) the picker doesn't close on
an outside press, as it already doesn't while the eyedropper samples. A press elsewhere on the
canvas still closes it. Docs: ui-parity.md.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* M1: a handle drag released off the canvas ends there (#1852)

Review: a gradient handle drag whose button went up over a panel, over the colour picker or outside
the window stayed stashed, its interaction uncommitted, until the pointer was back on the canvas,
and the next press there moved the handle to wherever it landed; with the picker now held open while
a gradient drag is stashed, it would also have stayed open meanwhile. `handles::end_released_drag`
commits and clears a stashed drag when the button is up, called from the canvas input's off-canvas
branch and when the pointer is over something drawn above the canvas. Test:
`a_gradient_drag_released_off_the_canvas_ends_there` (released over the sidebar and over the picker).

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

* M1: a handle drag ends on a release over a ruler, and when Escape or Undo closes its gesture (#1852)

The release check ran only where the pointer was not on the canvas, after the
comment pins and ruler guides had their turn; both return early, so a gradient
handle released over a ruler stayed dragged, uncommitted, until the next press.
It now runs first, wherever the pointer is, and the frame it ends a drag on is
the handle's, as the release on the handle itself was.

Escape (the picker closes and commits) or Undo can also close the gesture while
the button is still down. The stashed drag then kept setting the fill every
frame with no gesture open, one undo step a frame; with no gesture open the
drag is now over.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
Co-authored-by: FredApps <153173854+FredApps@users.noreply.github.com>

crates/ui-egui/src/canvas/handles.rs

@@ -185,6 +185,33 @@ fn drag_id() -> Id {
     Id::unique("figcraft_handle_drag")
 }
 
+/// Whether a gradient handle or stop on the canvas is being dragged, the press that starts it
+/// included: those are the open colour picker's own controls, so the picker stays through the drag
+/// instead of taking the press for one outside it (#1852).
+pub fn gradient_dragging(ctx: &egui::Context) -> bool {
+    matches!(ctx.data(|d| d.get_temp::<Drag>(drag_id())), Some(Drag::Gradient { .. }))
+}
+
+/// Whether the document has a gesture open: a handle drag edits only inside its own.
+fn gesture_open(app: &FigApp) -> bool {
+    app.session.ds().is_ok_and(|d| d.interaction.is_some())
+}
+
+/// A handle drag whose button went up wherever the pointer was (off the canvas, over a panel, the
+/// colour picker or a ruler, outside the window) ends as a release on the handle would: the edit
+/// commits now, instead of waiting, uncommitted, to move the handle to wherever the pointer next
+/// presses. So does one whose gesture Escape or Undo closed under the held button: it stops there
+/// rather than editing on with no gesture open, an undo step a frame. True when a drag ended.
+pub fn end_released_drag(app: &mut FigApp, ctx: &egui::Context) -> bool {
+    let stashed = ctx.data(|d| d.get_temp::<Drag>(drag_id())).is_some();
+    if !stashed || (ctx.input(|i| i.pointer.primary_down()) && gesture_open(app)) {
+        return false;
+    }
+    let _ = app.session.commit_interaction();
+    ctx.data_mut(|d| d.remove::<Drag>(drag_id()));
+    true
+}
+
 /// The gradient being edited (single node, picker open on a gradient paint).
 fn gradient_target(app: &FigApp) -> Option<(NodeId, bool, usize, figcraft_color::Gradient)> {
     let Some(PickerTarget::Paint { fills, index, ids }) = &app.ui.picker else { return None };
@@ -516,6 +543,11 @@ pub fn input(app: &mut FigApp, ctx: &egui::Context, pos: Pos2) -> bool {
     });
     let dragging: Option<Drag> = ctx.data(|d| d.get_temp(drag_id()));
     if let Some(drag) = dragging {
+        // Escape or Undo closed the gesture under the held button: the drag is over.
+        if !gesture_open(app) {
+            ctx.data_mut(|d| d.remove::<Drag>(drag_id()));
+            return true;
+        }
         if down {
             apply(app, &drag, pos, shift);
             ctx.set_cursor_icon(egui::CursorIcon::Grabbing);

crates/ui-egui/src/canvas/input.rs

@@ -183,6 +183,8 @@ pub(super) fn input(app: &mut FigApp, ui: &mut Ui, resp: &egui::Response) -> boo
         if !ctx.input(|i| i.pointer.button_down(egui::PointerButton::Middle) || i.pointer.primary_down()) {
             app.canvas.middle_pan = None;
         }
+        // So is a handle drag.
+        crate::handles::end_released_drag(app, &ctx);
         return true;
     };
     // A press on the canvas gives the keyboard back to it (arrows nudge again).
@@ -230,6 +232,12 @@ pub(super) fn input(app: &mut FigApp, ui: &mut Ui, resp: &egui::Response) -> boo
         }
         return true;
     }
+    // A handle drag whose button went up ends first, wherever on the canvas the pointer is: over a
+    // ruler or a comment pin, which take the pointer below, or over something drawn above the
+    // canvas (the colour picker, a card). The handle had the press, so it has the release too.
+    if crate::handles::end_released_drag(app, &ctx) {
+        return false;
+    }
     if !app.canvas.dragging && (resp.hovered() || app.ui.comments.dragging()) && crate::comments::input(app, &ctx, pos) {
         return false;
     }

crates/ui-egui/src/colorpicker_selection_tests.rs

@@ -215,3 +215,160 @@ fn a_press_in_the_gradient_variable_picker_keeps_the_colour_picker_open() {
     assert!(u.app.ui.picker.is_some(), "the child variable picker is part of the interaction");
     u.button(at, false);
 }
+
+#[test]
+fn dragging_a_gradient_handle_on_the_canvas_keeps_the_picker_open() {
+    // #1852: the handles belong to the picker, so the press that starts a drag is not a press
+    // outside it: the picker and the handles stay, the drag is one undo step, and a press elsewhere
+    // on the layer still closes it.
+    let mut s = Session::new();
+    let a = NodeId(s.execute("shape.rectangle", &json!({"x": 100, "y": 100, "width": 200, "height": 200})).unwrap()["id"].as_u64().unwrap());
+    s.execute("fill.set", &json!({"id": a, "paint": {"type": "linearGradient", "stops": ["#ff0000", "#0000ff"]}})).unwrap();
+    s.execute("select.set", &json!({"ids": [a]})).unwrap();
+    let mut u = Ui::new(s);
+    let target = PickerTarget::Paint { fills: true, index: 0, ids: vec![a] };
+    u.app.open_picker(target.clone(), None);
+    for _ in 0..5 {
+        u.frame(vec![]);
+    }
+    assert_eq!(u.app.ui.picker, Some(target.clone()));
+    let depth = u.app.session.undo_depth().0;
+    let from_handle = |u: &Ui| match &u.app.session.doc().unwrap().node(a).unwrap().fills[0].kind {
+        figcraft_color::PaintKind::Gradient(g) => g.from,
+        _ => panic!("the fill is a gradient"),
+    };
+    assert_eq!(from_handle(&u), (0.5, 0.0), "a linear gradient starts at the top middle");
+    // Drag the start handle from the top middle of the layer to its left middle.
+    let from = u.app.to_screen(200.0, 100.0);
+    let to = u.app.to_screen(100.0, 200.0);
+    u.button(from, true);
+    assert_eq!(u.app.ui.picker, Some(target.clone()), "the press on the handle keeps the picker open");
+    for _ in 0..2 {
+        u.frame(vec![egui::Event::PointerMoved(to)]);
+    }
+    assert_eq!(u.app.ui.picker, Some(target.clone()), "and so does the drag");
+    let (x, y) = from_handle(&u);
+    assert!(x.abs() < 0.02 && (y - 0.5).abs() < 0.02, "the handle followed the pointer: ({x}, {y})");
+    u.button(to, false);
+    u.frame(vec![]);
+    assert_eq!(u.app.ui.picker, Some(target.clone()), "released, the picker is still open");
+    assert_eq!(u.app.session.undo_depth().0, depth + 1, "the drag is one undo step");
+    // A press on the layer away from its handles is still a press outside the picker.
+    let body = u.app.to_screen(250.0, 250.0);
+    u.button(body, true);
+    u.button(body, false);
+    u.frame(vec![]);
+    assert!(u.app.ui.picker.is_none(), "a plain canvas press closes it");
+}
+
+#[test]
+fn a_gradient_drag_released_off_the_canvas_ends_there() {
+    // A handle drag whose button goes up over the sidebar or over the picker itself ends there:
+    // the edit commits, the picker stays, and the next press on the canvas is a plain press (it
+    // closes the picker) rather than a move of the handle to wherever it landed.
+    for over in ["sidebar", "picker"] {
+        let mut s = Session::new();
+        let a = NodeId(s.execute("shape.rectangle", &json!({"x": 100, "y": 100, "width": 200, "height": 200})).unwrap()["id"].as_u64().unwrap());
+        s.execute("fill.set", &json!({"id": a, "paint": {"type": "linearGradient", "stops": ["#ff0000", "#0000ff"]}})).unwrap();
+        s.execute("select.set", &json!({"ids": [a]})).unwrap();
+        let mut u = Ui::new(s);
+        let target = PickerTarget::Paint { fills: true, index: 0, ids: vec![a] };
+        u.app.open_picker(target.clone(), None);
+        for _ in 0..5 {
+            u.frame(vec![]);
+        }
+        let depth = u.app.session.undo_depth().0;
+        let from_handle = |u: &Ui| match &u.app.session.doc().unwrap().node(a).unwrap().fills[0].kind {
+            figcraft_color::PaintKind::Gradient(g) => g.from,
+            _ => panic!("the fill is a gradient"),
+        };
+        u.button(u.app.to_screen(200.0, 100.0), true);
+        u.frame(vec![egui::Event::PointerMoved(u.app.to_screen(100.0, 200.0))]);
+        assert!(crate::handles::gradient_dragging(&u.ctx), "{over}: the handle is being dragged");
+        let spot = match over {
+            "sidebar" => u.app.ui.right_panel_rect.unwrap().center(),
+            _ => u.app.ui.picker_popover.as_ref().unwrap().rect.center(),
+        };
+        if over == "picker" {
+            assert!(u.app.canvas.rect.contains(spot), "the picker lies over the canvas");
+        } else {
+            assert!(!u.app.canvas.rect.contains(spot), "the sidebar is off the canvas");
+        }
+        u.button(spot, false);
+        u.frame(vec![]);
+        assert!(!crate::handles::gradient_dragging(&u.ctx), "{over}: the release ended the drag");
+        assert_eq!(u.app.ui.picker, Some(target.clone()), "{over}: the picker is still open");
+        assert_eq!(u.app.session.undo_depth().0, depth + 1, "{over}: the drag is one undo step");
+        let moved = from_handle(&u);
+        assert!(moved.0.abs() < 0.02 && (moved.1 - 0.5).abs() < 0.02, "{over}: the handle stayed where it was dragged: {moved:?}");
+        // The next press on the layer is a plain one: the handle stays put and the picker closes.
+        let body = u.app.to_screen(250.0, 250.0);
+        u.button(body, true);
+        u.frame(vec![]);
+        assert_eq!(from_handle(&u), moved, "{over}: a later press doesn't move the handle");
+        assert!(u.app.ui.picker.is_none(), "{over}: a later press closes the picker as any outside press does");
+        u.button(body, false);
+    }
+}
+
+/// A rectangle with a linear gradient fill, selected, its picker open and the canvas settled; a
+/// second layer drawn after it leaves an undo step that isn't the gradient's. Rulers on or off.
+fn gradient_ui(rulers: bool) -> Ui {
+    let mut s = Session::new();
+    if rulers {
+        s.execute("view.rulers", &json!({"on": true})).unwrap();
+    }
+    let a = NodeId(s.execute("shape.rectangle", &json!({"x": 100, "y": 100, "width": 200, "height": 200})).unwrap()["id"].as_u64().unwrap());
+    s.execute("fill.set", &json!({"id": a, "paint": {"type": "linearGradient", "stops": ["#ff0000", "#0000ff"]}})).unwrap();
+    s.execute("shape.rectangle", &json!({"x": 400, "y": 100, "width": 20, "height": 20})).unwrap();
+    s.execute("select.set", &json!({"ids": [a]})).unwrap();
+    let mut u = Ui::new(s);
+    u.app.open_picker(PickerTarget::Paint { fills: true, index: 0, ids: vec![a] }, None);
+    for _ in 0..5 {
+        u.frame(vec![]);
+    }
+    u
+}
+
+#[test]
+fn a_gradient_drag_released_over_a_ruler_ends_there() {
+    // The rulers take the pointer on the canvas before the handles do: the release still ends the
+    // drag, as one undo step, instead of leaving it waiting for the next press.
+    let mut u = gradient_ui(true);
+    let depth = u.app.session.undo_depth().0;
+    u.button(u.app.to_screen(200.0, 100.0), true);
+    u.frame(vec![egui::Event::PointerMoved(u.app.to_screen(100.0, 200.0))]);
+    assert!(crate::handles::gradient_dragging(&u.ctx), "the handle is being dragged");
+    let r = u.app.canvas.rect;
+    let ruler = egui::Pos2::new(r.min.x + 5.0, r.center().y);
+    assert!(!u.app.ui.picker_popover.as_ref().unwrap().rect.contains(ruler), "the ruler is not under the picker");
+    u.button(ruler, false);
+    assert!(!crate::handles::gradient_dragging(&u.ctx), "the release over the ruler ended the drag");
+    assert_eq!(u.app.session.undo_depth().0, depth + 1, "the drag is one undo step");
+}
+
+#[test]
+fn a_gradient_drag_whose_gesture_closes_under_the_held_button_stops_editing() {
+    // Escape (the picker closes and commits) or Undo ends the gesture while the button is still
+    // down: moving on edits nothing more, rather than adding an undo step every frame.
+    for close in ["escape", "undo"] {
+        let mut u = gradient_ui(false);
+        u.button(u.app.to_screen(200.0, 100.0), true);
+        u.frame(vec![egui::Event::PointerMoved(u.app.to_screen(100.0, 200.0))]);
+        assert!(crate::handles::gradient_dragging(&u.ctx), "{close}: the handle is being dragged");
+        if close == "escape" {
+            crate::colorpicker::close(&mut u.app);
+        } else {
+            assert!(u.app.session.undo().unwrap(), "{close}: there is a step to undo");
+        }
+        let depth = u.app.session.undo_depth().0;
+        let doc = u.app.session.doc().unwrap().clone();
+        for k in 1..=4 {
+            u.frame(vec![egui::Event::PointerMoved(u.app.to_screen(100.0 + 20.0 * f64::from(k), 200.0))]);
+        }
+        assert!(!crate::handles::gradient_dragging(&u.ctx), "{close}: the drag is over");
+        assert_eq!(u.app.session.undo_depth().0, depth, "{close}: no undo step a frame");
+        assert!(*u.app.session.doc().unwrap() == doc, "{close}: the gradient stays where the gesture left it");
+        u.button(u.app.to_screen(180.0, 200.0), false);
+    }
+}

crates/ui-egui/src/widgets/colorpicker.rs

@@ -977,7 +977,10 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) {
     // Sampling a colour from the canvas — the eyedropper this picker's own button started, or the
     // colour it just sampled — is the press, not a press outside: neither may close the picker.
     let picking = app.session.tool.pick_only || app.session.tool.picked.is_some();
-    pop = pop.close_on_outside(!picking);
+    // Nor may a press on the open gradient's handles or stops on the canvas: they are this picker's
+    // controls, and the canvas took the press before the picker drew (#1852).
+    let own = picking || crate::handles::gradient_dragging(ctx);
+    pop = pop.close_on_outside(!own);
     // Only the opening control is held out: any other sidebar or canvas press dismisses it.
     let mut held: Vec<Rect> = app.ui.picker_button.into_iter().collect();
     held.extend(crate::variable_ui::picker_rect(ctx));