Media: Add media-specific fields for use with Attachment post types and DataViews/DataForm #73071
Conversation
andrewserong
added
[Type] Enhancement
|
Size Change: +2.69 kB (+0.1%) Total Size: 2.58 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in 13872d9. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/19155213532
|
andrewserong
force-pushed
the
try/add-media-fields
branch
from
13872d9 to
7a0de9c
Compare
andrewserong
requested review from
aaronrobertshaw,
ramonjd,
talldan and
tellthemachines
|
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. |
|
Okay, I've updated this to move to a |
|
I wonder if the media picker situation is a one-off, or if we may run into other cases of complex fields that use nested dataviews inside them. Would it be worth having a |
I was just thinking about that with the page parent use case for a generalised post picker. (I.e. what if the parent field needed to render a DataViewsPicker to pick posts, and that also includes a parent field). I think it's potentially a hard question to answer without playing around with how we might go about implementing it. That's one of the reasons I was leaning toward the idea in this PR of a private package, so that we can try out the ideas in the repo, and know that we can still move things around as we need to. That said, it might be worth playing around with the generalised post picker in a messy WIP PR to tease out some of these ideas further, if we're unsure of where to put the fields for now 🤔 |
Yeah I think so! That is something we definitely want to build, so it can help inform this work too. |
| /> | ||
| ); | ||
| }, | ||
| enableSorting: true, |
There was a problem hiding this comment.
Is it possible to sort by caption or are backend changes required?
&per_page=20&page=1&status=inherit&order=asc&orderby=caption&search=&media_type%5B0%5D=image&_locale=user returns a 400
| /> | ||
| ); | ||
| }, | ||
| enableSorting: true, |
There was a problem hiding this comment.
Same question as for caption.
orderby=description isn't accepted. I guess that's to be added in 🤔
&per_page=20&page=1&status=inherit&order=asc&orderby=description&search=&media_type%5B0%5D=image&_locale=user
There was a problem hiding this comment.
Sorry I missed your comments about the caption and description fields! I'm not sure how useful it'd be because these are rich text fields, so sorting may not be accurate to what's visible. I.e. if the data within the field begins with <em> or <strong> that could adversely affect the usefulness of the sorting.
Maybe we can switch these to enableSorting: false for now 🤔
|
Just checking, does this fix #72612 or just partly? |
Nearly! This PR does everything flagged in that issue except for The thing I'm looking into right now is how a PostPickerModal might work, to verify whether or not going with this separate media fields package is going to work okay. I believe it will, but would like to verify. |
…ate flag from package.json as this will need to be available for core
andrewserong
force-pushed
the
try/add-media-fields
branch
from
04e5eb4 to
d2447bc
Compare
|
Good news — I've put together a proof of concept for how a post picker modal could work over in #73710. The way that PR works (please ignore the messy code) is that an What this means is that the post picker modal can be quite decoupled from the fields themselves, which means I think we can safely add all these media-specific fields to this
All of the above is a long way of answering this question — yes, I believe this is a one-off as media is a special case. Other cases (parent, etc) can launch a modal that exists outside of the field itself, so the DataViews will not be a child of the field. That could be something we might want to explore with the media modal, too, depending on API constraints. But that's for another issue / PR if we want to go down that avenue. Alrighty, I think this should be ready for another round of testing / reviews! I've set the caption and descriptions to not enable sorting. I've been rushing around a bit today, so I hope I haven't missed anything obvious. If so, I can take a look again tomorrow 😄 |
andrewserong
requested review from
ajitbohra,
fabiankaegy and
juanmaguitar
as code owners
ramonjd
left a comment
ramonjd
left a comment
There was a problem hiding this comment.
This is working really well for me, thanks for sticking with it.
The only question I had is around the future of this package. If one day, the circular dependency is resolved, what's the best way to integrate these fields into @wordpress/fields?
Deprecate the package I'm guessing. Seems low risk, just curious.
cc @youknowriad @oandregal if they have follow up thoughts
| * @param decimals The number of decimal places to include in the result. | ||
| * @return The human-readable file size string. | ||
| */ | ||
| function formatFileSize( bytes: number, decimals = 2 ): string { |
There was a problem hiding this comment.
Can be follow ups. Just wondering if there's any logic in these new functions that might benefit from unit tests.
E.g., handling edge cases in formatFileSize() (NaN, null)
There was a problem hiding this comment.
Absolutely, happy to add coverage either here or in a follow-up 👍
| ) { | ||
| return ( | ||
| <div className="dataviews-media-field__media-thumbnail"> | ||
| <img |
There was a problem hiding this comment.
Idea for follow up. Loading state/error fallback? Hard to tell from the story. Maybe for later when we start using these in larger data views
There was a problem hiding this comment.
Good idea — yeah I think there'll be lots of little nuances like that to tease apart in usage
Good question. I don't think they'd necessarily ever need to be in In terms of removing the circular dependency, I'm wondering if the new media library could live in a separate package one day, and be invoked from fields via a simple I'll leave this PR open for a day or two in case there's any additional feedback. If not, I'll land it on Monday by the latest and it can be used as a foundation for some of the other work happening for media, like the media editor idea in #72734 |
| import type { MediaItem } from '../types'; | ||
| import MediaThumbnailView from './view'; | ||
|
|
||
| const mediaThumbnailField: Partial< Field< MediaItem > > = { |
There was a problem hiding this comment.
Would be nice if we also allowed setting thumbnails for audio and video files. At some point in the future 😄
There was a problem hiding this comment.
Yes, we should totally do that in a follow-up 👍
|
Thanks for the reviews, folks! This is a pretty big PR so I'll merge it in now, and we can continue on with follow-ups next week. Cheers! |
What?
Part of: #72612
This PR adds a private
media-fieldspackage with media specific fields for use with a media library or media modal. It also imports these fields into the new media library modal experiment. This PR is a collaborative effort and is the result of some trial/error/hacking with a few other contributors.Adds the following media fields and a Storybook Story to test them out:
Note that this PR does not cover
attached_toas that'll be a little more complex to implement, so can be looked at separately.Decisions made
This PR initially added the fields to the
fieldspackage. However in actual usage, there are fields likefeatured_imagethat will need to make use of the media modal. In order to avoid circular dependencies, and to simplify things for now, I've added a dedicated (yet private)media-fieldspackage to avoid the dependency issue. This allows us to make use of these fields now, while offering the flexibility to move the fields around further down the track.Why?
As described in #72612, in order to properly support displaying and editing media / attachments, we'll need some fields for the commonly displayed (or edited) properties of attachments. This PR seeks to do that, and it'll help unlock subsequent work on editing media items in Gutenberg.
How?
media-fieldspackage.Testing Instructions
npm run storybook:dev) and navigate to the stories for these fields:Try out a few of the different layouts to see how it all looks. I.e. in DataViews, table, list, and grid.
Next up, try out the media modal:
Screenshots or screencast
Storybook
DataForm
DataViews table
DataViews list
DataViews grid
Media Library in post / site editors
2025-11-14.14.42.26.mp4