Closed
Conversation
5a82f49 to
0a23229
Compare
|
Size Change: +20 B (0%) Total Size: 7.76 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in b817710. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24874379975
|
0a23229 to
63835d3
Compare
Hoist useCropperState to the modal so the Save handler can read the current transform state directly. When the cropper has edits, route through the private editMediaEntity action (creates a duplicate attachment via REST /edit). Otherwise fall through to the existing saveEditedEntityRecord path for metadata-only saves. On error, show a single snackbar; keep the modal open so edits aren't lost.
Previously buildModifiers only emitted a crop when the cropRect shrunk below full-frame. If the user zoomed in and panned to frame a region with the crop handles still at the image bounds, Save produced zero modifiers and fell through to the metadata path — no crop persisted. Treat any zoom > 1 or non-zero pan as a framed region, reusing the existing cropRect-derived math.
Replaces the legacy `modifiers` + mixed-frame `source_region` wire shapes with a two-part description of the user's framed image: 1. `transform` — rotate then flip, applied to source pixels. 2. `crop` — axis-aligned rect in the post-transform canvas. Client emits the payload directly from cropper state (`build-edit-payload`) without routing through the legacy `getSourceRegion` helper that mixed source-frame and rotated-frame coordinates. Server applies rotate → flip → crop with no AABB or center-rotation math. At snap angles (multiples of 90°) the result is pixel-exact WYSIWYG. Fine rotation is documented as visually close but not pixel-identical — to be addressed separately. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The snap-AABB and full-AABB share the source-image center, so the crop rect's conversion between frames is a pure translation by `(fullCenter - snapCenter)` — not a rotation by the fine angle. The server's full-angle rotate already orients the canvas correctly; the crop rect just slides to the full AABB's origin. Adds a parity test that asserts the export camera and the server pipeline produce the same source pixel for 6048 state/probe combinations (8 rotations × 3 zooms × 3 pans × 4 flips × 3 cropRects × 7 probes), covering the 201° case that motivated the fix. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
63835d3 to
ae90a44
Compare
- Add missing `notices` reference to media-editor tsconfig (added when wiring handleSave's error notice). - Tag unused `$route`/`$handler` params on the `rest_dispatch_request` callback (filter signature is fixed). - Use `'default'` text domain on `__()` calls that mirror Core's existing translations. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Fold the one-line normalizer helpers into `process()` at their only callsite. Drops two public functions from the module's namespace (`gutenberg_source_region_normalize_transform` and `gutenberg_source_region_normalize_crop`) and their dedicated tests — the remaining `validate` helper is the only piece with enough branching to warrant isolated unit coverage. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
ramonjd
commented
Apr 24, 2026
Comment on lines
+77
to
+79
| if ( ! isset( $params['transform'] ) && ! isset( $params['crop'] ) ) { | ||
| return null; | ||
| } |
Member
Author
There was a problem hiding this comment.
Returning null here means “I didn’t handle it, continue to the next handler.”
That means legacy requests will fall through to Core’s registered edit_media_item callback untouched.
Mirrors Core's `edit_media_item` behavior so a `{ transform, crop }`
payload that resolves to no change (rotation 0°, no flips, and either
no crop or a crop matching the full source canvas) returns HTTP 400
instead of writing a duplicate file and attachment row.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
A save that combines a transform (crop/rotate/flip) with Details-tab edits (title, caption, description, alt_text) previously dropped the metadata because `handleSave` only hit one path: `editMediaEntity` for image ops or `saveEditedEntityRecord` for field edits, never both. Bundle any staged field edits from core-data into the same `/edit` request, and teach the server to honor them when inserting the new attachment (mirrors Core's `request ?? original` fallback). Reset the old record's edits afterwards so the source attachment doesn't appear dirty in the Media Library once its metadata has migrated to the child. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
2 tasks
Member
Author
|
Closing in favour of #77641 Leaving this here for consideration in the longer term. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
Follow up to #77540
Kapture.2026-04-24.at.15.36.07.mp4
Wires the experimental media-editor modal's Save button to persist crops, rotations, and flips via a new experimental REST handler, replacing the unused modal save stub.
The save path uses a canonical
{ transform, crop }contract instead of the legacymodifiersarray:transform— source-pixel ops the server applies in order:rotate(rotation)thenflip.crop— an axis-aligned rect in the post-transform canvas, in canvas pixels.Handled by a new experimental handler at
lib/experimental/source-region-edit.phpthat interceptsPOST /wp/v2/media/{id}/editwhen the body carriestransform/crop.Existing
modifiers-based requests pass through untouched.Why
The
modifierscontract couples the transform sequence to the wire format and can only express axis-aligned crops against the post-rotate AABB (axis-aligned bounding box, which is the smallest upright rectangle thatcontains the rotated image) between each modifier — so snap-angle (90°/180°/270°) crops required the client to reason about the server's intermediate frames and broke in subtle ways. The canonical shape decouples "what the user sees" from "what operations produced it" and lets the client describe the result once, in one frame.
Fine rotation (non-multiple of 90°) is the straighten-then-crop case: the server produces the fully-rotated image and takes an axis-aligned slice at the stencil-framed position.
Test plan
npm run test:unit -- packages/media-editor— includes parity cases asserting the export camera and the server pipeline map every(u, v)to the same source pixel across 8 rotations × 3 zooms × 3 pans × 4 flips × 3 cropRects × 7 probesvendor/bin/phpunit phpunit/experimental/source-region-edit-test.phpmodifiers-based flows still work (endpoint passthrough)