Skip to content

[WC-3435]: Image Cropper Design and feats#2280

Open
rahmanunver wants to merge 25 commits into
mainfrom
feat/image-cropper-polish-rotate-bw
Open

[WC-3435]: Image Cropper Design and feats#2280
rahmanunver wants to merge 25 commits into
mainfrom
feat/image-cropper-polish-rotate-bw

Conversation

@rahmanunver

@rahmanunver rahmanunver commented Jun 19, 2026

Copy link
Copy Markdown
Contributor

Pull request type

New feature (non-breaking change which adds functionality)


Description

Initial development of the Image Cropper widget. This is a pre-release widget; the items below describe its current capabilities, not changes to a shipped version.

Cropping

  • Interactive crop with configurable shape (rectangle / circle) and aspect ratio (free or custom)
  • Zoom via inline slider and optional mouse-wheel zoom
  • Output configuration: format, quality, and size (original or viewport)

Toolbar

  • Single-row toolbar: flip-left, flip-right, grayscale, zoom slider, reset
  • Flip controls (90° rotation under the hood) rendered as SVG icon buttons
  • Grayscale toggle
  • Reset to the original image
  • Always-on native tooltips on every toolbar button

Editor experience (Studio Pro)

  • Structure-mode and design-mode previews
  • Design-mode preview renders the real crop area against the configured image
  • Studio Pro toolbox category and icons

Layout

  • Widget sizes to its image and renders block-level, so sibling widgets stack below it
  • Editor preview fills the available width

Behavior

  • Grayscale stays reversible after a flip; grayscale is baked into the saved file only when the toggle is on
  • Crop auto-commit is gated on a genuine user drag, so editing one cropper instance does not commit others in a list
  • Crop alignment preserved across flips via a live blob preview
  • Image source guarded through a safeImageUri allowlist

Testing

  • Unit/integration specs for flip, grayscale, zoom, crop commit, grayscale reversibility, multi-instance commit isolation, and editor previews (103 tests)

What should be covered while testing?

  • Crop, zoom, flip, grayscale, and reset on a bound image attribute
  • Grayscale ON -> flip -> grayscale OFF restores color; grayscale ON -> flip -> save with no further crop persists grayscale
  • Two croppers in a list: editing one and saving commits only that instance

@rahmanunver rahmanunver requested a review from a team as a code owner June 19, 2026 07:49
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@rahmanunver rahmanunver force-pushed the feat/image-cropper-polish-rotate-bw branch from 60f8a9b to 2741159 Compare June 25, 2026 08:15
rahmanunver and others added 15 commits June 29, 2026 15:29
… fix crop alignment

Replace CSS-transform rotation with a pixel re-bake so the crop selection
maps to the visible image, and add a blob-URL live preview so the rotation
is visible before the deferred Mendix commit (Save). Reset now restores the
true first-load original via an internal-change gate in useOriginalImage.
…esign mode

Simplify structure mode to a title row plus a single status/config row
([No attribute selected] vs config summary), dropping the icon and large
image render. Rewrite design mode into a three-state preview driven by the
bound image: static images render the real CropArea (non-interactive) with a
config caption, while dynamic/unbound images show a placeholder glyph with
[No image selected yet].

Extract shared describeConfig/aspectLabel into utils, add the cropper
placeholder asset, declare the png module for TypeScript, and cover both
editor surfaces with new specs.
…t fixes

Terminology:
- rename user-facing "Rotate" to "Flip" (toolbar, tooltips, enableFlip property)
- rename "Black and white"/"B&W" toggle to "Grayscale"

Toolbar:
- single-row layout: flip-left, flip-right, grayscale, zoom slider, reset
- flip buttons use SVG icons; square, icon-only styling
- always-on native tooltips on every toolbar button

Layout:
- size widget to its image and render block-level so sibling widgets stack below
- editor design-mode preview fills available width
- remove redundant "Image cropper" label above the design-mode preview

Behavior:
- keep a color working image on flip so grayscale stays reversible; bake
  grayscale only into the committed file when the toggle is on
- gate crop auto-commit on a genuine user drag so editing one cropper does not
  commit sibling instances in a list

Tests: rename specs for flip/grayscale; add grayscale-reversibility and
multi-instance commit regression specs.
@r0b1n r0b1n force-pushed the feat/image-cropper-polish-rotate-bw branch from 2741159 to 1472687 Compare June 29, 2026 15:05
@github-actions

This comment has been minimized.

Remove keepSelection and add MIN_CROP_PX min width/height so dragging on
empty canvas starts a fresh selection instead of nudging the old one.

A bare click (mousedown+mouseup, no drag) yields a ~0-size crop from
react-image-crop that minWidth/minHeight does not catch, which would wipe
the box. Guard it with isStrayCrop: crops below MIN_DRAW_FRACTION of the
displayed image are ignored so the existing selection survives.
Reset previously cleared the selection, leaving no box. Restore the
original bytes and re-seed the 80%-centered default via the extracted
buildInitialCrop (now in utils/initialCrop). Two paths: when restoring
changed the uri, CropArea's onLoad re-seeds against the correct
dimensions; when nothing was edited the uri is unchanged and onLoad
won't refire, so the container re-seeds directly from imageRef.

Also mirror handleFlip and drive the live preview with the original
bytes, so a stale rotated/flipped blob stops rendering after Reset.
Zoom now anchors on a fixed point (the crop-box center, captured when the
zoom value changes and frozen while the box moves/draws), so scaling keeps
that point on screen instead of drifting. The container owns the anchor and
passes it to both CropArea's transformOrigin and the export math.

Centralize the source-rect mapping into computeSourceRect, shared by
cropImage and PreviewPane, so exported pixels match the on-screen framing.
This also fixes a pre-existing off-center bug: the old pixelCrop.x / z math
assumed a top-left origin while the CSS used center, giving the wrong region
for any off-center crop. The read window is clamped into the image to guard
zoom-out (z < 1) from reading off-canvas.
@github-actions

This comment has been minimized.

The buttons rotate the image by ±90° (via rotateImage), so "flip" was the
wrong term. Rename it everywhere: the enableFlip XML property key becomes
enableRotation, the rotate-left/rotate-right icon assets, CropToolbar props
and aria-labels/titles, the container's handleRotate handler, and all
affected tests, comments, and generated typings.

Also polish the toolbar: render the "Zoom" label at regular weight (Atlas
styles labels bold), and rename the Reset button caption to "Enable reset".
The showResetButton property key is unchanged to avoid breaking existing
app bindings.
@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@github-actions

This comment has been minimized.

@rahmanunver rahmanunver force-pushed the feat/image-cropper-polish-rotate-bw branch from 37dfe9b to 050b169 Compare July 6, 2026 13:45
@github-actions

This comment has been minimized.

@rahmanunver rahmanunver force-pushed the feat/image-cropper-polish-rotate-bw branch from 050b169 to f4ecce1 Compare July 6, 2026 14:23
@github-actions

This comment has been minimized.

Remove the showPreview / previewWidth / previewHeight properties and the
PreviewPane component that rendered a live crop thumbnail next to the canvas.
Drops the XML property group, editor hide-logic, SCSS rule, spec requirement,
and the related test mock props.

The usePreviewSrc hook (rotate/reset live feedback) and the Studio Pro editor
preview machinery are unrelated and left intact.
samuelreichert
samuelreichert previously approved these changes Jul 9, 2026
…ar labels

Add an Accessibility property group with 8 translatable textTemplate props
(3 captions, 5 aria labels) covering the rotate, grayscale, reset, and zoom
toolbar controls. Labels ship en_US/nl_NL defaults and are resolved to plain
strings with English fallbacks before reaching the presentational components.
Replace the plain red 'No image' text with an informational placeholder:
an inline info-circle icon plus 'No uploaded image to crop', laid out on a
single row and colored with Atlas's semantic --brand-info token instead of
the error red. The icon inherits the text color via currentColor.
Comment on lines +38 to +44
if (prevUri.current !== committedUri) {
prevUri.current = committedUri;
if (blobRef.current) {
revoke();
setPreviewSrc(undefined);
}
}

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Low — External side effect (URL.revokeObjectURL) called during render

revoke() triggers URL.revokeObjectURL, which is a side effect with an observable external result. React allows calling setState during render as a derived-state bailout, but external side effects in render violate render-purity. In React 18 concurrent mode an interrupted render can leave the blobRef mutation in place without completing the paired setPreviewSrc(undefined) call on the abandoned pass.

A useLayoutEffect keyed on committedUri avoids the issue — it fires synchronously after paint, like a synchronous effect, but keeps render pure:

Suggested change
if (prevUri.current !== committedUri) {
prevUri.current = committedUri;
if (blobRef.current) {
revoke();
setPreviewSrc(undefined);
}
}
useLayoutEffect(() => {
if (prevUri.current !== committedUri) {
prevUri.current = committedUri;
if (blobRef.current) {
revoke();
setPreviewSrc(undefined);
}
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [committedUri]); // revoke is stable — intentionally omitted

case "portrait3x4":
return "3:4";
case "custom":
return `${values.customAspectWidth}:${values.customAspectHeight}`;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Low — Renders "null:null" when custom dimensions are not yet set

ImageCropperPreviewProps.customAspectWidth / customAspectHeight are typed number | null. Before the user fills in values in Studio Pro the interpolation produces the string "null:null". The null case is not covered by any spec.

Suggested change
return `${values.customAspectWidth}:${values.customAspectHeight}`;
return `${values.customAspectWidth ?? 1}:${values.customAspectHeight ?? 1}`;

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants