fix: resolve crop tool regression with stale ref and aspect ratio minSize (refs #120)

Three bugs fixed in the draw-first crop tool introduced by PR #114:

1. Stale cropAreaRef: replaced useEffect-based ref sync with direct
   synchronous updates in handleMove and handleDrawStart. The useEffect
   ran after browser paint, so handleDragEnd read stale values (often
   {width:0, height:0}), preventing cropDrawn from being set.

2. Aspect ratio minSize: when aspectRatio=6 (VIN mode), height=width/6
   required width>=60% to pass the height>=10% check. Now only checks
   width>=minSize when aspect ratio constrains height.

3. Bounds clamping: aspect-ratio-forced height could push crop area
   past 100% of container. Now clamps y position to keep within bounds.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Eric Gullickson
2026-02-07 11:29:16 -06:00
parent 9b6417379b
commit 3c1a090ae3

View File

@@ -95,10 +95,6 @@ export function useImageCrop(options: UseImageCropOptions = {}): UseImageCropRet
const drawOriginRef = useRef({ x: 0, y: 0 }); const drawOriginRef = useRef({ x: 0, y: 0 });
const cropAreaRef = useRef(cropArea); const cropAreaRef = useRef(cropArea);
useEffect(() => {
cropAreaRef.current = cropArea;
}, [cropArea]);
const setCropArea = useCallback( const setCropArea = useCallback(
(area: CropArea) => { (area: CropArea) => {
setCropAreaState(getAspectRatioAdjustedCrop(area)); setCropAreaState(getAspectRatioAdjustedCrop(area));
@@ -177,7 +173,9 @@ export function useImageCrop(options: UseImageCropOptions = {}): UseImageCropRet
startPosRef.current = { x: clientX, y: clientY }; startPosRef.current = { x: clientX, y: clientY };
drawOriginRef.current = { x, y }; drawOriginRef.current = { x, y };
setCropAreaState({ x, y, width: 0, height: 0 }); const initial = { x, y, width: 0, height: 0 };
setCropAreaState(initial);
cropAreaRef.current = initial;
isDrawingRef.current = true; isDrawingRef.current = true;
activeHandleRef.current = null; activeHandleRef.current = null;
@@ -203,18 +201,24 @@ export function useImageCrop(options: UseImageCropOptions = {}): UseImageCropRet
const originX = drawOriginRef.current.x; const originX = drawOriginRef.current.x;
const originY = drawOriginRef.current.y; const originY = drawOriginRef.current.y;
let newCrop: CropArea = { const drawnWidth = Math.abs(currentX - originX);
const drawnHeight = aspectRatio
? drawnWidth / aspectRatio
: Math.abs(currentY - originY);
let drawnY = Math.min(originY, currentY);
// Clamp so crop doesn't exceed container bounds when aspect ratio forces height
if (aspectRatio && drawnY + drawnHeight > 100) {
drawnY = Math.max(0, 100 - drawnHeight);
}
const newCrop: CropArea = {
x: Math.min(originX, currentX), x: Math.min(originX, currentX),
y: Math.min(originY, currentY), y: drawnY,
width: Math.abs(currentX - originX), width: drawnWidth,
height: Math.abs(currentY - originY), height: drawnHeight,
}; };
if (aspectRatio) {
newCrop.height = newCrop.width / aspectRatio;
}
setCropAreaState(newCrop); setCropAreaState(newCrop);
cropAreaRef.current = newCrop;
return; return;
} }
@@ -303,7 +307,9 @@ export function useImageCrop(options: UseImageCropOptions = {}): UseImageCropRet
break; break;
} }
setCropAreaState(constrainCrop(newCrop)); const constrained = constrainCrop(newCrop);
setCropAreaState(constrained);
cropAreaRef.current = constrained;
}, },
[isDragging, constrainCrop, aspectRatio] [isDragging, constrainCrop, aspectRatio]
); );
@@ -312,13 +318,17 @@ export function useImageCrop(options: UseImageCropOptions = {}): UseImageCropRet
if (isDrawingRef.current) { if (isDrawingRef.current) {
isDrawingRef.current = false; isDrawingRef.current = false;
const area = cropAreaRef.current; const area = cropAreaRef.current;
if (area.width >= minSize && area.height >= minSize) { // When aspect ratio constrains one dimension, only check the free dimension
const meetsMinSize = aspectRatio
? area.width >= minSize
: area.width >= minSize && area.height >= minSize;
if (meetsMinSize) {
setCropDrawn(true); setCropDrawn(true);
} }
} }
activeHandleRef.current = null; activeHandleRef.current = null;
setIsDragging(false); setIsDragging(false);
}, [minSize]); }, [minSize, aspectRatio]);
// Add global event listeners for drag // Add global event listeners for drag
useEffect(() => { useEffect(() => {