From 0fc00ceeff220ad84351580eb660748258337d5d Mon Sep 17 00:00:00 2001 From: Florian Sylvain Date: Mon, 28 Sep 2026 07:56:13 +0200 Subject: [PATCH] refac: cleaning round for image cropping/resizing --- web/src/components/ImageUploader.vue | 78 +++++------ .../ImageUploaderCropAndResize.spec.ts | 50 ++++++++ web/src/lib/__tests__/imageCrop.spec.ts | 121 ++++++++++++++++++ 3 files changed, 211 insertions(+), 38 deletions(-) diff --git a/web/src/components/ImageUploader.vue b/web/src/components/ImageUploader.vue index 8f5ef26..3a4458d 100644 --- a/web/src/components/ImageUploader.vue +++ b/web/src/components/ImageUploader.vue @@ -357,16 +357,27 @@ function pointFromEvent(event: MouseEvent | TouchEvent): { x: number; y: number return { x: mouseEvent.clientX, y: mouseEvent.clientY } } +function bindPointerDrag(onMove: (event: MouseEvent | TouchEvent) => void, onEnd: () => void) { + window.addEventListener('mousemove', onMove) + window.addEventListener('mouseup', onEnd) + window.addEventListener('touchmove', onMove) + window.addEventListener('touchend', onEnd) +} + +function unbindPointerDrag(onMove: (event: MouseEvent | TouchEvent) => void, onEnd: () => void) { + window.removeEventListener('mousemove', onMove) + window.removeEventListener('mouseup', onEnd) + window.removeEventListener('touchmove', onMove) + window.removeEventListener('touchend', onEnd) +} + let moveOrigin: { x: number; y: number; rect: CropRect } | null = null function onFrameDragStart(event: MouseEvent | TouchEvent) { event.preventDefault() const point = pointFromEvent(event) moveOrigin = { x: point.x, y: point.y, rect: { ...rect.value } } - window.addEventListener('mousemove', onFrameDragMove) - window.addEventListener('mouseup', onFrameDragEnd) - window.addEventListener('touchmove', onFrameDragMove) - window.addEventListener('touchend', onFrameDragEnd) + bindPointerDrag(onFrameDragMove, onFrameDragEnd) } function onFrameDragMove(event: MouseEvent | TouchEvent) { @@ -386,10 +397,7 @@ function onFrameDragMove(event: MouseEvent | TouchEvent) { function onFrameDragEnd() { moveOrigin = null - window.removeEventListener('mousemove', onFrameDragMove) - window.removeEventListener('mouseup', onFrameDragEnd) - window.removeEventListener('touchmove', onFrameDragMove) - window.removeEventListener('touchend', onFrameDragEnd) + unbindPointerDrag(onFrameDragMove, onFrameDragEnd) } let resizeOrigin: { @@ -411,10 +419,7 @@ function onHandleDragStart(event: MouseEvent | TouchEvent, corner: Corner) { corner, ratio: lockedRatioValue.value, } - window.addEventListener('mousemove', onHandleDragMove) - window.addEventListener('mouseup', onHandleDragEnd) - window.addEventListener('touchmove', onHandleDragMove) - window.addEventListener('touchend', onHandleDragEnd) + bindPointerDrag(onHandleDragMove, onHandleDragEnd) } function onHandleDragMove(event: MouseEvent | TouchEvent) { @@ -450,36 +455,33 @@ function onHandleDragMove(event: MouseEvent | TouchEvent) { function onHandleDragEnd() { resizeOrigin = null - window.removeEventListener('mousemove', onHandleDragMove) - window.removeEventListener('mouseup', onHandleDragEnd) - window.removeEventListener('touchmove', onHandleDragMove) - window.removeEventListener('touchend', onHandleDragEnd) + unbindPointerDrag(onHandleDragMove, onHandleDragEnd) +} + +const ARROW_KEY_DELTAS: Record = { + ArrowRight: [1, 0], + ArrowLeft: [-1, 0], + ArrowUp: [0, -1], + ArrowDown: [0, 1], } function onFrameKeydown(event: KeyboardEvent) { - const step = 1 - const lockedRatio = lockedRatioValue.value - if (event.key === 'ArrowRight') { - event.preventDefault() - rect.value = event.shiftKey - ? resizeRect(rect.value, step, 0, boundsWidth.value, boundsHeight.value, lockedRatio) - : moveRect(rect.value, step, 0, boundsWidth.value, boundsHeight.value) - } else if (event.key === 'ArrowLeft') { - event.preventDefault() - rect.value = event.shiftKey - ? resizeRect(rect.value, -step, 0, boundsWidth.value, boundsHeight.value, lockedRatio) - : moveRect(rect.value, -step, 0, boundsWidth.value, boundsHeight.value) - } else if (event.key === 'ArrowUp') { - event.preventDefault() - rect.value = event.shiftKey - ? resizeRect(rect.value, 0, -step, boundsWidth.value, boundsHeight.value, lockedRatio) - : moveRect(rect.value, 0, -step, boundsWidth.value, boundsHeight.value) - } else if (event.key === 'ArrowDown') { - event.preventDefault() - rect.value = event.shiftKey - ? resizeRect(rect.value, 0, step, boundsWidth.value, boundsHeight.value, lockedRatio) - : moveRect(rect.value, 0, step, boundsWidth.value, boundsHeight.value) + const delta = ARROW_KEY_DELTAS[event.key] + if (!delta) { + return } + event.preventDefault() + const [dx, dy] = delta + rect.value = event.shiftKey + ? resizeRect( + rect.value, + dx, + dy, + boundsWidth.value, + boundsHeight.value, + lockedRatioValue.value, + ) + : moveRect(rect.value, dx, dy, boundsWidth.value, boundsHeight.value) } async function confirmCropUpload() { diff --git a/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts b/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts index 31b007b..187fc41 100644 --- a/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts +++ b/web/src/components/__tests__/ImageUploaderCropAndResize.spec.ts @@ -340,6 +340,56 @@ describe('Arrow keys move the crop frame and Shift+arrow keys resize it', () => }) }) +describe('Dragging the crop frame moves it, and dragging a corner handle resizes it', () => { + it('clamps the dragged rect inside the image bounds', async () => { + const wrapper = mountUploader() + await openModal(wrapper) + await chooseFile(north_wall()) + await selectRatioPill('1:1') + const uploader = wrapper.findComponent(ImageUploader).vm as unknown as { + cropRect: CropRect + } + const before = { ...uploader.cropRect } + + cropFrame().dispatchEvent( + new MouseEvent('mousedown', { clientX: 0, clientY: 0, bubbles: true }), + ) + window.dispatchEvent(new MouseEvent('mousemove', { clientX: 100, clientY: 40 })) + window.dispatchEvent(new MouseEvent('mouseup')) + await nextTick() + + expect(uploader.cropRect.x).toBe(before.x + 100) + expect(uploader.cropRect.y).toBe(before.y) + expect(uploader.cropRect.width).toBe(before.width) + expect(uploader.cropRect.height).toBe(before.height) + + window.dispatchEvent(new MouseEvent('mousemove', { clientX: 999, clientY: 0 })) + await nextTick() + + expect(uploader.cropRect.x).toBe(before.x + 100) + + const handle = document.body.querySelectorAll( + '[data-testid="crop-handle"]', + )[3]! + handle.dispatchEvent(new MouseEvent('mousedown', { clientX: 0, clientY: 0, bubbles: true })) + const afterMove = { ...uploader.cropRect } + window.dispatchEvent(new MouseEvent('mousemove', { clientX: -50, clientY: -50 })) + window.dispatchEvent(new MouseEvent('mouseup')) + await nextTick() + + expect(uploader.cropRect.width).toBe(afterMove.width - 50) + expect(uploader.cropRect.height).toBe(afterMove.height - 50) + expect(uploader.cropRect.x).toBe(afterMove.x) + expect(uploader.cropRect.y).toBe(afterMove.y) + + window.dispatchEvent(new MouseEvent('mousemove', { clientX: -9999, clientY: 0 })) + await nextTick() + + expect(uploader.cropRect.width).toBe(afterMove.width - 50) + wrapper.unmount() + }) +}) + describe('Rotate cycles the image in 90° steps and Reset returns to the opened defaults', () => { it('rotates then resets', async () => { const wrapper = mountUploader() diff --git a/web/src/lib/__tests__/imageCrop.spec.ts b/web/src/lib/__tests__/imageCrop.spec.ts index 6b650aa..1249e39 100644 --- a/web/src/lib/__tests__/imageCrop.spec.ts +++ b/web/src/lib/__tests__/imageCrop.spec.ts @@ -1,11 +1,16 @@ import { describe, expect, it } from 'vitest' import { + clampRect, computeOutputSize, estimateOutputBytes, formatEstimatedSize, + moveRect, + ratioValueFor, rectForRatio, + resizeRect, rotateRectBySteps, rotatedDimensions, + widthPresetPixels, wholeRect, } from '../imageCrop' @@ -41,6 +46,122 @@ describe('estimateOutputBytes', () => { }) }) +describe('formatEstimatedSize', () => { + it('stays in KB just under the 1000 KB threshold', () => { + expect(formatEstimatedSize(999 * 1024)).toBe('999 KB') + }) + + it('switches to MB at and above the 1000 KB threshold', () => { + expect(formatEstimatedSize(1024 * 1024 * 2.5)).toBe('2.5 MB') + }) +}) + +describe('clampRect', () => { + it('rounds fractional coordinates and dimensions to whole pixels', () => { + expect(clampRect({ x: 100.2, y: 50.7, width: 200.4, height: 150.6 }, 4032, 3024)).toEqual({ + x: 100, + y: 51, + width: 200, + height: 151, + }) + }) + + it('clamps width and height to the bounds', () => { + expect(clampRect({ x: 0, y: 0, width: 5000, height: 4000 }, 4032, 3024)).toEqual({ + x: 0, + y: 0, + width: 4032, + height: 3024, + }) + }) + + it('clamps x and y so the rect stays inside the bounds', () => { + expect(clampRect({ x: 4000, y: 3000, width: 3024, height: 3024 }, 4032, 3024)).toEqual({ + x: 1008, + y: 0, + width: 3024, + height: 3024, + }) + }) + + it('never lets width or height drop below 1px', () => { + expect(clampRect({ x: 0, y: 0, width: -10, height: 0 }, 4032, 3024)).toEqual({ + x: 0, + y: 0, + width: 1, + height: 1, + }) + }) +}) + +describe('moveRect', () => { + it('offsets the rect and clamps it inside the bounds', () => { + const rect = { x: 100, y: 100, width: 200, height: 200 } + expect(moveRect(rect, 50, -50, 4032, 3024)).toEqual({ + x: 150, + y: 50, + width: 200, + height: 200, + }) + }) + + it('stops at the bound edge instead of moving past it', () => { + const rect = { x: 0, y: 0, width: 200, height: 200 } + expect(moveRect(rect, -50, -50, 4032, 3024)).toEqual({ + x: 0, + y: 0, + width: 200, + height: 200, + }) + }) +}) + +describe('resizeRect', () => { + it('grows freely on both axes when no ratio is locked', () => { + const rect = { x: 0, y: 0, width: 200, height: 200 } + expect(resizeRect(rect, 20, 10, 4032, 3024, null)).toEqual({ + x: 0, + y: 0, + width: 220, + height: 210, + }) + }) + + it('derives height from width when a ratio is locked and width changes', () => { + const rect = { x: 0, y: 0, width: 200, height: 200 } + expect(resizeRect(rect, 100, 0, 4032, 3024, 16 / 9)).toEqual({ + x: 0, + y: 0, + width: 300, + height: 169, + }) + }) +}) + +describe('ratioValueFor', () => { + it('returns null for Free so the frame is unconstrained', () => { + expect(ratioValueFor('Free', 4032, 3024)).toBeNull() + }) + + it('derives Original from the current bounds', () => { + expect(ratioValueFor('Original', 4032, 3024)).toBeCloseTo(4032 / 3024) + }) + + it('returns the fixed value for a named ratio', () => { + expect(ratioValueFor('1:1', 4032, 3024)).toBe(1) + }) +}) + +describe('widthPresetPixels', () => { + it('returns null for Original so the crop keeps its own width', () => { + expect(widthPresetPixels('Original')).toBeNull() + }) + + it('returns the pixel target for a fixed preset', () => { + expect(widthPresetPixels('800 px')).toBe(800) + }) +}) + describe('rotateRectBySteps', () => { it('rotates a whole-image rect 90deg clockwise into the new bounds', () => { const rotated = rotateRectBySteps(wholeRect(4032, 3024), 4032, 3024, 1)