diff --git a/MODULE.bazel.lock b/MODULE.bazel.lock index adb4eea1..b9f7f63e 100644 --- a/MODULE.bazel.lock +++ b/MODULE.bazel.lock @@ -292,7 +292,7 @@ "bzlTransitiveDigest": "3GB+HVbmUfi1gaE0kWIzYPstolh7vaSq+4B96AikLiA=", "usagesDigest": "ZXxKBmt+bDvqYeAjgAl/x/xI/QLW5EbehLOG2a9mYMM=", "recordedFileInputs": { - "@@//web/package.json": "42d058f510237a50981a9bc8e216bb7e6e7ab0fb39ea06bb3baf2713bd8246c4" + "@@//web/package.json": "a9adcddd476ace94df14633a871e483d0fe5cbdb2d83910e811540e296871e8f" }, "recordedDirentsInputs": {}, "envVariables": {}, diff --git a/web/BUILD.bazel b/web/BUILD.bazel index 317fdff5..2b70c643 100644 --- a/web/BUILD.bazel +++ b/web/BUILD.bazel @@ -301,8 +301,8 @@ js_library( ":node_modules/masonic", ":node_modules/qrcode.react", ":node_modules/react", + ":node_modules/react-advanced-cropper", ":node_modules/react-dom", - ":node_modules/react-easy-crop", ":node_modules/react-router-dom", ":node_modules/react-sketch-canvas", ":node_modules/react-swipeable", @@ -340,7 +340,7 @@ js_library( deps = [ ":generated_types", ":node_modules", - ":node_modules/react-easy-crop", + ":node_modules/react-advanced-cropper", ":util_lib", "//:workflow_js", ], @@ -414,7 +414,7 @@ js_library( srcs = ["src/components/CropOverlay.tsx"], deps = [ ":node_modules", - ":node_modules/react-easy-crop", + ":node_modules/react-advanced-cropper", ":util_lib", ], ) @@ -745,7 +745,7 @@ js_library( deps = [ ":generated_types", ":node_modules", - ":node_modules/react-easy-crop", + ":node_modules/react-advanced-cropper", ":ocr_detection_src", ":util_lib", ], diff --git a/web/package-lock.json b/web/package-lock.json index 794b689e..bbd7e574 100644 --- a/web/package-lock.json +++ b/web/package-lock.json @@ -33,8 +33,8 @@ "mui": "^0.0.1", "qrcode.react": "^4.2.0", "react": "^19.2.4", + "react-advanced-cropper": "^0.20.1", "react-dom": "^19.2.4", - "react-easy-crop": "^5.5.7", "react-router-dom": "^7.14.0", "react-sketch-canvas": "^6.2.0", "react-swipeable": "^7.0.2", @@ -4393,6 +4393,19 @@ "acorn": "^6.0.0 || ^7.0.0 || ^8.0.0" } }, + "node_modules/advanced-cropper": { + "version": "0.17.1", + "resolved": "https://registry.npmjs.org/advanced-cropper/-/advanced-cropper-0.17.1.tgz", + "integrity": "sha512-Z1P0sYOXa2tqZjeY742QtNERofXh1AuOa27LEurO9rbx3IfzLrGQlzy7sWEc5VN9hRg+J/qCiMmnB6tUDLb1TA==", + "license": "MIT", + "dependencies": { + "tslib": "^2.4.0" + }, + "engines": { + "node": ">=8", + "npm": ">=5" + } + }, "node_modules/agent-base": { "version": "7.1.4", "resolved": "https://registry.npmjs.org/agent-base/-/agent-base-7.1.4.tgz", @@ -4892,6 +4905,12 @@ "integrity": "sha512-4bHTS2YuzUvtoLjdy+98ykbNB5jS0+07EvFNXerqZQJ89F7DI6ET7OQo/HJuW6K0aVsKA9hj9/RVb2kQVOrPDQ==", "license": "MIT" }, + "node_modules/classnames": { + "version": "2.5.1", + "resolved": "https://registry.npmjs.org/classnames/-/classnames-2.5.1.tgz", + "integrity": "sha512-saHYOzhIQs6wy2sVxTM6bUDsQO4F50V9RQ22qBpEdCW+I+/Wmke2HOl6lS6dTpdxVhb88/I6+Hs+438c3lfUow==", + "license": "MIT" + }, "node_modules/cli-width": { "version": "4.1.0", "resolved": "https://registry.npmjs.org/cli-width/-/cli-width-4.1.0.tgz", @@ -7515,12 +7534,6 @@ "dev": true, "license": "MIT" }, - "node_modules/normalize-wheel": { - "version": "1.0.1", - "resolved": "https://registry.npmjs.org/normalize-wheel/-/normalize-wheel-1.0.1.tgz", - "integrity": "sha512-1OnlAPZ3zgrk8B91HyRj+eVv+kS5u+Z0SCsak6Xil/kmgEia50ga7zfkumayonZrImffAxPU/5WcyGhzetHNPA==", - "license": "BSD-3-Clause" - }, "node_modules/object-assign": { "version": "4.1.1", "resolved": "https://registry.npmjs.org/object-assign/-/object-assign-4.1.1.tgz", @@ -8103,6 +8116,24 @@ "node": ">=0.10.0" } }, + "node_modules/react-advanced-cropper": { + "version": "0.20.1", + "resolved": "https://registry.npmjs.org/react-advanced-cropper/-/react-advanced-cropper-0.20.1.tgz", + "integrity": "sha512-Pcmkv0xQMpig6+LkM+zLbEuqBbYG3+CwXvIfYU+LDNn9l8t91Jm0fp9MSTNW0pjIvT6frAGTfmlnvnZW4PEs7Q==", + "license": "MIT", + "dependencies": { + "advanced-cropper": "~0.17.1", + "classnames": "^2.2.6", + "tslib": "^2.4.0" + }, + "engines": { + "node": ">=8", + "npm": ">=5" + }, + "peerDependencies": { + "react": ">=16.8.0" + } + }, "node_modules/react-docgen": { "version": "7.1.1", "resolved": "https://registry.npmjs.org/react-docgen/-/react-docgen-7.1.1.tgz", @@ -8160,20 +8191,6 @@ "react": "^19.2.4" } }, - "node_modules/react-easy-crop": { - "version": "5.5.7", - "resolved": "https://registry.npmjs.org/react-easy-crop/-/react-easy-crop-5.5.7.tgz", - "integrity": "sha512-kYo4NtMeXFQB7h1U+h5yhUkE46WQbQdq7if54uDlbMdZHdRgNehfvaFrXnFw5NR1PNoUOJIfTwLnWmEx/MaZnA==", - "license": "MIT", - "dependencies": { - "normalize-wheel": "^1.0.1", - "tslib": "^2.0.1" - }, - "peerDependencies": { - "react": ">=16.4.0", - "react-dom": ">=16.4.0" - } - }, "node_modules/react-is": { "version": "19.2.4", "resolved": "https://registry.npmjs.org/react-is/-/react-is-19.2.4.tgz", diff --git a/web/package.json b/web/package.json index cb53e669..e254844e 100644 --- a/web/package.json +++ b/web/package.json @@ -40,8 +40,8 @@ "mui": "^0.0.1", "qrcode.react": "^4.2.0", "react": "^19.2.4", + "react-advanced-cropper": "^0.20.1", "react-dom": "^19.2.4", - "react-easy-crop": "^5.5.7", "react-router-dom": "^7.14.0", "react-sketch-canvas": "^6.2.0", "react-swipeable": "^7.0.2", diff --git a/web/pnpm-lock.yaml b/web/pnpm-lock.yaml index 68ecf874..2e41a2b0 100644 --- a/web/pnpm-lock.yaml +++ b/web/pnpm-lock.yaml @@ -83,12 +83,12 @@ importers: react: specifier: ^19.2.4 version: 19.2.4 + react-advanced-cropper: + specifier: ^0.20.1 + version: 0.20.1(react@19.2.4) react-dom: specifier: ^19.2.4 version: 19.2.4(react@19.2.4) - react-easy-crop: - specifier: ^5.5.7 - version: 5.5.7(react-dom@19.2.4(react@19.2.4))(react@19.2.4) react-router-dom: specifier: ^7.14.0 version: 7.14.0(react-dom@19.2.4(react@19.2.4))(react@19.2.4) @@ -1600,6 +1600,10 @@ packages: engines: {node: '>=0.4.0'} hasBin: true + advanced-cropper@0.17.1: + resolution: {integrity: sha512-Z1P0sYOXa2tqZjeY742QtNERofXh1AuOa27LEurO9rbx3IfzLrGQlzy7sWEc5VN9hRg+J/qCiMmnB6tUDLb1TA==} + engines: {node: '>=8', npm: '>=5'} + agent-base@7.1.4: resolution: {integrity: sha512-MnA+YT8fwfJPgBx3m60MNqakm30XOkyIoH1y6huTQvC0PwZG7ki8NacLBcrPbNoo8vEZy7Jpuk7+jMO+CUovTQ==} engines: {node: '>= 14'} @@ -1769,6 +1773,9 @@ packages: cjs-module-lexer@2.2.0: resolution: {integrity: sha512-4bHTS2YuzUvtoLjdy+98ykbNB5jS0+07EvFNXerqZQJ89F7DI6ET7OQo/HJuW6K0aVsKA9hj9/RVb2kQVOrPDQ==} + classnames@2.5.1: + resolution: {integrity: sha512-saHYOzhIQs6wy2sVxTM6bUDsQO4F50V9RQ22qBpEdCW+I+/Wmke2HOl6lS6dTpdxVhb88/I6+Hs+438c3lfUow==} + cli-width@4.1.0: resolution: {integrity: sha512-ouuZd4/dm2Sw5Gmqy6bGyNNNe1qt9RpmxveLSO7KcgsTnU7RXfsw+/bukWGo1abgBiMAic068rclZsO4IWmmxQ==} engines: {node: '>= 12'} @@ -2640,9 +2647,6 @@ packages: node-releases@2.0.37: resolution: {integrity: sha512-1h5gKZCF+pO/o3Iqt5Jp7wc9rH3eJJ0+nh/CIoiRwjRxde/hAHyLPXYN4V3CqKAbiZPSeJFSWHmJsbkicta0Eg==} - normalize-wheel@1.0.1: - resolution: {integrity: sha512-1OnlAPZ3zgrk8B91HyRj+eVv+kS5u+Z0SCsak6Xil/kmgEia50ga7zfkumayonZrImffAxPU/5WcyGhzetHNPA==} - object-assign@4.1.1: resolution: {integrity: sha512-rJgTQnkUnH1sFw8yT6VSU3zD3sWmu6sZhIseY8VX+GRu3P6F7Fu+JNDoXfklElbLJSnc3FUQHVe4cU5hj+BcUg==} engines: {node: '>=0.10.0'} @@ -2815,6 +2819,12 @@ packages: resolution: {integrity: sha512-y3bGgqKj3QBdxLbLkomlohkvsA8gdAiUQlSBJnBhfn+BPxg4bc62d8TcBW15wavDfgexCgccckhcZvywyQYPOw==} hasBin: true + react-advanced-cropper@0.20.1: + resolution: {integrity: sha512-Pcmkv0xQMpig6+LkM+zLbEuqBbYG3+CwXvIfYU+LDNn9l8t91Jm0fp9MSTNW0pjIvT6frAGTfmlnvnZW4PEs7Q==} + engines: {node: '>=8', npm: '>=5'} + peerDependencies: + react: '>=16.8.0' + react-docgen-typescript@2.4.0: resolution: {integrity: sha512-ZtAp5XTO5HRzQctjPU0ybY0RRCQO19X/8fxn3w7y2VVTUbGHDKULPTL4ky3vB05euSgG5NpALhEhDPvQ56wvXg==} peerDependencies: @@ -2829,12 +2839,6 @@ packages: peerDependencies: react: ^19.2.4 - react-easy-crop@5.5.7: - resolution: {integrity: sha512-kYo4NtMeXFQB7h1U+h5yhUkE46WQbQdq7if54uDlbMdZHdRgNehfvaFrXnFw5NR1PNoUOJIfTwLnWmEx/MaZnA==} - peerDependencies: - react: '>=16.4.0' - react-dom: '>=16.4.0' - react-is@16.13.1: resolution: {integrity: sha512-24e6ynE2H+OKt4kqsOvNd8kBpV65zoxbA4BVsEOB3ARVWQki/DHzaUoC5KuON/BiccDaCCTZBuOcfZs70kR8bQ==} @@ -4901,6 +4905,10 @@ snapshots: acorn@8.16.0: {} + advanced-cropper@0.17.1: + dependencies: + tslib: 2.8.1 + agent-base@7.1.4: {} ajv@6.14.0: @@ -5076,6 +5084,8 @@ snapshots: cjs-module-lexer@2.2.0: {} + classnames@2.5.1: {} + cli-width@4.1.0: {} cliui@8.0.1: @@ -5911,8 +5921,6 @@ snapshots: node-releases@2.0.37: {} - normalize-wheel@1.0.1: {} - object-assign@4.1.1: {} obug@2.1.1: {} @@ -6083,6 +6091,13 @@ snapshots: minimist: 1.2.8 strip-json-comments: 2.0.1 + react-advanced-cropper@0.20.1(react@19.2.4): + dependencies: + advanced-cropper: 0.17.1 + classnames: 2.5.1 + react: 19.2.4 + tslib: 2.8.1 + react-docgen-typescript@2.4.0(typescript@5.9.3): dependencies: typescript: 5.9.3 @@ -6107,13 +6122,6 @@ snapshots: react: 19.2.4 scheduler: 0.27.0 - react-easy-crop@5.5.7(react-dom@19.2.4(react@19.2.4))(react@19.2.4): - dependencies: - normalize-wheel: 1.0.1 - react: 19.2.4 - react-dom: 19.2.4(react@19.2.4) - tslib: 2.8.1 - react-is@16.13.1: {} react-is@17.0.2: {} diff --git a/web/src/components/CropOverlay.tsx b/web/src/components/CropOverlay.tsx index 2f820802..726aca65 100644 --- a/web/src/components/CropOverlay.tsx +++ b/web/src/components/CropOverlay.tsx @@ -3,8 +3,9 @@ import { Alert, Box, Button, CircularProgress } from "@mui/material"; import { Cloudinary } from "@cloudinary/url-gen"; import { format } from "@cloudinary/url-gen/actions/delivery"; import { auto as autoFormat } from "@cloudinary/url-gen/qualifiers/format"; -import Cropper from "react-easy-crop"; -import type { Area } from "react-easy-crop"; +import { Cropper, RectangleStencil, ImageRestriction } from "react-advanced-cropper"; +import type { CropperRef } from "react-advanced-cropper"; +import "react-advanced-cropper/dist/style.css"; import type { ImageCrop } from "../util/types"; interface CropOverlayProps { @@ -24,19 +25,25 @@ function buildUncroppedUrl(publicId: string, cloudName: string): string { const DEFAULT_IMAGE_CROP: ImageCrop = { x: 0, y: 0, width: 1, height: 1 }; -function toImageCrop(area: Area): ImageCrop { +/** + * Convert a react-advanced-cropper coordinate (image pixels) into the + * fraction-based ImageCrop the backend stores. Free-form crops yield + * independent width/height fractions. + */ +function toImageCrop( + coords: { left: number; top: number; width: number; height: number }, + imageSize: { width: number; height: number }, +): ImageCrop { return { - x: area.x / 100, - y: area.y / 100, - width: area.width / 100, - height: area.height / 100, + x: coords.left / imageSize.width, + y: coords.top / imageSize.height, + width: coords.width / imageSize.width, + height: coords.height / imageSize.height, }; } type State = { imageLoading: boolean; - crop: { x: number; y: number }; - zoom: number; committedCrop: ImageCrop; saving: boolean; saveError: string | null; @@ -44,9 +51,7 @@ type State = { type Action = | { type: "IMAGE_LOADED" } - | { type: "CROP_CHANGE"; crop: { x: number; y: number } } - | { type: "ZOOM_CHANGE"; zoom: number } - | { type: "CROP_COMPLETE"; area: Area } + | { type: "CROP_COMPLETE"; crop: ImageCrop } | { type: "SAVE_START" } | { type: "SAVE_SUCCESS" } | { type: "SAVE_ERROR"; error: string }; @@ -55,12 +60,8 @@ function reducer(state: State, action: Action): State { switch (action.type) { case "IMAGE_LOADED": return { ...state, imageLoading: false }; - case "CROP_CHANGE": - return { ...state, crop: action.crop }; - case "ZOOM_CHANGE": - return { ...state, zoom: action.zoom }; case "CROP_COMPLETE": - return { ...state, committedCrop: toImageCrop(action.area) }; + return { ...state, committedCrop: action.crop }; case "SAVE_START": return { ...state, saving: true, saveError: null }; case "SAVE_SUCCESS": @@ -79,8 +80,6 @@ export default function CropOverlay({ }: CropOverlayProps) { const [state, dispatch] = useReducer(reducer, { imageLoading: true, - crop: { x: 0, y: 0 }, - zoom: 1, committedCrop: initialCrop ?? DEFAULT_IMAGE_CROP, saving: false, saveError: null, @@ -91,6 +90,13 @@ export default function CropOverlay({ [cloudinaryPublicId, cloudName], ); + function handleChange(cropper: CropperRef) { + const coords = cropper.getCoordinates(); + const imageSize = cropper.getState()?.imageSize; + if (!coords || !imageSize || !imageSize.width || !imageSize.height) return; + dispatch({ type: "CROP_COMPLETE", crop: toImageCrop(coords, imageSize) }); + } + async function handleSave() { dispatch({ type: "SAVE_START" }); try { @@ -135,25 +141,33 @@ export default function CropOverlay({ )} initialCrop ? { - x: initialCrop.x * 100, - y: initialCrop.y * 100, - width: initialCrop.width * 100, - height: initialCrop.height * 100, + width: initialCrop.width * imageSize.width, + height: initialCrop.height * imageSize.height, } - : undefined + : { width: imageSize.width, height: imageSize.height } } - onCropChange={(crop) => dispatch({ type: "CROP_CHANGE", crop })} - onZoomChange={(zoom) => dispatch({ type: "ZOOM_CHANGE", zoom })} - onCropComplete={(croppedArea) => - dispatch({ type: "CROP_COMPLETE", area: croppedArea }) + defaultPosition={({ imageSize }: { imageSize: { width: number; height: number } }) => + initialCrop + ? { + left: initialCrop.x * imageSize.width, + top: initialCrop.y * imageSize.height, + } + : { left: 0, top: 0 } } - onMediaLoaded={() => dispatch({ type: "IMAGE_LOADED" })} + onChange={handleChange} + onReady={(cropper: CropperRef) => { + handleChange(cropper); + dispatch({ type: "IMAGE_LOADED" }); + }} /> {state.saveError && ( diff --git a/web/src/components/__tests__/CropOverlay.test.tsx b/web/src/components/__tests__/CropOverlay.test.tsx index 3c5c324e..e3b266d7 100644 --- a/web/src/components/__tests__/CropOverlay.test.tsx +++ b/web/src/components/__tests__/CropOverlay.test.tsx @@ -1,15 +1,38 @@ import React from "react"; import { render, screen, fireEvent, waitFor } from "@testing-library/react"; -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi, beforeEach } from "vitest"; -vi.mock("react-easy-crop", () => ({ - default: function MockCropper({ onMediaLoaded }: any) { - const onMediaLoadedRef = React.useRef(onMediaLoaded); +// Captures the props the component passes to the cropper so tests can assert on +// them (e.g. that no fixed aspect ratio is imposed — the #737 regression). +let lastCropperProps: any = null; + +// Fake cropper "ref" handed to onChange. getCoordinates returns a deliberately +// NON-square region (width !== height) so a fixed aspect ratio would be detectable. +const fakeCropper = { + getCoordinates: () => ({ left: 10, top: 20, width: 30, height: 40 }), + getState: () => ({ imageSize: { width: 100, height: 100 } }), +}; + +vi.mock("react-advanced-cropper", () => ({ + Cropper: function MockCropper(props: any) { + lastCropperProps = props; + const propsRef = React.useRef(props); + propsRef.current = props; React.useEffect(() => { - onMediaLoadedRef.current?.({ naturalWidth: 100, naturalHeight: 100 }); + propsRef.current.onReady?.(fakeCropper); + propsRef.current.onChange?.(fakeCropper); }, []); return
; }, + RectangleStencil: function MockRectangleStencil() { + return null; + }, + ImageRestriction: { + fillArea: "fillArea", + fitArea: "fitArea", + stencil: "stencil", + none: "none", + }, })); vi.mock("@cloudinary/url-gen", () => { @@ -42,11 +65,25 @@ const DEFAULT_PROPS = { }; describe("CropOverlay", () => { + beforeEach(() => { + lastCropperProps = null; + }); + it("renders the crop editor", () => { render(); expect(screen.getByTestId("mock-cropper")).toBeInTheDocument(); }); + // Regression for #737: the crop must be free-form — no fixed aspect ratio + // imposed on the stencil. + it("uses free-form cropping (no fixed aspect ratio)", () => { + render(); + const stencilProps = lastCropperProps?.stencilProps ?? {}; + expect(stencilProps.aspectRatio).toBeUndefined(); + expect(stencilProps.minAspectRatio).toBeUndefined(); + expect(stencilProps.maxAspectRatio).toBeUndefined(); + }); + it("Cancel button calls onCancel without calling onSave", async () => { const onCancel = vi.fn(); const onSave = vi.fn(); @@ -58,17 +95,15 @@ describe("CropOverlay", () => { expect(onSave).not.toHaveBeenCalled(); }); - it("Save Crop button calls onSave with a valid crop", async () => { + it("Save Crop persists the free-form crop with independent width/height", async () => { const onSave = vi.fn().mockResolvedValue(undefined); render(); fireEvent.click(screen.getByText("Save Crop")); await waitFor(() => expect(onSave).toHaveBeenCalledOnce()); const [crop] = onSave.mock.calls[0]; - expect(crop).toMatchObject({ - x: expect.any(Number), - y: expect.any(Number), - width: expect.any(Number), - height: expect.any(Number), - }); + // fakeCropper: {left:10, top:20, width:30, height:40} over a 100×100 image. + expect(crop).toMatchObject({ x: 0.1, y: 0.2, width: 0.3, height: 0.4 }); + // Crucially, width !== height — a fixed aspect ratio could not produce this. + expect(crop.width).not.toBe(crop.height); }); }); diff --git a/web/src/components/__tests__/ImageLightbox.test.tsx b/web/src/components/__tests__/ImageLightbox.test.tsx index c56036ae..a4a49944 100644 --- a/web/src/components/__tests__/ImageLightbox.test.tsx +++ b/web/src/components/__tests__/ImageLightbox.test.tsx @@ -5,14 +5,26 @@ import userEvent from "@testing-library/user-event"; import { Dialog, Box } from "@mui/material"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; -vi.mock("react-easy-crop", () => ({ - default: function MockCropper({ onMediaLoaded }: any) { - const onMediaLoadedRef = React.useRef(onMediaLoaded); +vi.mock("react-advanced-cropper", () => ({ + Cropper: function MockCropper({ onReady }: any) { + const onReadyRef = React.useRef(onReady); React.useEffect(() => { - onMediaLoadedRef.current?.({ naturalWidth: 100, naturalHeight: 100 }); + onReadyRef.current?.({ + getCoordinates: () => ({ left: 0, top: 0, width: 100, height: 100 }), + getState: () => ({ imageSize: { width: 100, height: 100 } }), + }); }, []); return
; }, + RectangleStencil: function MockRectangleStencil() { + return null; + }, + ImageRestriction: { + fillArea: "fillArea", + fitArea: "fitArea", + stencil: "stencil", + none: "none", + }, })); vi.mock("../CloudinaryImage", async (importOriginal) => { diff --git a/web/src/pages/__tests__/GlazeImportCropStage.test.tsx b/web/src/pages/__tests__/GlazeImportCropStage.test.tsx index 7f73370f..82a35333 100644 --- a/web/src/pages/__tests__/GlazeImportCropStage.test.tsx +++ b/web/src/pages/__tests__/GlazeImportCropStage.test.tsx @@ -1,22 +1,36 @@ -import { useState } from "react"; +import React, { useState } from "react"; import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { describe, expect, it, vi } from "vitest"; +import { describe, expect, it, vi, beforeEach } from "vitest"; -vi.mock("react-easy-crop", () => ({ - default: ({ onCropComplete }: any) => { +let lastCropperProps: any = null; + +// Fake cropper handed to onChange. aspectRatio={1} keeps the box square, so +// getCoordinates returns equal width/height. +const fakeCropper = { + getCoordinates: () => ({ left: 64, top: 48, width: 512, height: 512 }), + getState: () => ({ imageSize: { width: 640, height: 480 } }), +}; + +vi.mock("react-advanced-cropper", () => ({ + Cropper: function MockCropper(props: any) { + lastCropperProps = props; return (
- onCropComplete?.( - { x: 10, y: 10, width: 80, height: 80 }, - { x: 64, y: 48, width: 512, height: 512 }, - ) - } + onClick={() => props.onChange?.(fakeCropper)} /> ); }, + RectangleStencil: function MockRectangleStencil() { + return null; + }, + ImageRestriction: { + fillArea: "fillArea", + fitArea: "fitArea", + stencil: "stencil", + none: "none", + }, })); import GlazeImportCropStage from "../glazeImportTool/GlazeImportCropStage"; @@ -52,6 +66,10 @@ function makeRecord(overrides: Partial = {}): UploadedRecord { } describe("GlazeImportCropStage", () => { + beforeEach(() => { + lastCropperProps = null; + }); + function CropStageHarness() { const [records, setRecords] = useState([makeRecord()]); const [selectedRecordId, setSelectedRecordId] = useState( @@ -98,14 +116,17 @@ describe("GlazeImportCropStage", () => { ); expect(screen.getByText("Oribe")).toBeInTheDocument(); - expect( - screen.getByText( - "Create a crop to preview the transparency-safe square result.", - ), - ).toBeInTheDocument(); }); - it("handles the back button and updates crop geometry via onCropComplete", async () => { + // The import swatch is square by design (#146): the migration to + // react-advanced-cropper must preserve the 1:1 lock. + it("locks the crop stencil to a 1:1 aspect ratio", () => { + render(); + const stencilProps = lastCropperProps?.stencilProps ?? {}; + expect(stencilProps.aspectRatio).toBe(1); + }); + + it("handles the back button and updates crop geometry via onChange", async () => { render(); expect(screen.getByTestId("selected-id")).toHaveTextContent("record-1"); @@ -117,11 +138,10 @@ describe("GlazeImportCropStage", () => { await userEvent.click(screen.getByText("Oribe")); expect(screen.getByTestId("crop-probe")).toHaveTextContent("0,0,640"); - // Simulate crop complete via mock cropper click + // Simulate a crop change via the mock cropper. await userEvent.click(screen.getByTestId("mock-cropper")); - // After onCropComplete fires with { x:64, y:48, width:512, height:512 } - // crop.size = croppedAreaPixels.width = 512 + // coords {left:64, top:48, width:512} → crop.size = 512. expect(screen.getByTestId("crop-probe")).toHaveTextContent("64,48,512"); }); }); diff --git a/web/src/pages/glazeImportTool/GlazeImportCropStage.tsx b/web/src/pages/glazeImportTool/GlazeImportCropStage.tsx index 9ec50ef8..388d5a48 100644 --- a/web/src/pages/glazeImportTool/GlazeImportCropStage.tsx +++ b/web/src/pages/glazeImportTool/GlazeImportCropStage.tsx @@ -1,5 +1,5 @@ import { - useReducer, + useState, type Dispatch, type SetStateAction, } from "react"; @@ -12,11 +12,11 @@ import { Stack, Typography, } from "@mui/material"; -import Cropper from "react-easy-crop"; -import type { Area } from "react-easy-crop"; +import { Cropper, RectangleStencil, ImageRestriction } from "react-advanced-cropper"; +import type { CropperRef } from "react-advanced-cropper"; +import "react-advanced-cropper/dist/style.css"; import GlazeImportRecordList from "./GlazeImportRecordList"; import type { UploadedRecord } from "./glazeImportToolTypes"; -import type { CropSquare } from "../ocrDetection"; import { clampCrop, defaultCrop, @@ -35,40 +35,6 @@ interface GlazeImportCropStageProps { onContinueToOcr: () => void; } -type CropEditorState = { - crop: { x: number; y: number }; - zoom: number; - rotation: number; -}; - -type CropEditorAction = - | { type: "CROP_CHANGE"; crop: { x: number; y: number } } - | { type: "ZOOM_CHANGE"; zoom: number } - | { type: "ROTATION_CHANGE"; rotation: number } - | { type: "RECORD_SELECTED"; rotation: number }; - -function cropEditorReducer( - state: CropEditorState, - action: CropEditorAction, -): CropEditorState { - switch (action.type) { - case "CROP_CHANGE": - return { ...state, crop: action.crop }; - case "ZOOM_CHANGE": - return { ...state, zoom: action.zoom }; - case "ROTATION_CHANGE": - return { ...state, rotation: action.rotation }; - case "RECORD_SELECTED": - return { crop: { x: 0, y: 0 }, zoom: 1, rotation: action.rotation }; - } -} - -const INITIAL_CROP_EDITOR: CropEditorState = { - crop: { x: 0, y: 0 }, - zoom: 1, - rotation: 0, -}; - export default function GlazeImportCropStage({ records, selectedRecordId, @@ -80,10 +46,9 @@ export default function GlazeImportCropStage({ onDelete, onContinueToOcr, }: GlazeImportCropStageProps) { - const [cropEditor, dispatchCropEditor] = useReducer( - cropEditorReducer, - INITIAL_CROP_EDITOR, - ); + // Crop rotation is only ever seeded from the selected record; this stage has + // no rotation control. The cropper manages its own pan/zoom internally. + const [rotation, setRotation] = useState(0); const selectedRecord = records.find((record) => record.id === selectedRecordId) ?? null; @@ -91,20 +56,36 @@ export default function GlazeImportCropStage({ ? clampCrop(selectedRecord.dimensions, selectedRecord.crop) : null; - function handleCropComplete(_croppedArea: Area, croppedAreaPixels: Area) { + function handleCropChange(cropper: CropperRef) { if (!selectedRecord) return; - const crop: CropSquare = { - x: croppedAreaPixels.x, - y: croppedAreaPixels.y, - size: croppedAreaPixels.width, - rotation: cropEditor.rotation, - }; + const coords = cropper.getCoordinates(); + if (!coords) return; + // The import swatch is square by design (#146): aspectRatio={1} keeps + // width === height, so a single `size` captures the crop. + const next = clampCrop(selectedRecord.dimensions, { + x: coords.left, + y: coords.top, + size: coords.width, + rotation, + }); + // react-advanced-cropper's onChange fires continuously; skip the update + // (and the OCR-state reset below) when the clamped crop is unchanged. + const prev = selectedRecord.crop; + if ( + prev && + prev.x === next.x && + prev.y === next.y && + prev.size === next.size && + prev.rotation === next.rotation + ) { + return; + } setRecords((current) => current.map((record) => record.id === selectedRecord.id ? { ...record, - crop: clampCrop(record.dimensions, crop), + crop: next, cropped: true, ocrSuggestion: null, ocrStatus: "idle", @@ -118,7 +99,7 @@ export default function GlazeImportCropStage({ function handleResetCrop() { if (!selectedRecord) return; - dispatchCropEditor({ type: "RECORD_SELECTED", rotation: 0 }); + setRotation(0); setRecords((current) => current.map((record) => { if (record.id !== selectedRecord.id) return record; @@ -163,10 +144,7 @@ export default function GlazeImportCropStage({ selectedId={selectedRecordId} onSelect={(id) => { const record = records.find((r) => r.id === id); - dispatchCropEditor({ - type: "RECORD_SELECTED", - rotation: record?.crop?.rotation ?? 0, - }); + setRotation(record?.crop?.rotation ?? 0); setSelectedRecordId(id); }} onDelete={onDelete} @@ -222,31 +200,26 @@ export default function GlazeImportCropStage({ > - dispatchCropEditor({ type: "CROP_CHANGE", crop }) - } - onZoomChange={(zoom) => - dispatchCropEditor({ type: "ZOOM_CHANGE", zoom }) - } - onRotationChange={(rotation) => - dispatchCropEditor({ type: "ROTATION_CHANGE", rotation }) + defaultPosition={ + selectedCrop + ? { left: selectedCrop.x, top: selectedCrop.y } + : undefined } - onCropComplete={handleCropComplete} + onChange={handleCropChange} /> diff --git a/web/src/stories/CropOverlay.stories.tsx b/web/src/stories/CropOverlay.stories.tsx new file mode 100644 index 00000000..7938364c --- /dev/null +++ b/web/src/stories/CropOverlay.stories.tsx @@ -0,0 +1,50 @@ +import type { Meta, StoryObj } from "@storybook/react"; +import CropOverlay from "../components/CropOverlay"; + +/** + * CropOverlay is the image-lightbox crop editor. + * + * Rationale: + * - Free-form cropping (#737): the crop box resizes in any direction with no + * enforced aspect ratio (react-advanced-cropper RectangleStencil, no + * `aspectRatio`). + * - The crop is stored as fraction-based `ImageCrop` (independent width/height). + * + * These stories render the real react-advanced-cropper (not a mock), so they + * double as an integration check that the library is wired correctly — the + * unit tests mock the library wholesale. They load Cloudinary's public + * `demo/sample` asset. + */ +const meta = { + title: "Components/CropOverlay", + component: CropOverlay, + parameters: { + layout: "fullscreen", + }, +} satisfies Meta; + +export default meta; +type Story = StoryObj; + +const baseArgs = { + cloudinaryPublicId: "sample", + cloudName: "demo", + onSave: async () => {}, + onCancel: () => {}, +}; + +/** Free-form: drag any edge/corner to resize the crop box in any direction. */ +export const FreeForm: Story = { + args: { + ...baseArgs, + initialCrop: null, + }, +}; + +/** Opens seeded with an existing non-square crop (10%,10% → 50%×30%). */ +export const WithInitialCrop: Story = { + args: { + ...baseArgs, + initialCrop: { x: 0.1, y: 0.1, width: 0.5, height: 0.3 }, + }, +};