Merge pull request #2335 from TrevorBrown-dev/fix/crop-from-picker-2307

M8: after Crop is chosen in the image fill's menu, a drag on the image crops it, and no internal gesture message shows (#2307)

crates/engine/src/error.rs

@@ -14,6 +14,10 @@ pub enum EngineError {
     NoNode(NodeId),
     #[error("{0}")]
     Other(String),
+    /// A preview was asked for with no gesture open (something committed it under a held button).
+    /// Not for the user: the canvas drops it, as the drag it belonged to is over.
+    #[error("no gesture in progress")]
+    NoGesture,
     /// A command run in the background sent a request to a server and runs again with the answer
     /// ([`background`]); never returned to other callers.
     #[error("waiting for the server")]

crates/engine/src/session/undo.rs

@@ -65,7 +65,7 @@ impl Session {
     /// Replace the document with `f(snapshot)` for a live preview during a gesture.
     pub fn preview(&mut self, f: impl FnOnce(&mut Document, &mut Vec<NodeId>) -> Result<()>) -> Result<()> {
         let ds = self.ds_mut()?;
-        let Some(it) = ds.interaction.clone() else { return Err(EngineError::Other("no gesture in progress".into())) };
+        let Some(it) = ds.interaction.clone() else { return Err(EngineError::NoGesture) };
         let mut doc = (*it.doc).clone();
         let mut sel = it.selection.clone();
         // What the gesture changes on the selected layers is an explicit edit.

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

@@ -206,6 +206,13 @@ pub fn gradient_dragging(ctx: &egui::Context) -> bool {
     matches!(ctx.data(|d| d.get_temp::<Drag>(drag_id())), Some(Drag::Gradient { .. }))
 }
 
+/// Whether the open image fill's crop (its handles, or a drag on the image) is being dragged, the
+/// press that starts it included: the open colour picker holds that paint in Crop mode, so the
+/// press is its own control, not one outside it that would close it and commit the drag (#2307).
+pub fn crop_dragging(ctx: &egui::Context) -> bool {
+    matches!(ctx.data(|d| d.get_temp::<Drag>(drag_id())), Some(Drag::Crop { .. }))
+}
+
 /// 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())

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

@@ -398,7 +398,11 @@ fn send(app: &mut FigApp, kind: PointerKind, pos: Pos2, m: &egui::Modifiers) {
         app.canvas.pressure = None;
     }
     app.canvas.last_pointer = Some(Point::new(ev.x, ev.y));
-    if let Err(e) = app.session.pointer(&ev) {
+    // A gesture something else committed under the held button (a colour picker closing on this
+    // press) leaves the rest of its drag with nothing to edit: that is not worth saying.
+    if let Err(e) = app.session.pointer(&ev)
+        && !matches!(e, figcraft_engine::EngineError::NoGesture)
+    {
         app.toast(e.to_string());
     }
     app.canvas.last_hover = hover.then(|| HoverKey::of(app, ev));

crates/ui-egui/src/crop_image_tests.rs

@@ -194,6 +194,56 @@ fn crop_chosen_in_the_colour_picker_enters_crop_mode() {
     assert!((scaled[0] - after[0] * 5.0 / 6.0).abs() < 1e-6 && (scaled[3] - after[3] * 5.0 / 6.0).abs() < 1e-6, "{after:?} -> {scaled:?}");
 }
 
+/// The reporter's sequence (#2307): the colour picker open on an image fill held in Crop mode
+/// (Crop chosen in its mode dropdown, or reopened on a fill that already is), then a press on the
+/// layer and a drag. The drag moves the picture inside the layer (its crop), not the layer, the
+/// press doesn't close the picker under it, and no internal gesture message reaches the user.
+#[test]
+fn crop_from_the_colour_picker_then_a_drag_on_the_image_moves_the_crop_not_the_layer() {
+    let (mut u, layer, _) = setup();
+    u.app.ui.picker = Some(crate::colorpicker::PickerTarget::Paint { fills: true, index: 0, ids: vec![layer] });
+    for _ in 0..4 {
+        u.frame(vec![]);
+    }
+    choose_mode(&mut u, "Fill", "Crop");
+    // Crop mode on the canvas ended (a click away from the image); the picker opens on the Crop fill.
+    u.app.ui.crop = None;
+    u.app.ui.picker = Some(crate::colorpicker::PickerTarget::Paint { fills: true, index: 0, ids: vec![layer] });
+    for _ in 0..4 {
+        u.frame(vec![]);
+    }
+    let (_, Some(before)) = mode(&u, layer) else { panic!("a crop") };
+    let at = u.app.session.doc().unwrap().node(layer).unwrap().transform.as_coeffs();
+    let from = u.app.to_screen(100.0, 50.0);
+    u.drag_to(from, from + vec2(30.0, 10.0));
+    u.frame(vec![]);
+    u.button(from + vec2(30.0, 10.0), false);
+    let n = u.app.session.doc().unwrap().node(layer).unwrap().clone();
+    assert_eq!(n.transform.as_coeffs(), at, "the layer stays where it is");
+    assert_eq!((n.width, n.height), (200.0, 100.0), "and keeps its size");
+    let (m, Some(after)) = mode(&u, layer) else { panic!("a crop") };
+    assert_eq!(m, ImageScaleMode::Crop);
+    assert_ne!(after, before, "the drag moved the picture inside the layer");
+    assert_eq!(u.app.session.selection(), vec![layer]);
+    assert!(u.app.ui.toast.as_ref().is_none_or(|(t, _)| !t.contains("gesture")), "no internal gesture message: {:?}", u.app.ui.toast);
+}
+
+/// A drag the colour picker's close committed under the held button (a press on a layer closing
+/// the picker) leaves the rest of its drag with no gesture: that says nothing to the user.
+#[test]
+fn a_drag_whose_gesture_was_committed_under_the_button_shows_no_message() {
+    let (mut u, layer, _) = setup();
+    u.app.run("select.set", json!({"ids": [layer]}));
+    let from = u.app.to_screen(100.0, 50.0);
+    u.button(from, true);
+    u.app.session.commit_interaction().unwrap();
+    for k in 1..=10 {
+        u.frame(vec![Event::PointerMoved(from + vec2(3.0, 1.0) * k as f32)]);
+    }
+    u.button(from + vec2(30.0, 10.0), false);
+    assert!(u.app.ui.toast.is_none(), "no message: {:?}", u.app.ui.toast);
+}
+
 /// The first paint, an image, of `id`'s fills (`fills`) or strokes, or of region `region` of a vector.
 fn image_at(u: &Ui, id: NodeId, fills: bool, region: Option<usize>) -> figcraft_color::ImagePaint {
     let n = u.app.session.doc().unwrap().node(id).unwrap();

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

@@ -1041,7 +1041,8 @@ pub fn show(app: &mut FigApp, ctx: &egui::Context) {
     let picking = app.session.tool.pick_only || app.session.tool.picked.is_some();
     // 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);
+    // Nor may one on the crop of an image paint held in Crop mode (#2307).
+    let own = picking || crate::handles::gradient_dragging(ctx) || crate::handles::crop_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();