Merge pull request #9284 from davidmann4/fix/crop-click-snap

Crop: a click on the frame or a handle never snaps it (#9283)

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

@@ -524,6 +524,18 @@ fn follow_reposition(app: &mut PhotocraftApp) {
     }
 }
 
+/// A click on a crop frame or one of its handles (released where it was pressed, the frame not
+/// moved) changes nothing, as a Move-tool click doesn't move the layer (`canvas::tool_event`):
+/// snapping the release would pull an edge within reach onto a nearby one.
+fn crop_click(app: &PhotocraftApp, p: [f64; 2]) -> bool {
+    use crate::crop_ui::CropDrag;
+    let Some(snap) = app.prefs_rt.snap.as_ref() else { return false };
+    let Some(CropDrag::Move { rect, .. } | CropDrag::Resize { rect, .. } | CropDrag::MoveImage { rect, .. }) = app.crop.drag else {
+        return false;
+    };
+    app.ui.tool == Tool::Crop && p == snap.start && app.ui.crop_rect == Some(rect)
+}
+
 /// Snap a tool event's coordinates (called by `canvas::tool_event` before the tools see it).
 pub fn filter_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifiers) -> ToolEvent {
     match ev {
@@ -564,6 +576,11 @@ pub fn filter_event(app: &mut PhotocraftApp, ev: ToolEvent, mods: egui::Modifier
             if app.prefs_rt.snap.is_none() {
                 return ev;
             }
+            if crop_click(app, [x, y]) {
+                app.prefs_rt.snap = None;
+                app.prefs_rt.snap_lines.clear();
+                return ev;
+            }
             follow_reposition(app);
             let disabled = override_for_gesture(app, mods);
             if let Some(s) = app.prefs_rt.snap.as_mut() {

crates/ui-egui/src/tools/crop_ui/tests.rs

@@ -496,6 +496,27 @@ fn moving_the_frame_snaps_its_edges() {
     assert_eq!(app.ui.crop_rect, Some([0.0, 10.0, 40.0, 40.0]), "left edge snaps to the document edge");
 }
 
+/// A click on the frame (released where it was pressed) changes nothing, as a Move-tool click
+/// doesn't move the layer: snapping the release would pull an edge within reach onto the
+/// document's, so a double-click to commit cropped another rect.
+#[test]
+fn a_click_on_the_frame_or_a_handle_never_snaps_it() {
+    let frame = [5.0, 10.0, 105.0, 60.0];
+    for classic in [true, false] {
+        // Inside (Classic: moves the frame; default: moves the image), the left edge's handle, the top-left corner's.
+        for p in [[50.0, 30.0], [5.0, 35.0], [5.0, 10.0]] {
+            let mut app = app(SampleType::U8);
+            app.ui.tool_options.crop_shield.classic_mode = classic;
+            app.ui.extras.snap = true;
+            app.ui.crop_rect = Some(frame);
+            app.crop.default_frame = false;
+            drag(&mut app, &[p], NONE);
+            assert_eq!(app.ui.crop_rect, Some(frame), "classic {classic}, click at {p:?}");
+            assert!(!crate::menus::is_enabled(&app, "edit.undo"), "classic {classic}, click at {p:?}: no box step");
+        }
+    }
+}
+
 /// #3145: Edit › Undo while a crop is pending steps the *box*, not the document (Photoshop keeps
 /// Undo live during a crop), so a moved frame comes back.
 #[test]