diff --git a/packages/pluggableWidgets/image-cropper-web/CHANGELOG.md b/packages/pluggableWidgets/image-cropper-web/CHANGELOG.md index 21a86e5502..9553e27e86 100644 --- a/packages/pluggableWidgets/image-cropper-web/CHANGELOG.md +++ b/packages/pluggableWidgets/image-cropper-web/CHANGELOG.md @@ -8,4 +8,15 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added +- Custom aspect ratio width and height can now be set with an expression or bound to an attribute. + +### Fixed + +- The crop selection no longer briefly appears at the wrong ratio while a custom aspect ratio expression is still loading. +- The crop selection is no longer cleared and the image no longer reloaded when unrelated properties on the page change. + +## [1.0.0] - 2026-07-16 + +### Added + - Initial release of Image cropper widget. diff --git a/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/.openspec.yaml b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/.openspec.yaml new file mode 100644 index 0000000000..5e6d53a3fd --- /dev/null +++ b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/.openspec.yaml @@ -0,0 +1,2 @@ +schema: spec-driven +created: 2026-07-24 diff --git a/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/design.md b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/design.md new file mode 100644 index 0000000000..9617332142 --- /dev/null +++ b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/design.md @@ -0,0 +1,66 @@ +## Context + +`customAspectWidth` / `customAspectHeight` moved from static `integer` props to `expression` props with `returnType Integer`, delivered by community PR #2333. Static integers were available synchronously on first render; expressions (especially attribute bindings) are `DynamicValue` and resolve **asynchronously** through `ValueStatus.Loading → Available`. + +The aspect is exposed as a single MobX `computed` in `ImageCropperStore`: + +```ts +get aspect(): number | undefined { + const toNumber = p => p.status === ValueStatus.Available && p.value ? p.value.toNumber() : undefined; + return resolveAspectRatio(this.props.aspectRatio, toNumber(width), toNumber(height)); +} +``` + +`aspect` flows to `CropArea`, which calls `buildInitialCrop(img, aspect)` on `` `onLoad` and passes `aspect` to `ReactCrop`. The `handleImageLoad` callback lists `aspect` in its dependency array, so the seeding callback changes identity when the ratio resolves. + +Current gap: while an expression is `Loading`, `toNumber` returns `undefined` → `resolveAspectRatio` returns `undefined` → **free aspect**. If the image loads during this window, the box seeds free; when the expression resolves the ratio flips and the box jumps. Worse, an auto-apply during that window could commit a wrongly-cropped image back to the bound attribute. + +## Goals / Non-Goals + +**Goals:** + +- Data-driven custom ratio via attribute/expression binding. +- No visible box "jump" and no committed wrong-ratio crop during the async load window. +- Deterministic re-seed when the ratio transitions unknown → known or value → value. +- Editor preview renders literal ratios and degrades gracefully for non-literals. + +**Non-Goals:** + +- Reworking the preset (non-custom) aspect modes — they remain synchronous enum values. +- Supporting fractional/decimal ratios beyond what `Integer` return type allows. +- Changing the crop/zoom/export pipeline beyond ratio seeding. + +## Decisions + +### Decision 1: Distinguish "loading" from "free" at the computed layer + +`aspect` currently collapses three distinct states (Loading, resolved-to-free, resolved-to-ratio) into `number | undefined`. `undefined` is overloaded to mean both "free aspect" and "not yet known", which is exactly why the box seeds wrong. + +**Chosen:** In "Custom" mode, treat "either side not Available" as a distinct `loading` signal, separate from a resolved free aspect. Consumers (initial-crop seeding, auto-apply) gate on readiness: do not seed/commit until the custom ratio is resolved. + +**Alternatives considered:** + +- _Default to free while loading_ (current behavior) — rejected: produces the jump the note warns about. +- _Cache the last integer value_ — rejected: no meaningful "last value" on first load, and stale values across record changes are their own bug. + +### Decision 2: Re-seed deterministically on ratio change, never commit intermediate frames + +When the resolved ratio changes, rebuild the crop box in one step (`buildInitialCrop`) rather than letting `ReactCrop` interpolate. Guard the auto-apply gate so a ratio change alone does not push a wrong-ratio image to the bound attribute — seeding is programmatic and must remain "disarmed" (the store already distinguishes user-driven commits from programmatic ones via `userDragged` / `armed()`). + +### Decision 3: Editor preview parses numeric literals only + +The editor has no runtime data — only expression _text_. `toNumber` parses a numeric literal and falls back to `undefined` (free aspect) otherwise. This is display-only and already implemented in the PR; the spec pins it so it isn't regressed. + +### Decision 4: Raise `minimumMXVersion` to 11.12 + +Expression-typed properties with `returnType Integer` bound to attributes are the supported baseline. Bump `marketplace.minimumMXVersion` from `10.21.0` to `11.12`. + +## Risks / Trade-offs + +- **Deferred seeding shows an unconstrained image briefly** while the expression loads → Mitigation: the image is already gated behind its own `ValueStatus.Available`; in practice the ratio usually resolves in the same or adjacent frame. Deferring the _crop box_ (not the image) is the least-surprising option. +- **Record change mid-session** flips the ratio to unavailable then to a new value → Mitigation: retain last valid box until the new ratio resolves (spec scenario), avoiding a free-aspect flash. +- **`minimumMXVersion` bump** drops support for older Studio Pro → accepted: expression binding requires it; documented in CHANGELOG. + +## Open Questions + +- **Loading-window box policy**: while the ratio is unavailable, should the widget (a) render the image with **no crop overlay** until the ratio resolves, or (b) render a **free-aspect box** and re-seed on resolve? (a) is cleanest (no jump) but shows a bare image for a frame; (b) is more familiar but risks a visible snap. This is the one behavior decision worth confirming before implementation — see tasks.md. diff --git a/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/proposal.md b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/proposal.md new file mode 100644 index 0000000000..d441a16ae4 --- /dev/null +++ b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/proposal.md @@ -0,0 +1,33 @@ +## Why + +App developers need the Image Cropper's custom aspect ratio to be data-driven — bound to an attribute or expression — instead of a fixed design-time integer. This lets one page enforce different crop ratios per record (e.g. a product-type-specific ratio) without duplicating widgets. The community PR (#2333) delivered the core capability; taking it over surfaced a correctness gap: because expressions resolve asynchronously, the widget must define behavior for the loading/unavailable window so the crop box does not seed at the wrong ratio and then jump when the value arrives. + +## What Changes + +- Change `customAspectWidth` / `customAspectHeight` from `integer` to `expression` with `returnType Integer`, so they accept an attribute binding or any Integer-returning expression. +- Read each side as `DynamicValue`, guarded by `ValueStatus.Available`, and convert to `number` for the aspect computation; treat unavailable/empty as "not yet known". +- `resolveAspectRatio` tolerates `undefined` sides (no ratio applied unless both are positive numbers). +- Editor preview parses the expression _text_ (numeric literals only) and falls back to free aspect when the value can't be evaluated at design time. +- Define and enforce **loading-state behavior**: while either expression is unavailable, the widget must not seed or commit a default/free-aspect crop that would visibly jump once the real ratio resolves. (Gap not fully addressed by #2333 — the core of this takeover.) +- Fix a pre-existing bug found while testing the above: the uri reaction compared a freshly built object literal with `Object.is`, so every re-render counted as an image change and re-fetched the original bytes + cleared the crop box. Now compared by value. +- Update `CHANGELOG.md` under `[Unreleased]` following Keep a Changelog format. +- Raise `marketplace.minimumMXVersion` to `11.12` (minimum Studio Pro version) since expression-typed custom-ratio binding is the supported baseline. + +## Capabilities + +### New Capabilities + + + +### Modified Capabilities + +- `image-cropper`: **Aspect ratio** gains the expression-bound custom sides and the "not yet resolved" state; **Default crop selection** gains the deferred-seed / re-seed / retain rules for that async window. Adds requirements for crop retention across unrelated re-renders and for design-time preview of the custom ratio. + +## Impact + +- **Widget config**: `ImageCropper.xml` (property types + returnType), generated `typings/ImageCropperProps.d.ts`. +- **Runtime**: `stores/ImageCropperStore.ts` (`aspect` computed), `utils/aspectRatio.ts` (undefined tolerance), `components/CropArea.tsx` (crop-seeding on aspect change), `utils/initialCrop.ts`. +- **Editor**: `ImageCropper.editorPreview.tsx` (literal parsing / free-aspect fallback). +- **Packaging**: `package.json` (`minimumMXVersion` → `11.12`), `CHANGELOG.md`. +- **Tests**: store, editor, rotation, grayscale, multi-instance specs already updated for the new prop shape; loading-state behavior needs coverage. +- **No new dependencies** (Big/DynamicValue already provided by the `mendix` package). diff --git a/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/specs/image-cropper/spec.md b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/specs/image-cropper/spec.md new file mode 100644 index 0000000000..5747ea5c70 --- /dev/null +++ b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/specs/image-cropper/spec.md @@ -0,0 +1,91 @@ +## MODIFIED Requirements + +### Requirement: Aspect ratio + +The widget SHALL constrain the crop selection to the ratio chosen in `aspectRatio`, supporting +free-form, preset ratios, and a custom ratio built from `customAspectWidth` / `customAspectHeight`, +which are Integer-returning expressions (attribute bindings or expressions) and therefore resolve +asynchronously. + +#### Scenario: Preset ratio locks proportions + +- **WHEN** `aspectRatio` is a preset (`square` = 1:1, `landscape16x9` = 16:9, `landscape4x3` = 4:3, `portrait3x4` = 3:4) +- **THEN** the crop selection SHALL keep that width-to-height proportion while being resized + +#### Scenario: Free ratio + +- **WHEN** `aspectRatio` is `free` +- **THEN** the crop selection SHALL be resizable to any proportion + +#### Scenario: Custom ratio + +- **WHEN** `aspectRatio` is `custom` and both `customAspectWidth` and `customAspectHeight` resolve to values greater than 0 +- **THEN** the crop selection SHALL be locked to `customAspectWidth / customAspectHeight` +- **AND** if either resolved value is not greater than 0, the crop SHALL fall back to free-form + +#### Scenario: Custom ratio expression not yet resolved + +- **WHEN** `aspectRatio` is `custom` and either side is not yet Available +- **THEN** the resolved ratio SHALL be treated as not yet known, which is distinct from free-form + +### Requirement: Default crop selection + +On image load, the widget SHALL seed a default crop box centered on the image at the resolved +aspect ratio. When the custom ratio is not yet resolved, seeding SHALL be deferred so the box is +never shown at a placeholder ratio that changes once the real ratio arrives. + +#### Scenario: Initial box covers 80% centered + +- **WHEN** an image finishes loading +- **THEN** the default selection SHALL cover 80% of the image, centered, at the resolved aspect ratio (falling back to the image's own ratio when free) + +#### Scenario: Image loads before the custom ratio resolves + +- **WHEN** an image finishes loading while `aspectRatio` is `custom` and either side is still unavailable +- **THEN** no crop selection SHALL be seeded until both sides are Available +- **AND** once both resolve, the selection SHALL be seeded once at the correct ratio + +#### Scenario: Resolved custom ratio changes to a new value + +- **WHEN** the resolved custom ratio changes from one positive value to another (e.g. a different record is shown) +- **THEN** the crop selection SHALL be rebuilt in a single step at the new ratio +- **AND** the change alone SHALL NOT write a re-cropped image back to the bound attribute + +#### Scenario: Resolved custom ratio becomes unavailable + +- **WHEN** the custom ratio was resolved and either side becomes unavailable +- **THEN** the existing crop selection SHALL be retained until a new ratio resolves + +## ADDED Requirements + +### Requirement: Image reload and crop retention across re-renders + +The widget SHALL treat the bound image as changed only when its uri or name actually changes, so +that unrelated re-renders neither re-download the original bytes nor discard the crop selection. + +#### Scenario: Unrelated re-render with an unchanged image + +- **WHEN** the widget re-renders with a new props object but the bound image's uri and name are unchanged +- **THEN** the original-bytes capture SHALL NOT be re-run +- **AND** the current crop selection SHALL be retained + +#### Scenario: Genuinely new image + +- **WHEN** the bound image's uri changes to a different external image +- **THEN** the original bytes SHALL be re-captured for Reset +- **AND** the crop selection SHALL be cleared + +### Requirement: Design-time preview of a custom aspect ratio + +In Studio Pro the widget SHALL render a representative crop from the custom ratio expression text, +which is all that is available at design time. + +#### Scenario: Numeric-literal expressions + +- **WHEN** both custom sides are numeric literals (e.g. "3" and "2") +- **THEN** the preview SHALL render the crop at that ratio + +#### Scenario: Attribute or non-literal expressions + +- **WHEN** either custom side is an attribute binding or an expression that cannot be evaluated at design time +- **THEN** the preview SHALL fall back to free aspect without erroring diff --git a/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/tasks.md b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/tasks.md new file mode 100644 index 0000000000..5f90def6d5 --- /dev/null +++ b/packages/pluggableWidgets/image-cropper-web/openspec/changes/image-cropper-aspect-expression/tasks.md @@ -0,0 +1,31 @@ +## 1. Verify inherited PR code + +- [x] 1.1 Build shared deps then the widget, run the existing unit suite (`pnpm run test`) to confirm the PR's prop-shape changes pass on the takeover branch +- [x] 1.2 Confirm no resource leaks introduced by the PR: blob-URL revoke, fetch-generation guard, and uri-reaction disposer are all still torn down in `setup()`'s cleanup (Big→number conversion adds no lifecycle-bound resource) + +## 2. Loading-state contract (chosen policy: defer box until ratio ready) + +- [x] 2.1 In `ImageCropperStore`, expose a readiness signal for the custom ratio: in "Custom" mode, `false` while either `customAspectWidth`/`customAspectHeight` is not `Available`; irrelevant (always ready) for preset modes +- [x] 2.2 Gate initial crop seeding on readiness — in "Custom" mode, do NOT seed the crop box on image `onLoad` until both sides are `Available`; seed once at the correct ratio when ready +- [x] 2.3 Ensure the auto-apply gate stays disarmed across a ratio-driven re-seed so no wrong-ratio image is committed back to the bound attribute (reuse `armed()` / programmatic-vs-user distinction) +- [x] 2.4 Re-seed deterministically when the resolved ratio changes value→value (e.g. record swap): rebuild via `buildInitialCrop` in one step, no intermediate `ReactCrop` interpolation frame +- [x] 2.5 Retain the last valid crop box when the ratio transitions Available→unavailable, until a new ratio resolves (no free-aspect flash) + +## 3. Tests + +- [x] 3.1 Store spec: ratio unavailable → no seed; ratio resolves → single seed at correct ratio +- [x] 3.2 Store spec: ratio value→value transition re-seeds once; no `setValue` (commit) fires from a ratio change alone +- [x] 3.3 Store spec: Available→unavailable retains the last box +- [x] 3.4 Editor preview spec: numeric-literal expressions render the ratio; non-literal/attribute falls back to free aspect without throwing +- [x] 3.5 Regression: zero/negative/empty side → free aspect, no throw (aspectRatio util) + +## 4. Packaging & docs + +- [x] 4.1 Update `CHANGELOG.md` under `[Unreleased]` (Keep a Changelog format) — keep the user-facing "Custom aspect ratio ... expression/attribute" entry, no implementation detail +- [x] 4.2 Raise `marketplace.minimumMXVersion` from `10.21.0` to `11.12` in `package.json` +- [x] 4.3 Do NOT bump the widget `version` — release automation handles version bumps at release time + +## 5. Verify + +- [x] 5.1 `pnpm run test` green in the package +- [ ] 5.2 Manual/E2E check in a Studio Pro test project: attribute-bound ratio, loading window shows no jump, record swap re-seeds cleanly diff --git a/packages/pluggableWidgets/image-cropper-web/package.json b/packages/pluggableWidgets/image-cropper-web/package.json index 6be4bfb431..4250652868 100644 --- a/packages/pluggableWidgets/image-cropper-web/package.json +++ b/packages/pluggableWidgets/image-cropper-web/package.json @@ -17,7 +17,7 @@ }, "packagePath": "com.mendix.widget.web", "marketplace": { - "minimumMXVersion": "10.21.0", + "minimumMXVersion": "11.12.0", "appName": "Image Cropper", "appNumber": 1, "reactReady": true diff --git a/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.editorPreview.tsx b/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.editorPreview.tsx index 5f3075bfd3..9f69b24eff 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.editorPreview.tsx +++ b/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.editorPreview.tsx @@ -21,10 +21,16 @@ function StaticCropPreview(props: { imageUrl: string; values: ImageCropperPrevie const [crop, setCrop] = useState(undefined); const imageRef = createRef(); + // Preview only has the expression *text* (no runtime data). Numeric literals render a real + // ratio; an attribute/expression path can't be evaluated here, so it falls back to free aspect. + const toNumber = (v: string | null): number | undefined => { + const n = Number(v); + return v != null && v !== "" && Number.isFinite(n) ? n : undefined; + }; const aspect = resolveAspectRatio( values.aspectRatio, - values.customAspectWidth ?? 0, - values.customAspectHeight ?? 0 + toNumber(values.customAspectWidth), + toNumber(values.customAspectHeight) ); const handleImageLoad = (percentCrop: Crop): void => { diff --git a/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.xml b/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.xml index c7268821d7..96491e96d0 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.xml +++ b/packages/pluggableWidgets/image-cropper-web/src/ImageCropper.xml @@ -34,13 +34,15 @@ Custom - + Custom aspect width - Width side of the ratio (e.g. 3 in 3:2). Used when Aspect ratio is Custom. + Width side of the ratio (e.g. 3 in 3:2). Used when Aspect ratio is Custom. Can be bound to an attribute or expression. + - + Custom aspect height - Height side of the ratio (e.g. 2 in 3:2). Used when Aspect ratio is Custom. + Height side of the ratio (e.g. 2 in 3:2). Used when Aspect ratio is Custom. Can be bound to an attribute or expression. + diff --git a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.editor.spec.tsx b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.editor.spec.tsx index 8121eaca81..2254399bac 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.editor.spec.tsx +++ b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.editor.spec.tsx @@ -1,4 +1,4 @@ -import { render } from "@testing-library/react"; +import { fireEvent, render } from "@testing-library/react"; import { ImageCropperPreviewProps } from "../../typings/ImageCropperProps"; import { getPreview } from "../ImageCropper.editorConfig"; import { preview } from "../ImageCropper.editorPreview"; @@ -15,8 +15,8 @@ function makePreviewProps(overrides: Partial = {}): Im image: null, cropShape: "rect", aspectRatio: "free", - customAspectWidth: null, - customAspectHeight: null, + customAspectWidth: "1", + customAspectHeight: "1", onCropAction: null, boundaryWidth: null, boundaryHeight: null, @@ -105,4 +105,55 @@ describe("ImageCropper design mode (preview)", () => { expect(container.querySelector(".widget-image-cropper__preview-glyph")).toBeNull(); expect(getByText("Rectangle · Free aspect · PNG · Original")).toBeInTheDocument(); }); + + // The editor only has the expression *text*, never runtime data. A numeric literal can be + // parsed into a real ratio; an attribute path cannot, so it degrades to free aspect. + describe("custom aspect ratio from expression text", () => { + function renderStaticPreview(overrides: Partial): ReturnType { + const props = makePreviewProps({ + image: { type: "static", imageUrl: "http://localhost/photo.png" }, + aspectRatio: "custom", + boundaryWidth: 400, + boundaryHeight: 300, + ...overrides + }); + const utils = render(preview(props)); + const img = utils.container.querySelector("img") as HTMLImageElement; + Object.defineProperty(img, "naturalWidth", { value: 400, configurable: true }); + Object.defineProperty(img, "naturalHeight", { value: 300, configurable: true }); + Object.defineProperty(img, "width", { value: 400, configurable: true }); + Object.defineProperty(img, "height", { value: 300, configurable: true }); + fireEvent.load(img); + return utils; + } + + // react-image-crop renders the selection box with percentage width/height, so the drawn + // box's aspect (scaled by the image's own aspect) reveals which ratio was applied. + function selectionAspect(container: HTMLElement): number { + const selection = container.querySelector(".ReactCrop__crop-selection") as HTMLElement; + expect(selection).not.toBeNull(); + const width = parseFloat(selection.style.width); + const height = parseFloat(selection.style.height); + return width / height; + } + + test("renders the custom ratio when both sides are numeric literals", () => { + const { container } = renderStaticPreview({ customAspectWidth: "3", customAspectHeight: "2" }); + // 3:2 box over a 400x300 image, expressed in % of each axis. + expect(selectionAspect(container)).toBeCloseTo((3 / 2) * (300 / 400), 3); + }); + + test("falls back to free aspect for an attribute/expression path", () => { + const { container } = renderStaticPreview({ + customAspectWidth: "$currentObject/Width", + customAspectHeight: "2" + }); + // Free aspect seeds an 80% box matching the image's own ratio -> square in % terms. + expect(selectionAspect(container)).toBeCloseTo(1, 3); + }); + + test("does not throw when both sides are empty", () => { + expect(() => renderStaticPreview({ customAspectWidth: "", customAspectHeight: "" })).not.toThrow(); + }); + }); }); diff --git a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.spec.tsx b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.spec.tsx index 6bf764e3a8..381897fed0 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.spec.tsx +++ b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropper.spec.tsx @@ -87,8 +87,8 @@ function makeProps(overrides: Partial = {}): ImageCr image: makeImageProp(), cropShape: "rect", aspectRatio: "free", - customAspectWidth: 1, - customAspectHeight: 1, + customAspectWidth: dynamic.available(new Big(1)), + customAspectHeight: dynamic.available(new Big(1)), boundaryWidth: 300, boundaryHeight: 300, resizableEnabled: true, diff --git a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperGrayscale.spec.tsx b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperGrayscale.spec.tsx index e43fe42904..a80237e6be 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperGrayscale.spec.tsx +++ b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperGrayscale.spec.tsx @@ -3,7 +3,7 @@ import { Big } from "big.js"; import { ValueStatus } from "mendix"; import { Ref } from "react"; import type { Crop, PixelCrop } from "react-image-crop"; -import { actionValue } from "@mendix/widget-plugin-test-utils"; +import { actionValue, dynamic } from "@mendix/widget-plugin-test-utils"; import type { ImageCropperContainerProps } from "../../typings/ImageCropperProps"; // Integration test: proves grayscale reversibility after rotate. @@ -96,8 +96,8 @@ function makeProps(overrides: Partial = {}): ImageCr image: makeImageProp(), cropShape: "rect", aspectRatio: "free", - customAspectWidth: 1, - customAspectHeight: 1, + customAspectWidth: dynamic.available(new Big(1)), + customAspectHeight: dynamic.available(new Big(1)), boundaryWidth: 300, boundaryHeight: 300, resizableEnabled: true, diff --git a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperMultiInstance.spec.tsx b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperMultiInstance.spec.tsx index 49af30088f..f42d4b068a 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperMultiInstance.spec.tsx +++ b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperMultiInstance.spec.tsx @@ -3,7 +3,7 @@ import { Big } from "big.js"; import { ValueStatus } from "mendix"; import { Ref } from "react"; import type { Crop, PixelCrop } from "react-image-crop"; -import { actionValue } from "@mendix/widget-plugin-test-utils"; +import { actionValue, dynamic } from "@mendix/widget-plugin-test-utils"; import type { ImageCropperContainerProps } from "../../typings/ImageCropperProps"; // Multi-instance integration test: verifies that auto-commit only fires for the @@ -88,8 +88,8 @@ function makeProps(overrides: Partial = {}): ImageCr image: makeImageProp(), cropShape: "rect", aspectRatio: "free", - customAspectWidth: 1, - customAspectHeight: 1, + customAspectWidth: dynamic.available(new Big(1)), + customAspectHeight: dynamic.available(new Big(1)), boundaryWidth: 300, boundaryHeight: 300, resizableEnabled: true, diff --git a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperRotation.spec.tsx b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperRotation.spec.tsx index b3806b507e..9094eeb222 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperRotation.spec.tsx +++ b/packages/pluggableWidgets/image-cropper-web/src/__tests__/ImageCropperRotation.spec.tsx @@ -3,7 +3,7 @@ import { Big } from "big.js"; import { ValueStatus } from "mendix"; import { Ref } from "react"; import type { Crop, PixelCrop } from "react-image-crop"; -import { actionValue } from "@mendix/widget-plugin-test-utils"; +import { actionValue, dynamic } from "@mendix/widget-plugin-test-utils"; import type { ImageCropperContainerProps } from "../../typings/ImageCropperProps"; // Integration test: proves the rotate/grayscale actions reach the right util with the right args. @@ -105,8 +105,8 @@ function makeProps(overrides: Partial = {}): ImageCr image: makeImageProp(), cropShape: "rect", aspectRatio: "free", - customAspectWidth: 1, - customAspectHeight: 1, + customAspectWidth: dynamic.available(new Big(1)), + customAspectHeight: dynamic.available(new Big(1)), boundaryWidth: 300, boundaryHeight: 300, resizableEnabled: true, diff --git a/packages/pluggableWidgets/image-cropper-web/src/stores/ImageCropperStore.ts b/packages/pluggableWidgets/image-cropper-web/src/stores/ImageCropperStore.ts index 9b2e986c17..41af616304 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/stores/ImageCropperStore.ts +++ b/packages/pluggableWidgets/image-cropper-web/src/stores/ImageCropperStore.ts @@ -1,4 +1,5 @@ -import { ValueStatus } from "mendix"; +import { Big } from "big.js"; +import { DynamicValue, ValueStatus } from "mendix"; import { action, computed, makeObservable, observable, reaction, runInAction } from "mobx"; import { type SetStateAction } from "react"; import { type Crop, type PixelCrop } from "react-image-crop"; @@ -76,6 +77,8 @@ export class ImageCropperStore implements SetupComponent { private fetchGeneration = 0; // Disposer for the uri reaction, torn down alongside the debounce in setup(). private disposeUriEffect: (() => void) | undefined = undefined; + // Disposer for the custom-ratio reaction (seeds/re-seeds the box once the ratio resolves). + private disposeAspectEffect: (() => void) | undefined = undefined; constructor(gate: DerivedPropsGate) { this.gate = gate; @@ -85,7 +88,14 @@ export class ImageCropperStore implements SetupComponent { // values use observable.ref (we always replace, never mutate in place). makeObservable< this, - "props" | "applyCrop" | "applyNow" | "applyDebounced" | "armed" | "onUriChanged" | "revokePreview" + | "props" + | "applyCrop" + | "applyNow" + | "applyDebounced" + | "armed" + | "onUriChanged" + | "revokePreview" + | "onAspectResolved" >(this, { liveCrop: observable.ref, committedCrop: observable.ref, @@ -96,6 +106,8 @@ export class ImageCropperStore implements SetupComponent { canRestore: observable, props: computed, aspect: computed, + aspectReady: computed, + onAspectResolved: action, setLiveCrop: action, markUserDragged: action, commitCrop: action, @@ -124,7 +136,29 @@ export class ImageCropperStore implements SetupComponent { } get aspect(): number | undefined { - return resolveAspectRatio(this.props.aspectRatio, this.props.customAspectWidth, this.props.customAspectHeight); + const toNumber = (p: DynamicValue): number | undefined => + p.status === ValueStatus.Available && p.value ? p.value.toNumber() : undefined; + return resolveAspectRatio( + this.props.aspectRatio, + toNumber(this.props.customAspectWidth), + toNumber(this.props.customAspectHeight) + ); + } + + /** + * Whether the aspect ratio is known well enough to seed a crop box. + * + * `aspect` collapses three states into `number | undefined`: a resolved ratio, a resolved + * "free" aspect, and "custom ratio not loaded yet". The last two both read as `undefined`, + * which is why an unguarded seed picks free aspect and then visibly jumps once the + * expression resolves. Only custom mode can be pending; every preset is synchronous. + */ + get aspectReady(): boolean { + if (this.props.aspectRatio !== "custom") { + return true; + } + const isAvailable = (p: DynamicValue): boolean => p.status === ValueStatus.Available; + return isAvailable(this.props.customAspectWidth) && isAvailable(this.props.customAspectHeight); } setup(): () => void { @@ -148,13 +182,32 @@ export class ImageCropperStore implements SetupComponent { return { uri: value?.uri, name: value?.name }; }, ({ uri, name }) => this.onUriChanged(uri, name), - { fireImmediately: true } + // The data fn builds a fresh object literal every time props are swapped, and the + // gate deliberately uses observable.ref (not struct), so default Object.is equality + // would treat every render as a uri change and refetch/clear the crop. Compare by + // value instead. + { equals: (a, b) => a.uri === b.uri && a.name === b.name, fireImmediately: true } ); - // Combined teardown: stop the debounce, dispose the uri reaction, revoke any live blob. + // React to the resolved aspect ratio settling. The data fn emits `undefined` while the + // custom ratio is pending, so a value -> pending transition fires nothing (the last box + // is retained); only a genuine ratio value re-seeds. fireImmediately is deliberately OFF + // — the initial box comes from CropArea's onLoad. + this.disposeAspectEffect = reaction( + () => (this.aspectReady ? this.aspect : undefined), + aspect => { + if (aspect === undefined && !this.aspectReady) { + return; // ratio went pending — keep the current box + } + this.onAspectResolved(); + } + ); + + // Combined teardown: stop the debounce, dispose both reactions, revoke any live blob. return () => { abort(); this.disposeUriEffect?.(); + this.disposeAspectEffect?.(); this.revokePreview(); }; } @@ -364,9 +417,40 @@ export class ImageCropperStore implements SetupComponent { initFromImageLoad(percentCrop: Crop, pixelCrop: PixelCrop): void { this.zoom = Number(this.props.minZoom); this.zoomAnchor = CENTER_ANCHOR; + this.armed(); // programmatic load must not auto-commit + + // The image beat the custom-ratio expression. Seeding now would use free aspect (see + // aspectReady) and snap when the real ratio lands, so leave the box unseeded and let + // the aspect reaction seed it once — at the correct ratio. + if (!this.aspectReady) { + this.liveCrop = undefined; + this.committedCrop = undefined; + return; + } + this.liveCrop = percentCrop; this.committedCrop = pixelCrop; - this.armed(); // programmatic load must not auto-commit + } + + /** + * Runs when the resolved aspect ratio settles on a new value (fired by the setup() reaction, + * which only tracks ready states — a transition into "pending" never reaches here, so the + * last valid box survives an Available -> unavailable flip until a new ratio arrives). + * + * Rebuilds the box in one step via buildInitialCrop rather than letting ReactCrop reconcile + * the old box against the new ratio, and stays disarmed so a ratio change alone never + * commits a re-cropped image back to the bound attribute. + */ + private onAspectResolved(): void { + const img = this.deps.getImage(); + if (!img || !img.naturalWidth) { + // No on-screen image yet; CropArea's onLoad will seed with the now-ready ratio. + return; + } + const { percentCrop, pixelCrop } = buildInitialCrop(img, this.aspect); + this.liveCrop = percentCrop; + this.committedCrop = pixelCrop; + this.armed(); // programmatic re-seed must not auto-commit } // Inbound sync: clear the crop box and disarm the apply gate when the bound image changes. diff --git a/packages/pluggableWidgets/image-cropper-web/src/stores/__tests__/ImageCropperStore.spec.ts b/packages/pluggableWidgets/image-cropper-web/src/stores/__tests__/ImageCropperStore.spec.ts index f8df72000c..4dabee8bca 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/stores/__tests__/ImageCropperStore.spec.ts +++ b/packages/pluggableWidgets/image-cropper-web/src/stores/__tests__/ImageCropperStore.spec.ts @@ -3,6 +3,7 @@ import { ValueStatus } from "mendix"; import { action, makeObservable, observable } from "mobx"; import type { Crop, PixelCrop } from "react-image-crop"; import { DerivedPropsGate } from "@mendix/widget-plugin-mobx-kit/main"; +import { dynamic } from "@mendix/widget-plugin-test-utils"; import type { ImageCropperContainerProps } from "../../../typings/ImageCropperProps"; // The store calls cropImage/rotateImage (async canvas work). Mock them so the spec asserts @@ -52,8 +53,8 @@ function makeProps(overrides: Partial = {}): ImageCr image: makeImageProp(), cropShape: "rect", aspectRatio: "square", - customAspectWidth: 1, - customAspectHeight: 1, + customAspectWidth: dynamic.available(new Big(1)), + customAspectHeight: dynamic.available(new Big(1)), boundaryWidth: 300, boundaryHeight: 300, resizableEnabled: true, @@ -175,6 +176,120 @@ describe("ImageCropperStore", () => { expect(store.aspect).toBeCloseTo(16 / 9); dispose(); }); + + it("derives a custom ratio from the width/height expression values", () => { + const { store, gate, dispose } = makeStore({ + aspectRatio: "custom", + customAspectWidth: dynamic.available(new Big(16)), + customAspectHeight: dynamic.available(new Big(9)) + }); + expect(store.aspect).toBeCloseTo(16 / 9); + gate.setProps( + makeProps({ + aspectRatio: "custom", + customAspectWidth: dynamic.available(new Big(3)), + customAspectHeight: dynamic.available(new Big(2)) + }) + ); + expect(store.aspect).toBeCloseTo(3 / 2); + dispose(); + }); + + it("falls back to free aspect when a custom expression value is unavailable", () => { + const { store, dispose } = makeStore({ + aspectRatio: "custom", + customAspectWidth: dynamic.unavailable(), + customAspectHeight: dynamic.available(new Big(9)) + }); + expect(store.aspect).toBeUndefined(); + dispose(); + }); + }); + + // The custom ratio arrives asynchronously, so `aspect === undefined` is ambiguous: it means + // both "free aspect" and "not loaded yet". aspectReady disambiguates, and the seeding path + // must wait on it — otherwise the box seeds free and visibly jumps once the ratio lands. + describe("custom ratio loading window", () => { + const pendingCustom = { + aspectRatio: "custom" as const, + customAspectWidth: dynamic.loading(), + customAspectHeight: dynamic.available(new Big(2)) + }; + const resolvedCustom = (w: number, h: number): Partial => ({ + aspectRatio: "custom" as const, + customAspectWidth: dynamic.available(new Big(w)), + customAspectHeight: dynamic.available(new Big(h)) + }); + + it("reports aspectReady=false only while a custom side is not Available", () => { + const { store: pending, dispose: d1 } = makeStore(pendingCustom); + expect(pending.aspectReady).toBe(false); + d1(); + + const { store: ready, dispose: d2 } = makeStore(resolvedCustom(3, 2)); + expect(ready.aspectReady).toBe(true); + d2(); + + // Presets resolve synchronously — never pending, even with unavailable custom sides. + const { store: preset, dispose: d3 } = makeStore({ + aspectRatio: "square", + customAspectWidth: dynamic.loading() + }); + expect(preset.aspectReady).toBe(true); + d3(); + }); + + it("does not seed the crop box while the ratio is pending, then seeds once it resolves", () => { + const { store, gate, dispose } = makeStore(pendingCustom); + + // The image finished loading before the expression did. + store.initFromImageLoad(PERCENT_CROP, PIXEL_CROP); + expect(store.liveCrop).toBeUndefined(); + expect(store.committedCrop).toBeUndefined(); + + // Ratio lands: a single seed, at the resolved ratio (400x300 fake image, 3:2). + gate.setProps(makeProps(resolvedCustom(3, 2))); + expect(store.liveCrop).toBeDefined(); + const seeded = store.liveCrop!; + expect(seeded.width / seeded.height).toBeCloseTo((3 / 2) * (300 / 400), 5); + dispose(); + }); + + it("re-seeds on a value-to-value ratio change without committing anything", async () => { + const { store, gate, dispose } = makeStore(resolvedCustom(3, 2)); + store.initFromImageLoad(PERCENT_CROP, PIXEL_CROP); + (gate.props.image.setValue as jest.Mock).mockClear(); + (cropImage as jest.Mock).mockClear(); + + // e.g. a record swap hands over a different ratio. + gate.setProps(makeProps(resolvedCustom(1, 1))); + const reseeded = store.liveCrop!; + expect(reseeded.width / reseeded.height).toBeCloseTo(1 * (300 / 400), 5); + + // A ratio change is programmatic — it must never push a re-cropped image back. + await flush(); + expect(cropImage).not.toHaveBeenCalled(); + expect(gate.props.image.setValue).not.toHaveBeenCalled(); + dispose(); + }); + + it("retains the last valid box when the ratio goes from Available to unavailable", () => { + const { store, gate, dispose } = makeStore(resolvedCustom(3, 2)); + store.initFromImageLoad(PERCENT_CROP, PIXEL_CROP); + const before = store.liveCrop; + expect(before).toBeDefined(); + + // Record changes: the expression briefly stops resolving. No free-aspect flash. + gate.setProps( + makeProps({ + aspectRatio: "custom", + customAspectWidth: dynamic.loading(), + customAspectHeight: dynamic.loading() + }) + ); + expect(store.liveCrop).toBe(before); + dispose(); + }); }); describe("commitCrop gate (user-drag vs programmatic)", () => { @@ -378,6 +493,24 @@ describe("ImageCropperStore", () => { dispose(); }); + it("ignores a re-render that carries the same uri in a new props object", async () => { + const { store, gate, dispose } = makeStore(); + await flush(); + (global.fetch as jest.Mock).mockClear(); + store.setLiveCrop(PERCENT_CROP); + store.commitCrop(PIXEL_CROP); + + // React hands over a fresh props object on every parent re-render, and the gate uses + // observable.ref by design — so identity alone must not count as an image change. + gate.setProps(makeProps()); + await flush(); + + expect(global.fetch).not.toHaveBeenCalled(); + expect(store.liveCrop).toEqual(PERCENT_CROP); + expect(store.committedCrop).toEqual(PIXEL_CROP); + dispose(); + }); + it("adopts our own bake's uri without refetching or clearing the crop", async () => { const { store, gate, dispose } = makeStore(); store.markUserDragged(); diff --git a/packages/pluggableWidgets/image-cropper-web/src/utils/__tests__/aspectRatio.spec.ts b/packages/pluggableWidgets/image-cropper-web/src/utils/__tests__/aspectRatio.spec.ts index 71f5966172..8fab8be866 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/utils/__tests__/aspectRatio.spec.ts +++ b/packages/pluggableWidgets/image-cropper-web/src/utils/__tests__/aspectRatio.spec.ts @@ -36,4 +36,18 @@ describe("resolveAspectRatio", () => { test("returns undefined when custom width is negative", () => { expect(resolveAspectRatio("custom", -1, 9)).toBeUndefined(); }); + + test("returns undefined when custom height is negative", () => { + expect(resolveAspectRatio("custom", 16, -9)).toBeUndefined(); + }); + + // Sides come from expressions now, so either can be undefined while it is unavailable or + // empty. That must degrade to free aspect, not NaN or a throw. + test.each([ + ["both sides undefined", undefined, undefined], + ["width undefined", undefined, 9], + ["height undefined", 16, undefined] + ])("returns undefined for custom when %s", (_label, width, height) => { + expect(resolveAspectRatio("custom", width, height)).toBeUndefined(); + }); }); diff --git a/packages/pluggableWidgets/image-cropper-web/src/utils/aspectRatio.ts b/packages/pluggableWidgets/image-cropper-web/src/utils/aspectRatio.ts index e5d305dce4..905a330352 100644 --- a/packages/pluggableWidgets/image-cropper-web/src/utils/aspectRatio.ts +++ b/packages/pluggableWidgets/image-cropper-web/src/utils/aspectRatio.ts @@ -2,8 +2,8 @@ import { AspectRatioEnum } from "../../typings/ImageCropperProps"; export function resolveAspectRatio( aspect: AspectRatioEnum, - customWidth: number, - customHeight: number + customWidth: number | undefined, + customHeight: number | undefined ): number | undefined { switch (aspect) { case "free": @@ -17,7 +17,7 @@ export function resolveAspectRatio( case "portrait3x4": return 3 / 4; case "custom": - if (customWidth > 0 && customHeight > 0) { + if (customWidth != null && customHeight != null && customWidth > 0 && customHeight > 0) { return customWidth / customHeight; } return undefined; diff --git a/packages/pluggableWidgets/image-cropper-web/typings/ImageCropperProps.d.ts b/packages/pluggableWidgets/image-cropper-web/typings/ImageCropperProps.d.ts index 515e799308..c05ef67416 100644 --- a/packages/pluggableWidgets/image-cropper-web/typings/ImageCropperProps.d.ts +++ b/packages/pluggableWidgets/image-cropper-web/typings/ImageCropperProps.d.ts @@ -25,8 +25,8 @@ export interface ImageCropperContainerProps { image: EditableImageValue; cropShape: CropShapeEnum; aspectRatio: AspectRatioEnum; - customAspectWidth: number; - customAspectHeight: number; + customAspectWidth: DynamicValue; + customAspectHeight: DynamicValue; onCropAction?: ActionValue; boundaryWidth: number; boundaryHeight: number; @@ -67,8 +67,8 @@ export interface ImageCropperPreviewProps { image: { type: "static"; imageUrl: string; } | { type: "dynamic"; entity: string; } | null; cropShape: CropShapeEnum; aspectRatio: AspectRatioEnum; - customAspectWidth: number | null; - customAspectHeight: number | null; + customAspectWidth: string; + customAspectHeight: string; onCropAction: {} | null; boundaryWidth: number | null; boundaryHeight: number | null;