Media Editor Modal: In the image cropper, pan when dragging the crop instead of constraining to the visible area#77899
Merged
Conversation
andrewserong
added
[Feature] Media
This was referenced May 4, 2026
|
Size Change: +645 B (+0.01%) Total Size: 7.91 MB 📦 View Changed
ℹ️ View Unchanged
|
|
Flaky tests detected in 745631c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/25356535455
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates the experimental media editor cropper so users can expand a crop beyond the currently visible canvas area by temporarily panning the viewport during resize, then settling the crop back into view afterward. It fits into the broader media-editor work by making crop resizing feel less constrained without introducing a full free-pan/navigator UI yet.
Changes:
- Removes the container-viewport clamp from
getCropBoundsso resize handles can reach the full transformed image bounds. - Adds transient viewport state infrastructure and wires the cropper to pan the canvas while a crop resize overflows the visible area.
- Updates crop-bound tests to reflect the new “full image AABB” behavior.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
packages/media-editor/src/image-editor/react/hooks/use-viewport-state.ts |
Adds a hook-backed viewport state API for transient pan/zoom/reset actions. |
packages/media-editor/src/image-editor/react/components/viewport-provider.tsx |
Introduces context/provider plumbing so cropper internals can read and update viewport state. |
packages/media-editor/src/image-editor/react/components/cropper.tsx |
Implements resize-time viewport panning, settle/reset behavior, and wraps the cropper in the new provider. |
packages/media-editor/src/image-editor/core/viewport-state.ts |
Adds the viewport reducer and default viewport state. |
packages/media-editor/src/image-editor/core/types.ts |
Defines viewport state/action types used by the new reducer and hook. |
packages/media-editor/src/image-editor/core/test/camera.ts |
Updates getCropBounds expectations to match the new full-image-bound semantics. |
packages/media-editor/src/image-editor/core/containment.ts |
Changes getCropBounds to return the transformed image AABB without intersecting the container viewport. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If 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. |
ramonjd
reviewed
May 4, 2026
Member
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
Thanks @andrewserong
I played with this and it's feeling natural to drag out and each way, things remain accurate. I think you've just about cracked it!🥇 The holy grail of cropping!
I added some notes from GPT 5.5's review - I know you could have done this yourself, but I thought I'd them as a confidence check. Sorry for the noise. Feel free to just ignore them.
The only things I noticed as a human so far was a little jankiness.
The dimming overlay seems to jump to the settled crop rect during settle. It looks like it is catching up or tearing away from the crop frame.
Here's a slowed-down version:
Kapture.2026-05-04.at.20.00.35.mp4
GPT's solution was to give the dimming overlay the same geometry transition as the stencil, and make sure the grid disappears immediately when settle starts.
That improved things. It also said that
Animating a huge box-shadow can be repaint-heavy. Four panes are more code, but usually smoother and easier for the browser to paint.
I tried that locally, and it seemed to improve the animations a lot with a few other bugs 😆 so I think it's a follow up thing more than anything.
Comment on lines
+379
to
+382
| setViewportPan( { | ||
| x: -rightOverflow + leftOverflow, | ||
| y: -bottomOverflow + topOverflow, | ||
| } ); |
Member
There was a problem hiding this comment.
It also wanted an equality check here for smoother interaction/performance.
const nextViewportPan = {
x: -rightOverflow + leftOverflow,
y: -bottomOverflow + topOverflow,
};
if ( ! arePointsEqual( viewportPanRef.current, nextViewportPan ) ) {
viewportPanRef.current = nextViewportPan;
setViewportPan( nextViewportPan );
}The handleCropChange check avoids dispatching at all on the drag hot path. That means no reducer call, no context provider update attempt, and no React scheduling for viewport state when the computed pan is unchanged.
The reducer check is a correctness/backstop guard. It catches any other caller of setViewportPan() and keeps the reducer referentially stable if someone dispatches the same pan later. It also makes the reducer more robust as the viewport API grows.
Contributor
Author
There was a problem hiding this comment.
Cheers. For this one, I think we've already fixed it with the feedback on updating the reducer to bail if values match, so the consumer (this callback) doesn't need to worry about performing the check and avoiding calling setViewportPan. Rather, the code here can happily call with an update and the reducer is smart enough to do a no-op if it doesn't need to change anything.
Contributor
Author
|
Thanks for the review and detailed notes! I think I've implemented most of the feedback so far. I've pushed a couple of fixes.
Ah, I see! I'm still digging into that one. I thought I had a fix, but then I noticed the fix added an odd little bounce, so going to spend a little more time on it. |
… overlay always renders precisely with the pan
Contributor
Author
|
Alrighty, I think I've fixed it. After a little back and forth, I've added in a 2026-05-05.11.18.08.mp4This should be ready for review again now. Let me know if the changes look overkill or if I've missed something! |
ramonjd
approved these changes
May 5, 2026
Member
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
This is so sweet. Thanks for taking this UX challenge on - it's been worth it.
I tested on Firefox/Chrome/Safari and mobile safari:
ScreenRecording_05-05-2026.11-39-37_1.mp4
Things are working as I'd expect, and the resulting crop saves as expected.
I've left some questions comments, all of which I implemented and tested locally and things work as expected. I reckon the transition + useMemo are harmless enough to get in - up to you.
Nice work.
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Contributor
Author
|
Thanks again for all the back and forth on this one!
Cheers, that's some good polish, I've committed those in 745631c I'll merge this in once the tests pass 👍 |
Member
|
Looking forward to this one. 🕺🏻 |
This was referenced May 5, 2026
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.
What?
Part of:
In the image cropper in the media editor modal experiment, allow dragging a drag handle beyond the current viewable area of the cropper canvas. Pan to follow the drag, and then settle once the interaction is complete.
This effectively (to a user) should be the inverse of the behaviour we have when reducing a crop size. When we reduce the crop, we then zoom in to the new crop size so that it sits comfortably in the viewport. This PR is the reverse of that behaviour for when we expand the crop area.
Why?
In
trunkit's very hard to expand the crop size again, as we quickly reach thecanvasSizebounds of the cropper. A user needs to either manually zoom out or perform multiple gestures in order to expand the crop size again.While in the future we might offer more fine-grained control over the viewport (e.g. the navigator idea I was looking at in #77879), the goal here is to simply add a little panning in order to allow expanding the crop size, without creating a complex set of new UI gestures or components.
How?
canvasSizefromgetCropBounds. The idea in this PR is that the crop isn't bounded by the viewport area, and instead we'll pan (and then zoom) to try to fit things into frame.Testing Instructions
Screenshots or screencast
Before
Note how easy it is to get stuck and how many times it takes to try to get the crop back to its original size:
2026-05-04.15.19.18.mp4
After
It should now feel much easier to spring back to a larger size:
2026-05-04.14.34.33.mp4
Use of AI Tools
Claude Code