fix: setting states correctly for cropping

packages/excalidraw/components/App.tsx

@@ -461,6 +461,7 @@ import type {
 } from "../types";
 import type { RoughCanvas } from "roughjs/bin/canvas";
 import type { Action, ActionResult } from "../actions/types";
+import type { GlobalPoint } from "@excalidraw/math";
 
 const AppContext = React.createContext<AppClassProperties>(null!);
 const AppPropsContext = React.createContext<AppProps>(null!);
@@ -6543,7 +6544,6 @@ class App extends React.Component<AppProps, AppState> {
     this.updateBindingEnabledOnPointerMove(event);
 
     // Check if we're in crop mode and hitting uncropped area - if so, skip selection handling
-    let skipSelectionHandling = false;
     if (this.state.croppingElementId) {
       const croppingElement = this.scene
         .getNonDeletedElementsMap()
@@ -6559,7 +6559,7 @@ class App extends React.Component<AppProps, AppState> {
           this.scene.getNonDeletedElementsMap(),
         );
         const hitUncroppedArea = hitElementItself({
-          point: pointFrom(
+          point: pointFrom<GlobalPoint>(
             pointerDownState.origin.x,
             pointerDownState.origin.y,
           ),
@@ -6569,7 +6569,6 @@ class App extends React.Component<AppProps, AppState> {
         });
 
         if (hitUncroppedArea) {
-          skipSelectionHandling = true;
           // Set a dedicated flag for crop position movement
           pointerDownState.cropPositionMovement.enabled = true;
           pointerDownState.cropPositionMovement.croppingElementId =
@@ -6582,10 +6581,7 @@ class App extends React.Component<AppProps, AppState> {
       }
     }
 
-    if (
+    if (this.handleSelectionOnPointerDown(event, pointerDownState)) {
-      !skipSelectionHandling &&
-      this.handleSelectionOnPointerDown(event, pointerDownState)
-    ) {
       return;
     }
 
@@ -7223,11 +7219,39 @@ class App extends React.Component<AppProps, AppState> {
           return true;
         }
 
-        if (
+        if (this.state.croppingElementId) {
-          this.state.croppingElementId &&
+          const croppingElement = this.scene
-          pointerDownState.hit.element?.id !== this.state.croppingElementId
+            .getNonDeletedElementsMap()
-        ) {
+            .get(this.state.croppingElementId);
-          this.finishImageCropping();
+          if (croppingElement) {
+            const uncroppedElement = getUncroppedImageElement(
+              croppingElement as any,
+              this.scene.getNonDeletedElementsMap(),
+            );
+            const hitUncroppedArea = hitElementItself({
+              point: pointFrom<GlobalPoint>(
+                pointerDownState.origin.x,
+                pointerDownState.origin.y,
+              ),
+              element: uncroppedElement,
+              threshold: this.getElementHitThreshold(uncroppedElement),
+              elementsMap: this.scene.getNonDeletedElementsMap(),
+            });
+            if (!hitUncroppedArea) {
+              this.finishImageCropping();
+            } else {
+              // ensure the image remains selected so crop handles are rendered
+              if (
+                (!this.state.selectedElementIds ||
+                  Object.keys(this.state.selectedElementIds).length === 0) &&
+                this.state.croppingElementId
+              ) {
+                this.setState({
+                  selectedElementIds: { [this.state.croppingElementId]: true },
+                });
+              }
+            }
+          }
         }
 
         if (pointerDownState.hit.element) {
@@ -8094,87 +8118,120 @@ class App extends React.Component<AppProps, AppState> {
           isImageElement(croppingElement) &&
           croppingElement.crop
         ) {
-          const crop = croppingElement.crop;
+          const transformHandleType = pointerDownState.resize.handleType;
-          const image =
-            isInitializedImageElement(croppingElement) &&
-            this.imageCache.get(croppingElement.fileId)?.image;
 
-          if (image && !(image instanceof Promise)) {
+          if (!transformHandleType) {
-            const dragOffset = vectorScale(
+            const crop = croppingElement.crop;
-              vector(
+            const image =
-                pointerCoords.x - pointerDownState.lastCoords.x,
+              isInitializedImageElement(croppingElement) &&
-                pointerCoords.y - pointerDownState.lastCoords.y,
+              this.imageCache.get(croppingElement.fileId)?.image;
-              ),
-              Math.max(this.state.zoom.value, 2),
-            );
 
-            const elementsMap = this.scene.getNonDeletedElementsMap();
+            if (image && !(image instanceof Promise)) {
-            const [x1, y1, x2, y2, cx, cy] = getElementAbsoluteCoords(
+              // calculate total drag offset from the original pointer down position
-              croppingElement,
+              const totalDragOffset = {
-              elementsMap,
+                x: pointerCoords.x - pointerDownState.origin.x,
-            );
+                y: pointerCoords.y - pointerDownState.origin.y,
+              };
 
-            const topLeft = vectorFromPoint(
+              // apply shift key constraint for directional movement
-              pointRotateRads(
+              if (event.shiftKey) {
-                pointFrom(x1, y1),
+                const distanceX = Math.abs(totalDragOffset.x);
-                pointFrom(cx, cy),
+                const distanceY = Math.abs(totalDragOffset.y);
-                croppingElement.angle,
-              ),
-            );
-            const topRight = vectorFromPoint(
-              pointRotateRads(
-                pointFrom(x2, y1),
-                pointFrom(cx, cy),
-                croppingElement.angle,
-              ),
-            );
-            const bottomLeft = vectorFromPoint(
-              pointRotateRads(
-                pointFrom(x1, y2),
-                pointFrom(cx, cy),
-                croppingElement.angle,
-              ),
-            );
-            const topEdge = vectorNormalize(vectorSubtract(topRight, topLeft));
-            const leftEdge = vectorNormalize(
-              vectorSubtract(bottomLeft, topLeft),
-            );
 
-            // project dragOffset onto leftEdge and topEdge to decompose
+                const lockX = distanceX < distanceY;
-            const offsetVector = vector(
+                const lockY = distanceX > distanceY;
-              vectorDot(dragOffset, topEdge),
-              vectorDot(dragOffset, leftEdge),
-            );
 
-            const nextCrop = {
+                if (lockX) {
-              ...crop,
+                  totalDragOffset.x = 0;
-              x: clamp(
+                }
-                crop.x - offsetVector[0] * Math.sign(croppingElement.scale[0]),
-                0,
-                image.naturalWidth - crop.width,
-              ),
-              y: clamp(
-                crop.y - offsetVector[1] * Math.sign(croppingElement.scale[1]),
-                0,
-                image.naturalHeight - crop.height,
-              ),
-            };
 
-            this.scene.mutateElement(croppingElement, {
+                if (lockY) {
-              crop: nextCrop,
+                  totalDragOffset.y = 0;
-            });
+                }
+              }
 
-            // Update last coords for next move and set drag occurred flag
+              // scale the drag offset
-            pointerDownState.lastCoords.x = pointerCoords.x;
+              const scaledDragOffset = vectorScale(
-            pointerDownState.lastCoords.y = pointerCoords.y;
+                vector(totalDragOffset.x, totalDragOffset.y),
-            // @ts-ignore - we need to set this for proper shift direction locking
+                Math.max(this.state.zoom.value, 2),
-            pointerDownState.drag.hasOccurred = true;
+              );
 
-            return;
+              const elementsMap = this.scene.getNonDeletedElementsMap();
+              const [x1, y1, x2, y2, cx, cy] = getElementAbsoluteCoords(
+                croppingElement,
+                elementsMap,
+              );
+
+              const topLeft = vectorFromPoint(
+                pointRotateRads(
+                  pointFrom(x1, y1),
+                  pointFrom(cx, cy),
+                  croppingElement.angle,
+                ),
+              );
+              const topRight = vectorFromPoint(
+                pointRotateRads(
+                  pointFrom(x2, y1),
+                  pointFrom(cx, cy),
+                  croppingElement.angle,
+                ),
+              );
+              const bottomLeft = vectorFromPoint(
+                pointRotateRads(
+                  pointFrom(x1, y2),
+                  pointFrom(cx, cy),
+                  croppingElement.angle,
+                ),
+              );
+              const topEdge = vectorNormalize(
+                vectorSubtract(topRight, topLeft),
+              );
+              const leftEdge = vectorNormalize(
+                vectorSubtract(bottomLeft, topLeft),
+              );
+
+              // project scaledDragOffset onto leftEdge and topEdge to decompose
+              const offsetVector = vector(
+                vectorDot(scaledDragOffset, topEdge),
+                vectorDot(scaledDragOffset, leftEdge),
+              );
+
+              // get the original crop from when the drag started
+              const originalCroppingElement =
+                pointerDownState.originalElements.get(croppingElement.id) as
+                  | ExcalidrawImageElement
+                  | undefined;
+
+              const originalCrop = originalCroppingElement?.crop || crop;
+
+              const nextCrop = {
+                ...crop,
+                x: clamp(
+                  originalCrop.x -
+                    offsetVector[0] * Math.sign(croppingElement.scale[0]),
+                  0,
+                  image.naturalWidth - crop.width,
+                ),
+                y: clamp(
+                  originalCrop.y -
+                    offsetVector[1] * Math.sign(croppingElement.scale[1]),
+                  0,
+                  image.naturalHeight - crop.height,
+                ),
+              };
+
+              this.scene.mutateElement(croppingElement, {
+                crop: nextCrop,
+              });
+
+              // set drag occurred flag for consistency
+              pointerDownState.drag.hasOccurred = true;
+
+              return;
+            }
           }
         }
       }
-      // #endregion dedicated crop position movement
 
       if (this.state.activeLockedId) {
         this.setState({
@@ -8460,128 +8517,6 @@ class App extends React.Component<AppProps, AppState> {
             }
           }
 
-          // #region move crop region
-          if (this.state.croppingElementId) {
-            const croppingElement = this.scene
-              .getNonDeletedElementsMap()
-              .get(this.state.croppingElementId);
-
-            if (
-              croppingElement &&
-              isImageElement(croppingElement) &&
-              croppingElement.crop !== null
-            ) {
-              const crop = croppingElement.crop;
-              const image =
-                isInitializedImageElement(croppingElement) &&
-                this.imageCache.get(croppingElement.fileId)?.image;
-
-              if (image && !(image instanceof Promise)) {
-                // Check if we're hitting either the cropped element or the uncropped area
-                const hitCroppedElement =
-                  pointerDownState.hit.element === croppingElement;
-                const uncroppedElement = getUncroppedImageElement(
-                  croppingElement,
-                  elementsMap,
-                );
-                const hitUncroppedArea =
-                  !hitCroppedElement &&
-                  hitElementItself({
-                    point: pointFrom(pointerCoords.x, pointerCoords.y),
-                    element: uncroppedElement,
-                    threshold: this.getElementHitThreshold(uncroppedElement),
-                    elementsMap,
-                  });
-
-                if (hitCroppedElement || hitUncroppedArea) {
-                  const instantDragOffset = vectorScale(
-                    vector(
-                      pointerCoords.x - lastPointerCoords.x,
-                      pointerCoords.y - lastPointerCoords.y,
-                    ),
-                    Math.max(this.state.zoom.value, 2),
-                  );
-
-                  // Apply shift key constraint for directional movement
-                  let constrainedDragOffset = instantDragOffset;
-                  if (event.shiftKey) {
-                    const absX = Math.abs(instantDragOffset[0]);
-                    const absY = Math.abs(instantDragOffset[1]);
-
-                    if (absX > absY) {
-                      // Horizontal movement only
-                      constrainedDragOffset = vector(instantDragOffset[0], 0);
-                    } else {
-                      // Vertical movement only
-                      constrainedDragOffset = vector(0, instantDragOffset[1]);
-                    }
-                  }
-
-                  const [x1, y1, x2, y2, cx, cy] = getElementAbsoluteCoords(
-                    croppingElement,
-                    elementsMap,
-                  );
-
-                  const topLeft = vectorFromPoint(
-                    pointRotateRads(
-                      pointFrom(x1, y1),
-                      pointFrom(cx, cy),
-                      croppingElement.angle,
-                    ),
-                  );
-                  const topRight = vectorFromPoint(
-                    pointRotateRads(
-                      pointFrom(x2, y1),
-                      pointFrom(cx, cy),
-                      croppingElement.angle,
-                    ),
-                  );
-                  const bottomLeft = vectorFromPoint(
-                    pointRotateRads(
-                      pointFrom(x1, y2),
-                      pointFrom(cx, cy),
-                      croppingElement.angle,
-                    ),
-                  );
-                  const topEdge = vectorNormalize(
-                    vectorSubtract(topRight, topLeft),
-                  );
-                  const leftEdge = vectorNormalize(
-                    vectorSubtract(bottomLeft, topLeft),
-                  );
-
-                  // project instantDragOffset onto leftEdge and topEdge to decompose
-                  const offsetVector = vector(
-                    vectorDot(constrainedDragOffset, topEdge),
-                    vectorDot(constrainedDragOffset, leftEdge),
-                  );
-
-                  const nextCrop = {
-                    ...crop,
-                    x: clamp(
-                      crop.x -
-                        offsetVector[0] * Math.sign(croppingElement.scale[0]),
-                      0,
-                      image.naturalWidth - crop.width,
-                    ),
-                    y: clamp(
-                      crop.y -
-                        offsetVector[1] * Math.sign(croppingElement.scale[1]),
-                      0,
-                      image.naturalHeight - crop.height,
-                    ),
-                  };
-
-                  this.scene.mutateElement(croppingElement, {
-                    crop: nextCrop,
-                  });
-
-                  return;
-                }
-              }
-            }
-          }
-
           // Snap cache *must* be synchronously popuplated before initial drag,
           // otherwise the first drag even will not snap, causing a jump before
           // it snaps to its position if previously snapped already.
@@ -9072,8 +9007,7 @@ class App extends React.Component<AppProps, AppState> {
       this.setState((prevState) => ({
         isResizing: false,
         isRotating: false,
-        // Keep isCropping true if we were doing crop position movement
+        isCropping: false,
-        isCropping: wasCropPositionMovement,
         resizingElement: null,
         selectionElement: null,
         frameToHighlight: null,
@@ -9609,19 +9543,46 @@ class App extends React.Component<AppProps, AppState> {
       }
 
       // click outside the cropping region to exit
-      if (
+      if (croppingElementId) {
-        // not in the cropping mode at all
+        const croppingElement = this.scene
-        !croppingElementId ||
+          .getNonDeletedElementsMap()
-        // in the cropping mode
+          .get(croppingElementId);
-        (croppingElementId &&
+
-          // not cropping and no hit element (but not doing crop position movement)
+        if (
-          ((!hitElement &&
+          croppingElement &&
-            !isCropping &&
+          isImageElement(croppingElement) &&
-            !pointerDownState.cropPositionMovement.enabled) ||
+          croppingElement.crop
-            // hitting something else
+        ) {
-            (hitElement && hitElement.id !== croppingElementId)))
+          const uncroppedElement = getUncroppedImageElement(
-      ) {
+            croppingElement,
-        this.finishImageCropping();
+            this.scene.getNonDeletedElementsMap(),
+          );
+          const pointer = pointFrom<GlobalPoint>(sceneCoords.x, sceneCoords.y);
+          const hitUncroppedArea = hitElementItself({
+            point: pointer,
+            element: uncroppedElement,
+            threshold: this.getElementHitThreshold(uncroppedElement),
+            elementsMap: this.scene.getNonDeletedElementsMap(),
+          });
+
+          if (!hitUncroppedArea) {
+            this.finishImageCropping();
+          } else {
+            // ensure the image remains selected so crop handles are rendered
+            if (
+              (!this.state.selectedElementIds ||
+                Object.keys(this.state.selectedElementIds).length === 0) &&
+              this.state.croppingElementId
+            ) {
+              this.setState({
+                selectedElementIds: { [this.state.croppingElementId]: true },
+              });
+            }
+          }
+        } else {
+          // fallback: if not in cropping mode or no cropping element, finish cropping
+          this.finishImageCropping();
+        }
       }
 
       const pointerStart = this.lastPointerDownEvent;
@@ -9832,7 +9793,7 @@ class App extends React.Component<AppProps, AppState> {
         ((hitElement &&
           hitElementBoundingBoxOnly(
             {
-              point: pointFrom(
+              point: pointFrom<GlobalPoint>(
                 pointerDownState.origin.x,
                 pointerDownState.origin.y,
               ),
@@ -10784,6 +10745,8 @@ class App extends React.Component<AppProps, AppState> {
       event[KEYS.CTRL_OR_CMD] ? null : this.getEffectiveGridSize(),
     );
 
+    console.log("hi");
+
     const croppingElement = this.scene
       .getNonDeletedElementsMap()
       .get(this.state.croppingElementId);