Media Editor: Make zoom floor coverage-aware instead of fixed at 1x#78222
Merged
Conversation
ramonjd
added
[Feature] Media
Contributor
There was a problem hiding this comment.
Pull request overview
Updates the Media Editor image cropper to use a coverage-aware minimum zoom (instead of a fixed 1x floor), allowing valid sub-1x zoom when the rotated image still fully covers the crop area. This aligns the zoom floor with the real invariant (coverage) across interactions and UI surfaces.
Changes:
- Removes the hard
MIN_ZOOM = 1clamp from coverage math and introduces a sharedgetMinZoom(state)helper for consistent min-zoom behavior. - Updates Cropper interactions and zoom sliders (panel + story) to use the coverage-aware minimum by default, while keeping
minZoomas an explicit override. - Adds/extends tests for sub-1x zoom scenarios across containment, settling, and preview/export parity.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/media-editor/src/image-editor/stories/rectangle-crop.story.tsx | Uses getMinZoom(state) for story zoom slider minimum to match runtime behavior. |
| packages/media-editor/src/image-editor/react/hooks/use-cropper-state.ts | Updates public hook docs to reflect coverage-aware clamping behavior. |
| packages/media-editor/src/image-editor/react/components/cropper.tsx | Defaults Cropper interaction min-zoom to coverage-aware minimum unless overridden. |
| packages/media-editor/src/image-editor/core/test/state.ts | Adds tests for degenerate crop zoom floor and sub-1x settling behavior. |
| packages/media-editor/src/image-editor/core/test/camera.ts | Adds restrictPanZoom test ensuring sub-1x zoom is allowed when coverage holds. |
| packages/media-editor/src/image-editor/core/state.ts | Adjusts zoom clamping to allow sub-1x (down to an absolute minimum) when an image is loaded. |
| packages/media-editor/src/image-editor/core/containment.ts | Implements coverage-aware getMinZoom(state) and removes implicit 1x floor in cover math. |
| packages/media-editor/src/image-editor/core/constants.ts | Introduces ABSOLUTE_MIN_ZOOM and clarifies MIN_ZOOM as the resting default. |
| packages/media-editor/src/components/media-editor-modal/test/build-modifiers.test.ts | Adds a sub-1x zoom parity probe case for preview/export consistency. |
| packages/media-editor/src/components/media-editor-crop-panel/index.tsx | Updates sidebar zoom control to use getMinZoom(state) for its minimum. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Size Change: +88 B (0%) Total Size: 7.97 MB 📦 View Changed
ℹ️ View Unchanged
|
|
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. |
Contributor
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
Generally looks good, and nice catch with this one — for this use case it's way better being able to zoom out to below 1, and when not fine-rotated the min is still 1 👍
I ran into one bug that appears to be introduced in this PR. On trunk, if I zoom in very, very small, I can keep going to a very pixelated size. In this PR, when I do the same, it unexpectedly performs a pan and resize... not sure if it's related to the change to restrictPanZoom? Or... if I've got a weird situation happening in my specific testing environment 😄
In the below video, it's the odd pan and resize after the last change in the crop that I make:
2026-05-13.16.23.24.mp4
Member
Author
Nice catch! I'll look into it. |
Member
Author
So this is going to require a larger, defensive fix to restrict the pan zoom and define a floor min zoom. Currently it's percentage based, so I think we should put a pixel floor on it, e.g., 32x32 or something. I'll commit something here... I hope it's not going to be another one of "those" PRs 😄 Always a rabbit hole! |
ramonjd
force-pushed
the
fix/image-editor-min-zoom-computed
branch
from
ccb73ae to
4d75a9a
Compare
Member
Author
|
I've broken the drag handle zoom out. 🤦🏻 Fixing |
|
Flaky tests detected in c765ed8. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/26081345192
|
ramonjd
force-pushed
the
fix/image-editor-min-zoom-computed
branch
from
4d75a9a to
62779b0
Compare
70 tasks
ramonjd
force-pushed
the
fix/image-editor-min-zoom-computed
branch
from
62779b0 to
8fe5e8c
Compare
…low 1 when containment math confirms the crop remains covered. This keeps 1x as the default/resting zoom, but removes the hard floor that prevented thin, tall crops on fine-rotated portrait images from filling the available space. Add coverage for: - sub-1 containment on fine-rotated portrait crops - settle behavior for tall rotated crops - export/server modifier parity at sub-1 zoom
…ues below 1x while keeping zoom bounded by the configured maximum. Clamp containment zoom to MAX_ZOOM so crop settling and rotation cannot push state beyond the UI control range, then let crop restriction handle cases that cannot be covered at the capped zoom. Add regressions for fine-rotated sub-1 crops, degenerate crop zoom floors, MAX_ZOOM capping, settle behavior, and export modifier parity.
ramonjd
force-pushed
the
fix/image-editor-min-zoom-computed
branch
from
8fe5e8c to
17b1793
Compare
…avior in the media editor cropper. Adjusted documentation for `minZoom` and `maxZoom` properties, clarifying their new constraints. Added reference to `getMinZoom` function in architecture documentation.
ramonjd
commented
May 19, 2026
| const zoomFromBeta = 2 * spanBeta; | ||
|
|
||
| return Math.max( 1, zoomFromAlpha, zoomFromBeta ); | ||
| return Math.max( zoomFromAlpha, zoomFromBeta ); |
Member
Author
There was a problem hiding this comment.
This is the most important line, where the old hard 1x floor is removed from the geometric minimum.
That enables the new behavior: zoom can go below 1 when coverage math says the crop is still fully covered.
Contributor
There was a problem hiding this comment.
Thanks for flagging the key part of this! Helps with the reading 👀 😄
…N_ZOOM constant for coverage-aware zoom. Update getMinZoom function to handle invalid image dimensions and ensure it returns a finite value for non-finite rotation and crop fields. Add unit tests for getMinZoom to validate new behavior.
andrewserong
approved these changes
May 19, 2026
Contributor
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
This is testing great for me now, thanks for the follow-ups both in this PR and across your other UX and code hardening PRs — it feels like things are getting more and more stable.
Just left a tiny nit on one of the tests that I struggled to comprehend while reading. No need to address the feedback if it's working as you intend!
LGTM 🚀
| const zoomFromBeta = 2 * spanBeta; | ||
|
|
||
| return Math.max( 1, zoomFromAlpha, zoomFromBeta ); | ||
| return Math.max( zoomFromAlpha, zoomFromBeta ); |
Contributor
There was a problem hiding this comment.
Thanks for flagging the key part of this! Helps with the reading 👀 😄
Comment on lines
+395
to
+400
| const expectedUncappedZoom = | ||
| state.zoom * ( newH / state.cropRect.height ); | ||
| if ( expectedUncappedZoom > MAX_ZOOM ) { | ||
| skippedCappedCount++; | ||
| continue; | ||
| } |
Contributor
There was a problem hiding this comment.
Tiny nit: this stood out to me a little in this test's changes. It feels a little odd that we have a test that's randomly skipping an unknown number of the iterations. Is there an alternate thing we could test in this branch that we would expect to happen for zooms above the threshold? Or change the generation math to be within ranges that we'd expect?
Not a blocker for landing this PR, but my read of this test is that skipping iterations might diminish its usefulness 🤔
Member
Author
There was a problem hiding this comment.
TBH I missed this. Lemme see what's going on. Thanks for flagging it!
Member
Author
There was a problem hiding this comment.
Yeah, I don't get why we just don't generate cases whose expected settle zoom is under MAX_ZOOM. I think it was shoe-horned in.
I'll update.
Thanks for testing this!! I think it's nearly the last quality of life update.
… logic. Adjust crop and zoom calculations for improved test accuracy, ensuring expected uncapped zoom values remain within MAX_ZOOM constraints. Remove unnecessary pass count tracking for cleaner test execution.
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:
Replace the hard
MIN_ZOOM = 1floor with a coverage-aware minimum. The cropper can now zoom below 1x when the crop area is still fully covered by image pixels. 1x remains the default/resting zoom.Kapture.2026-05-14.at.12.05.28.mp4
Why
With the old floor, rotating an image by a small angle and drawing a thin, tall crop made it impossible for the crop area to use the full vertical space. The image was big enough to cover that slice at smaller zoom, but the 1x floor blocked it. The real invariant is coverage, not "image at least 100%".
Manual testing