[WC-3435]: Image Cropper Design and feats

[rahmanunver]

Pull request type

New feature (non-breaking change which adds functionality)

---

Description

[github-actions]

AI Code Review

⚠️ Approved with suggestions — low-severity items only, safe to merge

---

What was reviewed

File	Change
src/ImageCropper.editorConfig.ts	Refactored preview layout; moved describeConfig to shared util
src/ImageCropper.editorPreview.tsx	Full rewrite — real CropArea preview for static images, placeholder for dynamic
src/ImageCropper.xml	Added studioProCategory, studioCategory, and three new Editing properties
typings/ImageCropperProps.d.ts	Added enableRotation, enableGrayscale, showResetButton to both prop interfaces
typings/modules.d.ts	Added .png module declaration
src/components/CropArea.tsx	Added grayscale prop, safeImageUri guard, removed hard crossOrigin attribute
src/components/CropToolbar.tsx	New component — rotate left/right, B&W toggle, reset button
src/components/ImageCropperContainer.tsx	Wired up toolbar, useOriginalImage, usePreviewSrc, handleRotate/Reset/Grayscale
src/components/PreviewPane.tsx	Added grayscale prop + canvas filter; removed orphaned ctx.save/restore
src/hooks/useImageCropperState.ts	Added grayscale / setGrayscale state
src/hooks/useOriginalImage.ts	New hook — fetches and holds the original image bytes for Reset
src/hooks/usePreviewSrc.ts	New hook — blob URL preview bridging Mendix deferred-commit model
src/utils/cropImage.ts	Added grayscale option; inlined Math.max/round into destW/H
src/utils/cropMapping.ts	New util — normalizeRotation, rotatedCanvasSize
src/utils/describeConfig.ts	Extracted from editorConfig.ts; now shared
src/utils/rotateImage.ts	New util — canvas-based 90°-step image rotation
src/utils/safeImageUri.ts	New util — URI scheme allowlist guard
src/ui/ImageCropper.scss	Replaced __dropzone/__icon/__label with __toolbar/__tool/__reset/__preview-*
CHANGELOG.md	Updated entry under [Unreleased]
src/__tests__/ImageCropper.editor.spec.tsx	New — tests for getPreview and preview
src/__tests__/ImageCropperRotation.spec.tsx	New — rotation/grayscale integration tests
src/__tests__/ImageCropper.spec.tsx	Added reset tests; mocked global.fetch in beforeEach
Various __tests__/*.spec.{ts,tsx}	New unit tests for all new hooks, utils, and components

Skipped (out of scope): dist/, pnpm-lock.yaml, binary PNG/SVG assets

---

Findings

⚠️ Low — createRef in a function component body (editorPreview)

File: src/ImageCropper.editorPreview.tsx line 22
Note: createRef is called on every render, producing a new ref object each time. This is benign in the design preview (it's static/display-only), but it's a footgun that becomes a real bug the moment someone adds any behaviour. Use useRef instead — it's always the right choice in a function component.

// before
const imageRef = createRef<HTMLImageElement>();

// after
const imageRef = useRef<HTMLImageElement>(null);

---

⚠️ Low — ctx.save/restore removed from PreviewPane but canvas state is not reset between renders

File: src/components/PreviewPane.tsx lines 49–56
Note: The PR removes ctx.save/restore around the circle clip path (reasonable — the canvas is reset via ctx.clearRect on every render). However, the ctx.filter = "grayscale(1)" assignment at line 50 is never reset. clearRect clears pixel data but does not reset the rendering state. If grayscale transitions from true → false, the filter remains active because the context is reused across renders.

Fix: Reset the filter before each draw, or use ctx.save/restore to bracket the drawing block:

// Option A — explicit reset before conditional set
ctx.filter = grayscale ? "grayscale(1)" : "none";

// Option B — save/restore (also protects the clip path state)
ctx.save();
if (grayscale) ctx.filter = "grayscale(1)";
if (circle) { ctx.beginPath(); ctx.ellipse(...); ctx.clip(); }
ctx.drawImage(...);
ctx.restore();

---

⚠️ Low — usePreviewSrc mutates refs and calls setState during render

File: src/hooks/usePreviewSrc.ts lines 38–44
Note: The block that compares prevUri.current !== committedUri and calls revoke() + setPreviewSrc(undefined) runs synchronously during the render phase. Calling setState during render is only safe when it is guarded to run at most once per render (the pattern React uses internally for getDerivedStateFromProps-style state). The current code satisfies that constraint, but it also calls the side-effectful revoke() (which invokes URL.revokeObjectURL) during render — side effects during render are not safe in React's concurrent mode and can run multiple times. Move this logic into a useEffect keyed on committedUri:

useEffect(() => {
    if (blobRef.current) {
        revoke();
        setPreviewSrc(undefined);
    }
    prevUri.current = committedUri;
}, [committedUri, revoke]);

---

⚠️ Low — crossOrigin removed from CropArea <img> — may silently break toBlob on remote images

File: src/components/CropArea.tsx line 742 (diff context)
Note: crossOrigin="anonymous" was removed from the <img> element that feeds into the canvas operations (cropImage, rotateImage). Without this attribute, images loaded from a different origin taint the canvas, causing canvas.toBlob() to throw a SecurityError. The widget already renders an error if safeImageUri blocks a URI, but a cross-origin image with a valid http(s) URI will still load and taint silently. The toBlob path then rejects with CropError("canvas may be tainted"), which is recoverable — but the removal will regress real-world cross-origin Mendix CDN images that previously worked. Consider restoring crossOrigin="anonymous" with a note that the server must set Access-Control-Allow-Origin.

---

Positives

• safeImageUri URI scheme allowlist is a solid XSS defence-in-depth measure, correctly blocks javascript: and data:text/html while allowing data:image/ — and it has tests covering both pass and block cases.
• useOriginalImage correctly guards against stale state after unmount with the cancelled flag in the async closure.
• CropToolbar buttons all carry correct aria-label / aria-pressed attributes and use semantic <button type="button"> elements throughout — no <div onClick> patterns.
• rotateImage re-bakes the grayscale filter on every rotation so the staged file stays consistent regardless of operation order — this is the right call and the test for it proves the invariant.
• The markInternalChange / internalChange.current skip-on-next-uri pattern in useOriginalImage is a clean solution to the deferred-commit problem, and its test cases cover both the skip path and the preservation of the original file.
• Test coverage across the new hooks and utils is thorough — every public function has both the happy path and the failure path covered.