Media Editor: add cropper controls to the media editor modal#77540
Media Editor: add cropper controls to the media editor modal#77540
Conversation
There was a problem hiding this comment.
Pull request overview
Adds image-cropper controls to the experimental Media Editor modal, wiring the cropper controller through context so header actions (Save), the bottom bar, and the Crop sidebar tab share the same state.
Changes:
- Introduces an image-only bottom toolbar (rotate/flip/fine-rotate slider/reset) and a new Crop sidebar tab (aspect ratio + freeform toggle).
- Wraps the image modal content in
CropperProviderand gates Save oncontroller.isDirty. - Updates modal skeleton layout/CSS so the footer is in-flow and remains visible on narrow viewports.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| packages/media-editor/src/style.scss | Includes new bottom-bar styles in the package stylesheet entrypoint. |
| packages/media-editor/src/components/media-editor-modal/style.scss | Reworks InterfaceSkeleton layout so the footer/bottom bar doesn’t overlap the canvas and stays visible. |
| packages/media-editor/src/components/media-editor-modal/index.tsx | Adds CropperProvider wiring, image-specific body (tabs + bottom bar), and Save gating via isDirty. |
| packages/media-editor/src/components/media-editor-crop-panel/index.tsx | New Crop sidebar panel with aspect-ratio presets + freeform toggle and an aspect-ratio resolver helper. |
| packages/media-editor/src/components/media-editor-canvas/index.tsx | Switches to using the shared cropper controller from context; forwards aspectRatio/freeform props to <Cropper>. |
| packages/media-editor/src/components/media-editor-bottom-bar/style.scss | New styling for the bottom toolbar layout and slider width constraint. |
| packages/media-editor/src/components/media-editor-bottom-bar/index.tsx | New bottom toolbar component implementing rotate/flip/fine rotation/reset using the shared cropper controller. |
💡 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. |
|
Size Change: +1.56 kB (+0.02%) Total Size: 7.76 MB 📦 View Changed
ℹ️ View Unchanged
|
| /** | ||
| * Sidebar panel for crop-shape controls — aspect-ratio presets and | ||
| * freeform toggle. The tactile verbs (rotate, flip) live in the | ||
| * bottom toolbar instead. | ||
| * @param props |
There was a problem hiding this comment.
I like the logic for which to keep in the footer and which to be in the sidebar. It also means that for mobile (or close to it) viewports there's still some useful tactile controls while more technical things can be edited from the sidebar.
That said, in follow-ups I do think it'll be useful to expand the sidebar with further controls for keyboard entry of specific things like pixel dimensions for exact crop sizes, etc. But nothing that needs to be added in this PR!
There was a problem hiding this comment.
Good call. Agree with the keyboard controls. Everything is up for grabs really! Happy to try different things out to see what sticks.
| * @param props | ||
| * @param props.onReset | ||
| */ | ||
| export default function MediaEditorBottomBar( { |
There was a problem hiding this comment.
A bit of a nitpick and totally optional — this could be a general MediaEditorToolbar rather than BottomBar as it's the overall interface that determines its placement.
There was a problem hiding this comment.
👍🏻 Yes, let's remove the British 70s comedy innuendo 🤣
There was a problem hiding this comment.
👍🏻 Yes, let's remove the British 70s comedy innuendo 🤣
LOL!
| // body (canvas, bottom bar, sidebar Crop panel) and the header actions | ||
| // (Save button, gated on `isDirty`) share one cropper controller. | ||
| // React context flows through `<Modal>`'s portal. | ||
| const content = ( |
There was a problem hiding this comment.
Another code style nit, but if we need to compose, I'd be in favour of having a wrapper component that renders the <CropperProvider rather than placing the modal content in const content. It might be a little more readable and mean we don't need to have an explanatory comment?
| <Tabs> | ||
| <MediaEditorModalSidebar tabs={ tabs } /> | ||
| </Tabs> | ||
| <InterfaceSkeleton |
There was a problem hiding this comment.
The whole modal fork was a little convoluted. Thanks for pointing it out.
The trade off is that CropperProvider is always mounted/wraps the entire Modal unconditionally to drive the tools. I think it's a pretty cheap trade off. We can change it later.
There was a problem hiding this comment.
The trade off is that CropperProvider is always mounted/wraps the entire Modal unconditionally to drive the tools. I think it's a pretty cheap trade off. We can change it later.
I think so, too. One option is to do the wrapping outside of this component altogether, but now I'm being really nitpicky 😄
I'd say what you've got here now's looking pretty good, as you say we can change it later!
There was a problem hiding this comment.
One option is to do the wrapping outside of this component altogether, but now I'm being really nitpicky
Neat idea 👍🏻
There was a problem hiding this comment.
One option is to do the wrapping outside of this component altogether, but now I'm being really nitpicky
Neat idea 👍🏻
There was a problem hiding this comment.
This is looking really good, and feels very close to me! One bug I found is that you can no longer save if you just changed some metadata from the Details tab:
2026-04-22.17.18.43.mp4
UX-wise I really like it: provides useful controls that are present irrespective of the sidebar being open, and creates a good space for us to continue building out the controls in follow-ups. Also, in terms of where the controls are, there's some common patterns in use here with other photo editing tools (e.g. Google Photos), so it feels like we're building a good interface here IMO.
Will take this for another spin tomorrow!
| value={ media ?? undefined } | ||
| onChange={ handleChange } | ||
| settings={ { fields } } | ||
| <CropperProvider key={ media?.id ?? 'none' }> |
There was a problem hiding this comment.
Good question! The key is there allegedly to force a remount if the modal is ever called with a new id while it's open. But the modal closes anyway on a close/save, so it's a premature optimisation.
| const { isDirty } = useCropper(); | ||
| const saveDisabled = | ||
| isSaving || ! hasMedia || ( requireDirty && ! isDirty ); |
There was a problem hiding this comment.
I think this is where we're running into trouble where the Save button is disabled when non-cropping related changes have been made. Also, if we include entity-dirtiness to cover the Details tab, then we might not need a requireDirty prop as we can be consistent... anyway, easy to go down a rabbithole with this stuff, but the main thing to fix up is ensuring folks can save when they change Details (e.g. title, alt text) but perform no changes to the crop itself.
There was a problem hiding this comment.
the main thing to fix up is ensuring folks can save when they change Details
May as well try to wire it up properly with hasEditsForEntityRecord or something. Thanks for testing! I'll try not to be in such a rush tomorrow
|
Flaky tests detected in d0c0f37. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/24774607729
|
Adds a bottom toolbar (rotate, flip, fine rotation, reset) and a Crop sidebar tab (aspect ratio, freeform) that share a single cropper controller with the canvas. Freeform crop is on by default; Save is gated on cropper `isDirty`; Reset also clears aspect-ratio and freeform state back to defaults.
Wrap the DEFAULT_ASPECT_RATIOS preset labels in __() so they land in the translation catalog.
Replace Flex/FlexItem with @wordpress/ui Stack. The components package README recommends migrating to @wordpress/ui, and the modal's sidebar panels already use Stack — switching the bottom bar keeps the package consistent. The rotation slider's `flex: 1 1 auto` (previously `FlexItem isBlock`) moves to CSS so the slider still absorbs extra width between the icon buttons and the Reset button.
The component's placement is a concern of the hosting interface, not the component itself — 'toolbar' describes what it is (controls) instead of where it happens to live (the bottom). Renames the component, directory, import, and CSS class names.
The number input next to the slider already surfaces the current value, and the handle tooltip gets clipped by the modal edge when the thumb is at either extreme.
Previously the modal forked on media type: images got one Modal subtree wrapped in CropperProvider with its own InterfaceSkeleton, non-images got a second subtree with a different InterfaceSkeleton. Header actions were also split into two components so the image path could read the cropper's `isDirty`. Fold both paths into a single InterfaceSkeleton whose parts (canvas, toolbar, sidebar tabs) each decide what to render based on media type. CropperProvider is always mounted — it's just a useReducer with no side effects — so the header actions can read `isDirty` unconditionally. The aspect-ratio / freeform UI state lives on the modal and resets when the media id changes. This also makes it much easier to add non-image editing surfaces (video, PDF, etc.) in future — each part grows its own branch rather than forking the whole modal.
Save was gated on the cropper being dirty for image media, which
blocked saving when the user only edited metadata (e.g. caption, alt).
Use core-data's `hasEditsForEntityRecord` so either a cropper edit or
a metadata edit enables Save, regardless of media type.
Also clarify the comment above `<CropperProvider key={…}>` explaining
the remount guard is belt-and-braces for future flows that swap the
edited attachment without closing the modal.
0af13e4 to
d0c0f37
Compare
andrewserong
left a comment
There was a problem hiding this comment.
Thanks for all the back and forth on this one! Tests well, gives us a good structure for the controls and lots of ways to enhance and extend this in follow-ups. Just capturing a couple of other thoughts for follow-ups:
In mobile view, we might want to progressively hide the input field on rotation so that it's just the slider, otherwise the slider gets really narrow:
Also for the right-hand sidebar: input / range control for the zoom level. But again, not a blocker for now.
Saving metadata is working again, so looks good to merge to me! 🚀
Great idea. I'll do a quick follow up. |
This too 👍🏻 |
|
Follow up for those two items here: |
What? How?
Part of:
Follow-up to #77537. Adds cropper controls to the experimental Media Editor modal:
All controls share one cropper controller via CropperProvider wrapping the , so context flows through the portal to the Save button.
Testing Instructions
Screenshots or screencast
Kapture.2026-04-22.at.15.37.07.mp4
Use of AI Tools
Claude cod help to scaffold, test and build the components