MobX crop store for Image Cropper

[rahmanunver]

Pull request type

Refactoring (e.g. file rename, variable rename, etc.)

---

Description

Stacked on top of the Image Cropper PR (feat/image-cropper-polish-rotate-bw) — base is set to that branch, so this diff shows only the MobX refactor, not the base PR's changes.

Why. Review feedback flagged that the widget's state had become too coupled to React. ImageCropperContainer carried five state-mirroring refs (committedCropRef, zoomRef, grayscaleRef, liveCropRef, zoomAnchorRef) plus two callback mirrors (markInternalRef, showPreviewRef). They existed only because applyCrop was a stable-identity useCallback (kept stable for the 400ms auto-apply debouncer) and therefore couldn't read current state through its stale closure. A MobX store method reads this.zoom directly, so the refs are no longer needed.

Moved into ImageCropperStore (MobX): the crop-domain working state — liveCrop, committedCrop, zoomAnchor (observable.ref), zoom, grayscale (observable) — plus aspect (computed) and the crop operations as actions (setZoom, commitCrop, toggleGrayscale, rotate, reset, initFromImageLoad, onImageChanged). The store also owns the debounce + auto-apply gate (absorbing useAutoApplyCrop). Crop ops are now plain calls like store.setZoom(v) / store.rotate(90).

Stayed normal React state (imperative / DOM-bound): the imageRef DOM node, usePreviewSrc (blob object-URL lifecycle), useOriginalImage (fetch + unmount cleanup + internal-change flag), and CropArea's local DOM-measurement state. These are injected into the store via setDeps. Config props are read live through the mobx-kit gate, not copied into observables.

Controlled/uncontrolled unchanged. The only externally controlled value is props.image (Mendix EditableValue); zoom/rotation/grayscale/crop are internal. Inbound image changes sync through one uri-keyed effect calling store.onImageChanged(); outbound writes go markInternalChange() → setValue(), with the internal-change flag absorbing the resulting uri change so there's no feedback loop. No reactions used for prop syncing.

Preview stays an async action, not a computed — cropImage/rotateImage are async canvas operations reading a live DOM <img>, so they can't be a pure synchronous computed.

Removed: the 5 mirror refs + 2 callback mirrors, and the useAutoApplyCrop / useImageCropperState hooks (subsumed by the store). ImageCropperContainer is now an observer wired via the shared widget-plugin-mobx-kit gate pattern (GateProvider + useConst + useSetup), matching custom-chart-web.

Public API unchanged — no XML/props changes; widget behavior is identical.

[github-actions]

AI Code Review

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

---

What was reviewed

File	Change
packages/pluggableWidgets/image-cropper-web/src/stores/ImageCropperStore.ts	New MobX store — crop-domain state, debounce gate, async ops, original capture, preview blob lifecycle
packages/pluggableWidgets/image-cropper-web/src/components/ImageCropperContainer.tsx	Gutted from 254 lines to 37 — now an observer wired via GateProvider + useConst + useSetup
packages/pluggableWidgets/image-cropper-web/src/components/CropArea.tsx	Import reorder only
packages/pluggableWidgets/image-cropper-web/src/stores/__tests__/ImageCropperStore.spec.ts	New store spec (462 lines) replacing the three deleted hook specs
packages/pluggableWidgets/image-cropper-web/src/hooks/useAutoApplyCrop.ts	Deleted — absorbed into store
packages/pluggableWidgets/image-cropper-web/src/hooks/useImageCropperState.ts	Deleted — absorbed into store
packages/pluggableWidgets/image-cropper-web/src/hooks/useOriginalImage.ts	Deleted — absorbed into store
packages/pluggableWidgets/image-cropper-web/src/hooks/usePreviewSrc.ts	Deleted — absorbed into store
packages/pluggableWidgets/image-cropper-web/src/hooks/__tests__/useImageCropperState.spec.ts	Deleted (hook deleted)
packages/pluggableWidgets/image-cropper-web/src/hooks/__tests__/useOriginalImage.spec.ts	Deleted (hook deleted)
packages/pluggableWidgets/image-cropper-web/src/hooks/__tests__/usePreviewSrc.spec.ts	Deleted (hook deleted)
packages/pluggableWidgets/image-cropper-web/package.json	Added mobx, mobx-react-lite, @mendix/widget-plugin-mobx-kit deps
packages/pluggableWidgets/image-cropper-web/openspec/specs/image-cropper/spec.md	Spec text update (no-image caption)
pnpm-lock.yaml	Lockfile update — out of scope

Skipped (out of scope): dist/, pnpm-lock.yaml (except to confirm dep resolution)

CI status could not be verified automatically — run gh pr checks 2321 manually before merging.

---

Findings

⚠️ Low — Store tests use manual mock for EditableValue instead of EditableValueBuilder

File: packages/pluggableWidgets/image-cropper-web/src/stores/__tests__/ImageCropperStore.spec.ts line 1262
Problem: makeImageProp hand-rolls the EditableValue<WebImage> shape with as unknown as ImageProp casting. The repo convention (documented in the skill and used across other widget specs) is to use EditableValueBuilder from @mendix/widget-plugin-test-utils. The manual mock covers the Available + readOnly guard cases but doesn't exercise the Loading or Unavailable branches of the store's guards in applyCrop, rotate, and reset — where all three guard image.status !== ValueStatus.Available.
Fix:

import { EditableValueBuilder } from "@mendix/widget-plugin-test-utils";

// Available with a value
const image = new EditableValueBuilder<WebImage>()
    .withValue({ uri: "http://localhost/img.png", name: "img.png" } as WebImage)
    .build();

// Read-only
const readOnlyImage = new EditableValueBuilder<WebImage>()
    .withValue({ uri: "http://localhost/img.png", name: "img.png" } as WebImage)
    .isReadOnly()
    .build();

// Loading (verify guards in applyCrop/rotate/reset skip the canvas op)
const loadingImage = new EditableValueBuilder<WebImage>().isLoading().build();

Adding a Loading-state guard test would make the coverage complete.

---

Positives

• Correct makeObservable discipline: explicit annotations, not makeAutoObservable; private apply-gate methods (armed, applyCrop, applyNow, applyDebounced, revokePreview) are correctly annotated false because they touch only plain (non-observable) fields and thus don't need action batching.
• Proper runInAction after every await: both rotate (wraps the liveCrop/committedCrop reset) and captureOriginal (wraps canRestore) correctly batch their post-await observable mutations. applyCrop has no post-await observable writes so no runInAction needed there — correctly omitted.
• Generation-token race guard in captureOriginal: the fetchGeneration increment correctly drops superseded in-flight fetches, mirroring the cancelled flag from the old useOriginalImage hook. The new test "drops a superseded in-flight fetch" verifies this path.
• Internal-change feedback-loop prevention preserved: markInternalChange() + internalChange flag in onUriChanged correctly absorbs own-baked URI changes, so setValue → uri-change → onUriChanged doesn't trigger a re-fetch or crop-clear. The "adopts our own bake's uri without refetching" test covers this.
• Comprehensive spec: 462 lines, describe/it structure with a makeStore factory, flush() helper for async, fake timers, and afterEach cleanup — matches repo test conventions. Coverage spans initial state, aspect computed, commitCrop gate, zoom debouncing, no-stale-reads proof, rotate, reset, inbound sync, guards, original capture, and preview blob lifecycle.
• Ref-free container: removing all five state-mirroring refs and two callback-mirror refs from the container is exactly the right outcome — the PR description nails why they existed and why the store makes them unnecessary.