Fields: Add MediaEdit component #73537
Merged
Conversation
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
ntsekouras
added
[Type] Enhancement
|
Size Change: +975 B (+0.04%) Total Size: 2.58 MB
ℹ️ View Unchanged
|
oandregal
reviewed
Nov 25, 2025
oandregal
reviewed
Nov 25, 2025
ntsekouras
force-pushed
the
fields/media-edit-control
branch
from
34a6ba0 to
fd68577
Compare
ntsekouras
force-pushed
the
fields/media-edit-control
branch
from
fd68577 to
577a4b8
Compare
|
Flaky tests detected in fb77be9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/20376422173
|
ntsekouras
force-pushed
the
fields/media-edit-control
branch
5 times, most recently
from
90fab95 to
9d99182
Compare
|
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. |
ntsekouras
changed the title
oandregal
reviewed
Dec 18, 2025
| } | ||
| } | ||
|
|
||
| .dataforms-layouts-panel__field-control { |
Member
There was a problem hiding this comment.
What do we need to remove these internal DataViews/DataForm classes from here? It's risky to use them (there are not public API and so can break at any moment).
Contributor
Author
There was a problem hiding this comment.
I can check, but noting that these styles were there from before. They are not part of the actual changes.
oandregal
reviewed
Dec 18, 2025
oandregal
reviewed
Dec 18, 2025
Member
|
Is the media upload working for you? The new uploads only show up for me after reloading the screen: Screen.Recording.2025-12-18.at.17.54.46.mov |
Contributor
Author
Hm... Tested in trunk and this seems an issue for the |
ntsekouras
force-pushed
the
fields/media-edit-control
branch
from
fb77be9 to
57c8efc
Compare
Contributor
Author
That could be quite challenging for a few reasons.
--edit Thinking about it more, maybe we won't need changes in media upload components and re-sort the returned value from the selected values. I'll have to explore in code. It would also need some design (-cc @jameskoster ), and I guess it would be easier to have something similar to the block movers (buttons with icons) and not drag and drop. Screen.Recording.2025-12-22.at.1.24.19.PM.mov |
jameskoster
reviewed
Dec 22, 2025
jameskoster
reviewed
Dec 22, 2025
jameskoster
reviewed
Dec 22, 2025
jameskoster
reviewed
Dec 22, 2025
oandregal
reviewed
Dec 22, 2025
Comment on lines
20
to
22
| setValue: ( { value } ) => ( { | ||
| featured_media: value ? Number( value ) : 0, | ||
| } ), |
Member
There was a problem hiding this comment.
Can we absorb this as part of the MediaEdit?
I understand that it already returns a number (so no need to coerce it again). In that link, perhaps the MediaEdit can remove the undefined type and return 0 when we remove the image? would that work?
Member
There was a problem hiding this comment.
It seems inline with what we do here
Contributor
Author
There was a problem hiding this comment.
Actually I updated this because of the feedback here. 0 is specific to featured image setting and not generic, so it should not be part of MediaEdit.
I can remove the extra Number coercion though. It was just a bit defensive programming :)
Member
There was a problem hiding this comment.
My concern here is that every consumer needs to add the same.
Why can't we do this in the MediaEdit? As far as I understood, isn't this restriction (0 for no featured media image) part of the how the REST API endpoints work?
Member
There was a problem hiding this comment.
As far as I understood, isn't this restriction (0 for no featured media image) part of the how the REST API endpoints work?
I looked at WP REST API, and I can see that it's the approach there; we use 0 as the default value for object ID, like featured media, menu order, parent, or author. So it makes sense for WordPress, and a sensible default, too. But because Data Views should be framework agnostic, I think we need to at least document the behavior of default/empty value here.
Contributor
Author
There was a problem hiding this comment.
We could change the default to zero and document this, but in general we can use this component for updating some post meta that holds an array of ids, where 0 as an empty value doesn't make much sense.
I have no strong opinions and maybe the next usages of this component might dictate what's better to use as default empty value.
Member
There was a problem hiding this comment.
So it makes sense for WordPress, and a sensible default, too. But because Data Views should be framework agnostic, I think we need to at least document the behavior of default/empty value here.
The MediaEdit component we created in this PR is WordPress-specific, so I think it's best to absorb it as part of that instead of asking every consumer to add the same setValue.
Contributor
Author
There was a problem hiding this comment.
WordPress-specific doesn't mean necessarily that is featured image specific. The above examples for author, parent etc.. don't apply here and this will probably be used mostly for custom post meta and even more for fields with multiple values, where storing 0 instead of no value doesn't feel natural to me. I'm still not sold about changing to 0 and would appreciate some more opinions.
oandregal
approved these changes
Dec 22, 2025
Member
oandregal
left a comment
oandregal
left a comment
There was a problem hiding this comment.
API-wise, this is in a good place to land, and we've identified follow ups as well. Thanks for all the work here!
Member
|
I've reopened #72614 and listed there some follow-ups. |
Contributor
|
Great work here, folks! I'm about to head out the door on AFK so apologies for the drive-by comment (I won't see any replies). It's incredibly minor but I noticed that when focused via keyboard the focus outline and the border on the button is duplicated so it looks a tiny bit unpolished in that context: 
Also, I was wondering if the border color should be a bit lighter? It might be clearer when we go to replace the Featured Image button in the post sidebar: 
Currently I feel like the button in this PR is styled to look a bit like an input field, but I'm wondering if the styling of the 
In any case, all very minor nitpicky stuff and easy to iterate on (and I'd defer to @jameskoster on all this stuff anyway). I mostly wanted to capture the thought before I close the laptop for the year. Nice work again, and excited to see this reusable component taking shape! |
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?
Resolves: #72614
This PR starts the work for adding a reusable
MediaEditcomponent that can be shared from other media fields, in WordPress context in order to handle file management through media library or the GB experimental media library modal powered by DataViews.Designs
Right now I've implemented the 'compact' variant of the component and the expanded one should be implemented in a follow up.
Next steps
fieldspackage.site logo, content only controls etc..imgfor images andiconsfor the rest), but we might want to improve these. It can also be a follow up.Testing Instructions
Data Views: new media modalGB experiment.setValueto something like:This code would allow any file types and also multiple selection. You can test combinations with the above by adding specific
allowedTypesand/or removingmultiple. Noting that the with the above code thesavewon't work properly (unless it's a single image), because we're hijacking thefeatured imagepost's value. It should be enough to test the UI/UX though.Recording
The above snippet for multiple files and multiple allowed file types is used for this video.
Screen.Recording.2025-12-18.at.10.28.59.AM.mov