feat(image-cropper-web): allow custom aspect ratio width/height as expressions #2333
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| schema: spec-driven | ||
| created: 2026-07-24 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<Big>` 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 `<img>` `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`. | ||
|
Collaborator
There was a problem hiding this comment. It is already 11.12. If it is not set correctly it should be corrected. This is not part of the story. |
||
|
|
||
| ## 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. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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<Big>`, 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 | ||
|
|
||
| <!-- None — this widget already has a captured spec at openspec/specs/image-cropper/. --> | ||
|
|
||
| ### 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). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 | ||
|
Comment on lines
+26
to
+29
Collaborator
There was a problem hiding this comment. I miss explicit scenario definition when properties never resolve to a real value, for example they are |
||
|
|
||
| ### 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 | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version header should not be added