AI Code Review

🔶 Changes requested — one or more medium-severity items must be addressed

What was reviewed

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

Findings

🔶 Medium — 3 unit tests failing in CI

File: src/__tests__/ImageCropper.editor.spec.tsx lines 55, 60, 72
Problem: The CI "Run code quality check" job exits with code 1. Three tests in ImageCropper.editor.spec.tsx fail because the actual structure-preview output doesn't match what the spec expects.

Test 1 expects the text node "Image cropper" but the component returns "[Configure Image Cropper]".
Test 2 expects "[No attribute selected]" but receives "[Configure Image Cropper]".
Test 3 expects "Circle · 1:1 · JPEG · Viewport" but receives "[Circle · 1:1 · JPEG · Viewport] Image Cropper" — the whole string is wrapped with a widget-title prefix instead of being a standalone body text.

The mismatch is between editorConfig.ts's getPreview implementation (which wraps the caption inside "[…] Image Cropper") and the collectText helper in the spec that expects the caption and title to be separate Text nodes. Either the structure-preview tree layout doesn't match the assertions, or the caption strings changed during development and the tests were never updated.

Fix: Bring the spec into sync with the actual getPreview output. Based on what CI reports, the simplest fix is:

// test 1 — title is embedded in the body string, not a separate node
expect(texts.some(t => t.includes("Image Cropper"))).toBe(true);

// test 2 — placeholder is different
expect(texts).toContain("[Configure Image Cropper]");

// test 3 — full caption string, not just the config part
expect(texts).toContain("Circle · 1:1 · JPEG · Viewport");  // this one only needs the full string to be present as a sub-item — check whether collectText splits it

Alternatively, align the spec assertions to "[Configure Image Cropper]" and "[Circle · 1:1 · JPEG · Viewport] Image Cropper" if those are the intended strings. Do not merge until tests are green.

🔶 Medium — Lint warning (import order) in CI

File: src/components/CropArea.tsx line 6
Problem: CI lint step reports:

warning  `../utils/cropGuard` import should occur before import of `../utils/cropImage`  import/order

While this is a warning not an error, it contributes to the failed CI job and will accumulate technical debt. The auto-format hook does not fix import ordering.

Fix: Move the cropGuard import above the cropImage import:

import { isStrayCrop, MIN_CROP_PX } from "../utils/cropGuard";
import { CENTER_ANCHOR, type ZoomAnchor } from "../utils/cropImage";

⚠️ Low — editorPreview.tsx: createRef used inside a function component

File: src/ImageCropper.editorPreview.tsx line 22
Note: createRef is used inside StaticCropPreview instead of useRef. createRef creates a new ref object on every render — for a functional component useRef is the correct API. In the editor context this is unlikely to cause visible problems (the preview re-renders infrequently), but it's a latent misuse of the ref API.

// Replace
const imageRef = createRef<HTMLImageElement>();
// With
const imageRef = useRef<HTMLImageElement>(null);

⚠️ Low — usePreviewSrc: state mutation during render

File: src/hooks/usePreviewSrc.ts lines 38–44
Note: The prevUri comparison and the revoke()/setPreviewSrc(undefined) calls happen unconditionally during render (not inside a useEffect or useMemo). Calling setPreviewSrc during render is allowed by React as a synchronous bail-out, but mixing revoke() (a side-effect on a Ref) with the state update makes the pattern fragile and will trigger warnings in React Strict Mode's double-invoke behaviour. Consider wrapping this in a useEffect that reacts to committedUri changes instead.

Positives

• safeImageUri correctly allowlists only https?:, blob:, and data:image/ URIs with a tested unit spec — good defense-in-depth against XSS via Mendix image attributes.
• onCropAction.canExecute is properly checked before execute() — Mendix API contract honoured.
• props.image.status and props.image.readOnly are checked in both applyCrop and handleRotate before any setValue call — correct guard against read-only and loading states.
• The useAutoApplyCrop / userDraggedRef pattern cleanly separates layout-driven onCropComplete from genuine user drags, preventing unintended auto-commits in list views — the multi-instance test suite confirms this.
• useOriginalImage's async fetch sets a cancelled flag and returns a cleanup function — correct async-effect guard against state updates after unmount.
• computeSourceRect is shared between cropImage and PreviewPane, ensuring the preview and the exported file always frame the same pixels.
• Jest config correctly extends @mendix/pluggable-widgets-tools/test-config/jest.config, and setupFilesAfterEnv is properly extended rather than replaced.
• 115 tests across 18 suites, with real scenario coverage (grayscale reversibility, multi-instance isolation, reset, zoom debounce).