Media Upload Modal: Try an uploading state with popover in the footer#76228
Conversation
|
Size Change: +2.62 kB (+0.03%) Total Size: 8.75 MB
ℹ️ View Unchanged
|
|
Flaky tests detected in a38f60c. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/23173804359
|
46368dc to
64333a4
Compare
fcoveram
left a comment
There was a problem hiding this comment.
Nice. The interaction works great. I just noticed a few things.
review.media.upload.modal.mp4
- When uploading files the whole area overlaps the checkbox.
- The button seems to include the spinner. But in the design, the button is only for the label and the chevron icon.
- The button used is the default, medium in size, with the icon on the right. Here is the storybook reference I'm using
|
Thanks for taking a look @fcoveram! I've tweaked things a bit, please let me know if I've gotten some assumptions wrong:
Here's how that's all looking:
One question I have is to do with how to handle all of these footer elements on mobile. Currently it's pretty cramped there!
Note that last screenshot is the same on trunk, but now we're looking at the footer, it feels like it could use some love! In terms of status of this PR: I've fixed some other reliability issues of the uploading logic, but I'll leave it as a draft until we're happy with the design. |
|
Nice. It looks better now. I noticed three spacing issues that happen in both image and gallery blocks. : Horizontal misalignment.Before and after uploading the media files, the row looks horizontally misaligned.
Spacing at the bottom of the componentThis is something that I don't see in the storybook's component.
A screenshot from storybook.
Waggling UIThe new element waggle when changes from/to the previous UI. This might have to do with the first point. CleanShot.2026-03-11.at.10.00.35.mp4Regarding the UI on small breakpoints, I think we need to address this at the component level. I can work on one idea and open a new issue to address it separately. What do you think of this? |
3bb0b37 to
055d89e
Compare
|
I think this is getting close to a final shape now, so switching over to ready for review 🙂 |
|
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. |
|
I just gave this a quick test run and it's looking pretty good! Only one odd thing I noticed about the upload state: if you open the popover, once things have finished uploading its button says "upload complete" but the spinner is still there which is a bit confusing:
Other thing is we're still not showing uploaded images if the upload is done while we're on a page other than the first. I'm not sure if that was intended to be addressed in this PR or not? Regarding the possibility of using placeholders, would the logic being added in infinite scroll PR be any help/inspiration for that? |
I was just coming to say this! Also the popover header says “Uploading” when all are complete too. Maybe it could be dynamic, e.g.:
|
| onDismissError?.( file.id ) | ||
| } | ||
| > | ||
| { file.name }: { file.error } |
There was a problem hiding this comment.
I'm not a fan of truncating text as some media files tend to have similar titles and seeing the whole is crucial to identifying. I'm thinking of camera-generated names like (IMG_001-AB0050121, IMG_001-AB0050122, IMG_001-AB0050123, etc)
There was a problem hiding this comment.
Good points both of you. I think in a follow-up we could play around with how we style this. I.e. possibly a line-break between the filename and the error message, or subtle styling of the filename to make it look slightly different to the message.
fcoveram
left a comment
There was a problem hiding this comment.
Just confirming, was this about the misaligned vertical padding, or something else in that screenshot? (I've fixed up the padding)
It was about the alignment of elements on the horizontal line, but now it looks good in your screenshot.
Looks like the assumptions we made in previous PRs was to go for consistent spacing at the bottom and sides for the Select button, which looks off compared to Storybook. I've updated to match how the Storybook example looks.
My approach to spacing has two sides: the visual white balance and following what’s currently in the storybook. In the screenshot the change looks better
I've added it back in, which fixes the waggling, but has the downside that the spinner is present if the panel it open when an upload completes
Yeah, this looks odd. I would go back to the previous version where popover closes and spinner disappears once the upload is done.
The footer looks good in your screenshot, so I will continue with that until I open a new issue to address the change at the component level.
Thanks for all the great work Andrew
|
Thanks for the reviews, everyone! Here's the latest round of changes:
Thank you for flagging! I'd tweaked when
Yes, that placeholder idea is good inspiration. After this PR (and the infinite scroll one) I think that'll be a good thing to try out and see how it feels. I previously had a go at it back in #74838 and found it hard to get things feeling smooth enough when an upload finishes and before the invalidation / reload comes in. But it'll be good to play around with it again once we've got these other pieces in place.
Sounds good. Thanks again for all the reviews and feedback, I think this should be ready for another look now. Let me know if there's any other tweaks / points of friction that need addressing 🙂 |
ramonjd
left a comment
There was a problem hiding this comment.
This PR is testing well, even on slow connections.
Thanks for the changes! Dynamic labels LGTM.
I like how the upload state is persistent, so if I close the modal during an upload sequence the status isn't lost when I reopen the modal.
I'll give a ✅ from my end, but once @fcoveram is good with it, then ![]()
packages/media-utils/src/components/media-upload-modal/upload-status-popover.tsx
Show resolved
Hide resolved
|
|
||
| return ( | ||
| <div className="media-upload-modal__upload-status"> | ||
| { isUploading && <Spinner /> } |
There was a problem hiding this comment.
Nothing to do here, just makes me wonder if there could be any pre-calculation using File API to get file size on select and navigator.connection.effectiveType to warn users about slow uploading.
Maybe a nothing burger. I was testing uploading 6mb images using 3g :)
There was a problem hiding this comment.
I'll make a note for follow-ups after this one lands, good idea to explore!
| export function useUploadStatus( { | ||
| onBatchComplete, | ||
| }: UseUploadStatusOptions = {} ): UseUploadStatusReturn { | ||
| const [ uploadingFiles, setUploadingFiles ] = useState< UploadingFile[] >( |
There was a problem hiding this comment.
Also nothing to do, just commenting that there's an xhr progress event, but I can't see where we use it in the block editor or anywhere. Curious if it's even useful to show a progress bar during upload.
There was a problem hiding this comment.
Curious if it's even useful to show a progress bar during upload.
I'd love to explore that in follow-ups if we can. Especially for larger files it'd be cool to be able to show it in the popover somewhere. Sounds like a fun one to hack on after this lands 👍
| () => [ | ||
| { | ||
| id: 'select', | ||
| label: multiple ? __( 'Select' ) : __( 'Select' ), |
fcoveram
left a comment
There was a problem hiding this comment.
It looks great ✨
Just a minor visual bug I noticed before and forgot to add. Before uploading the files, the footer's top border looks correct, but then, during the upload, there is a white area covering it. Watch the recording below.
CleanShot.2026-03-16.at.17.00.59.mp4
| * This hook provides a middle ground: it iterates over every cached | ||
| * resolution for `getEntityRecords` and invalidates only the entries where | ||
| * the first two arguments match `['postType', 'attachment']`. | ||
| */ |
There was a problem hiding this comment.
Yeah this makes sense, and it works as expected in testing! Uploading 3 items when on page 2 shifts the whole list forward by 3 and when navigating back to page 1 shows the new items at the top.
… invalidate once all uploads have finished to reduce flickering
…d of in the registerBatch function
… bottom, so that the solid background color does not overlap borders, while alignment is preserved
05f88ee to
a38f60c
Compare
Thanks so much for mentioning! I'd had my browser window fairly small vertically so always had a grid of items adjacent to the border so didn't see it. I've pushed a tiny update in a38f60c that sets the upload status to be
I think this is all looking good now, so I'll aim to merge this in later today if there aren't any objections. Thanks again for all the back and forth, everyone! |
|
Alrighty, I've opened up a follow-up issue to capture ideas / thoughts mentioned thus far. Feel free to drop any comments there if there's anything I missed: I've made that a sub-issue of the media interaction in a modal issue (#73085) to try to keep that parent issue for the main features we haven't implemented yet. I'll be traveling for much of the rest of the month, but plan to continue on with media modal features in April. |

















Related to:
What?
This PR tries out an idea from #73085 (comment) of an uploading status button in the footer of the experimental medial upload modal.
It displays a spinner and uploading status. If you click on the button, you'll see a popover of all the files being uploaded.
If the popover is open, it'll stay open even after uploads have completed until you click out to close it.
If an error occurs during an upload, the popover will auto open to show the error state.
Why?
Right now (and to satisfy #74837) we use a snackbar notice when uploads begin, which feels a bit unresponsive and doesn't properly indicate "something is in progress".
Additionally, the idea of using transient/temporary items within the grid itself to flag in progress uploads is quite tricky as we don't necessarily know which level of pagination someone is on, so injecting a loading grid item isn't necessarily going to be reliable.
Also, as in other web apps like Google Drive, sometimes folks will want more granular data about the upload, its progress, and a place to pause or retry uploads (eventually). So even if we do add transient items in the grid to indicate uploading status, I think a footer status area here is going to be useful.
How?
This PR is still a work in progress, and I'm keen to hear feedback on how it feels / whether it's a decent direction. For now it's implemented by:
DataViewsPicker— this allows us to inject the footer upload status when we need toupload-mediabut implementing them locally to the media modal for now, as this needs to work for server-side uploads, not just the client-side media process)Note: I did have to refactor things a bit to get this working, so very happy for feedback on the approach, and if this looks dodgy.
If you'd like to hard-code the popover state so that you can see how it works without uploading lots of files, you can use the following snippet somewhere around line 58 of the
media-upload-modal/use-upload-status.tsfile. Basically, set the default of the setUploadingFiles useState to the following instead of an empty array:Testing Instructions
Enable the new media modal experiment:
Screenshots or screencast
2026-03-06.13.04.16.mp4