Extract media editor save hook#78225
Conversation
ramonjd
force-pushed
the
update/media-editor-save-hook
branch
from
c32ba0a to
e14ad97
Compare
ramonjd
force-pushed
the
update/media-editor-save-hook
branch
from
e14ad97 to
87c131d
Compare
ramonjd
added
[Feature] Media
|
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. |
There was a problem hiding this comment.
Pull request overview
This PR refactors the media editor modal’s save flow by extracting the save logic out of MediaEditorContent into a dedicated useSaveMediaEditor hook, keeping save-related concerns (crop modifier generation, metadata bundling, entity-store updates, notices, and onSaved handling) together while simplifying the UI shell component.
Changes:
- Added
useSaveMediaEditorhook encapsulating the full save pipeline for cropped and non-cropped edits. - Updated
MediaEditorContentto delegate saving and notices context to the new hook while keeping discard/interaction logic local. - Re-exported
MediaEditorSaveResulttype from the new hook module for continuity.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/media-editor/src/components/media-editor/use-save-media-editor.ts | New hook implementing the media editor save flow (crop + metadata + entity updates + notices). |
| packages/media-editor/src/components/media-editor/index.tsx | Removes inline save implementation and wires saving through useSaveMediaEditor and shared notices context. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
andrewserong
left a comment
andrewserong
left a comment
There was a problem hiding this comment.
Nice, this looks like a good and clean refactor to me!
✅ Crops work and save as on trunk
✅ Metadata gets saved correctly alongside the crop
✅ Updating metadata separately from a crop works as on trunk
The only thing of interest I found from giving this a quick pass with Claude was that the refactor might make it easier to add tests for some of the save logic in follow-ups:
The extracted seam is testable in isolation now, and packages/media-editor/src/components/media-editor/test/ happens to be empty, so this would be a natural place to add
useSaveMediaEditor unit tests in a follow-up if useful (modifier branch vs. non-modifier branch, post-parent carry-over, error formatting).
LGTM! 🚀
What?
Part of:
MediaEditorContentinto a dedicateduseSaveMediaEditorhook.onSavedresult handling together.MediaEditorContentfocused on editor layout, interaction state, discard behavior, and frame wiring.Why?
This makes the modal save path easier to reason about independently from the UI shell, while preserving the current crop + metadata save behavior.
Testing
There should be no regressions.
Enable the media modal experiment.
Insert an Image block into a post.
Hit the crop icon in the Image block toolbar.
Crop an image and save it.
Crop an image, change the metadata and save it.
Change the metadata and save it.