Repository navigation
Media editor: Let the crop instruction helper read the cropper - #86147
Conversation
`buildModifiers` took the cropper's edit state and the image size as two separate arguments, so every caller had to pull the size out of the state itself. It now takes the cropper controller and reads both from it. Callers pass one value and no longer reach into the shape of the edit state. The output is unchanged and the existing parity test still passes. Adds a test for the case where no image has loaded yet.
🤖 PR meta 🤖🎉 PropsIf you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. Props Bot commandsYou can trigger Props Bot actions by commenting on this PR:
Updated as activity occurs, without notifying anyone named here. Add the 📦 Bundle sizeSize Change: +11 B (0%) Total Size: 8.58 MB 📦 View Changed
⚡ PerformanceShow the resultsClient side metrics exclude the server response time. front-end-block-theme
front-end-classic-theme
media-processing
media-upload
post-editor
site-editor
🏁 Flaky testsSome tests passed with failed attempts. The failures may not be related to this commit but are still reported for visibility. See the documentation for more information. Custom Completer: should insert option in Undo restores the post selection when another synced entity is loaded (RTC WebSocket) in
|
There was a problem hiding this comment.
🟢 Approval recommended
The review identifies no active issues requiring changes to the controller-based buildModifiers refactor.
No open issues 🎉
What changed in this PR
Refactors the media editor’s internal buildModifiers helper to read edit state and image dimensions from the cropper controller, preparing for reuse under #86145 without changing its output or exports.
Changes:
- Replaces the helper’s two arguments with a controller argument.
- Moves the missing-image guard into the helper and simplifies the save caller.
- Updates parity tests and adds coverage for an unloaded image.
| File | Description |
|---|---|
packages/media-editor/src/components/media-editor/use-save-media-editor.ts |
Passes session.cropper directly to the helper. |
packages/media-editor/src/components/media-editor-modal/test/build-modifiers.test.ts |
Updates helper calls and tests the missing-image case. |
packages/media-editor/src/components/media-editor-modal/build-modifiers.ts |
Reads state and natural image dimensions from the controller. |
Review effort: Balanced
andrewserong
left a comment
There was a problem hiding this comment.
Looks like a tidy change to me (and makes for a cleaner buildModifiers signature).
Smoke-tested a bunch of crops in the editor and all seemed to be identical between how they looked in the editor and how they appear post-crop.
Thanks for the example consumer usage, too!
LGTM ![]()
What?
See #86145.
The helper that turns the cropper's edit into the instructions for the media REST endpoint (
buildModifiers) now takes the cropper controller, instead of the edit state plus the image size as two arguments.buildModifiersis a helper to build a Core-compatible modifiers array for the attachments REST controller from the cropper state.Why?
This is a first small step of #86145, which makes the image cropper reusable by other editing flows.
With two arguments, every caller had to pull the image size out of the edit state itself and reach into its shape. If the helper reads both from the controller, callers pass one value and the shape of the edit state stays out of their code, which makes the helper easier to expose to other packages later.
How?
buildModifiersreadsstateandstate.imagefrom the controller it is given. The one caller, the media editor's save path, now passessession.cropper, and its own "no image loaded yet" check moved into the helper. The output is identical. Nothing is exported or changed in the hook, and there is no changelog entry because no consumer calls the helper.A new test covers the case where no image has loaded yet.
Testing Instructions
npm run test:unit -- packages/media-editor. The helper's parity test (6,000+ cases that compare its output with the export camera) and the rest of the package's tests should pass.2026-11-02.13.57.41.mp4
Example usage by outside consumer
This is just FYI
Use of AI
Claude helped plan and execute