Allow dropping multiple images to the image block #65030
Conversation
kevin940726
added
[Type] Enhancement
kevin940726
requested review from
Mamaduka,
noisysocks,
talldan,
swissspidy,
ramonjd and
andrewserong
kevin940726
requested review from
ellatrix and
artemiomorales
as code owners
|
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: +256 B (+0.01%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
| @@ -394,6 +423,7 @@ export function ImageEdit( { | |||
| placeholder={ placeholder } | |||
There was a problem hiding this comment.
The MediaPlaceholder has a multiple prop - is that of any help here?
There was a problem hiding this comment.
I thought about that too. Unfortunately, passing multiple will change the UI of the media library. I think we'll have to refactor the media library to make it not do so when only selecting one image 😅.
There was a problem hiding this comment.
IIRC, the media placeholder should be optimized for multiple file uploads.
There was a problem hiding this comment.
Great work @kevin940726, this is testing well for me!
2024-09-04.15.59.49.mp4
Aside from what the others mentioned, just left a comment about whether we should check that a Gallery block can be inserted.
I added a notice the same as the gallery block and filter the supported files out. The wording can be changed. |
The wording is possibly a little awkward, but I like the idea of reusing the string for now. As you mention it can be changed later! |
|
Flaky tests detected in 48cdb09. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10696496913
|
Can't we just make it work? I find these notices very annoying and unnecessary when we could actually make it work. Here's how I solved it in my project: When you drop e.g. an audio file, image and a video at the same time, it will insert an audio block, image block, and a video block. Anything else could be a file block. |
I think this makes sense when dropping a single file, but probably not when dropping multiple different types. For instance, if a user drops an image, a file, and another image, what should we do in this case? We can certainly create an image block, a file block, and another image block respectively, but I think it's also pretty obvious that the user made a mistake and wants to undo it. 🤔 |
There was a problem hiding this comment.
Nice work @kevin940726 — from my perspective this iteration is in a good place to land. There are a couple of ideas for potential things to improve upon, but I think they're good territory for follow-ups.
Can't we just make it work? I find these notices very annoying and unnecessary when we could actually make it work.
That's an interesting point. On this one I think I lean toward what Kai mentions that it's likely unintentional if folks are dragging directly onto an image block. For dragging onto the editor canvas in general or the paragraph block, then I'd expect it to work differently. But possibly something to play around with separately to this PR? This PR is already a nice improvement over what's in trunk.
In my testing, this one's working really nicely:
✅ Converts the Image block to a Gallery block and handles the upload for multiple images dragged onto the block in a smooth way
✅ Dragging onto an Image block within a Gallery block appends as expected
✅ If a Gallery block cannot be inserted where the Image block is, then the drag fails. This could potentially be improved upon, i.e. with a notice of some kind? Again, I'd possibly leave that for a follow-up if need-be as most of the time a Gallery block will be available. If we think it warrants doing now, though, we could ping for some design feedback there.
Here's how it's looking when dragging onto an Image block placeholder using multiple images, and when the Gallery block is disabled:
2024-09-05.14.46.36.mp4
Overall, this LGTM!
I think we already support dropping arbitrary files onto an empty paragraph block 🎉. |
I should have checked before adding my comment, but yes, we do: 
In practice I notice there that we still get that Gallery error message when dragging onto an empty paragraph block, so @swissspidy has a good point there. But I reckon that would suit a separate PR.
I think that'd make for a good approach for the dragging onto a paragraph block / the general editor canvas. |
+1 Regardless, it addresses the issue exactly and follow ups are always possible 👍🏻 Great work! |
|
I've opened a separate issue to track the suggestion from @swissspidy here: #65074 |
|
I've put up a follow-up PR to look at the idea from @swissspidy over in #65144. Happy for any feedback there, I realized we could take a slightly different approach to what's used in |
What?
Close #64795.
Allow dragging and dropping multiple images to a image block (to convert to a gallery block).
Why?
A UX improvement. See #64795.
How?
The usage of the
mediaUploadutil in<MediaPlaceholder>is a bit counterintuitive. It callsonSelectmultiple times if there are multiple images. Let's say you drop two images from your local file system to the component, theonSelectfires four times with the different ` parameters:[{url: 'blob:...'}]: The first image in blob format (before upload)[{url: 'blob:...'}, {url: 'blob:...'}]: Both images in blob format (before upload)[media1, {url: 'blob:...'}]: The first image is uploaded and replace itself with the uploaded media format (from the media library)[media1, media2]: Both images are uploaded.This gets significantly worse when there are more than two images. We also want to defer the upload to each individual image so that we can display some loading states too.
To avoid breaking compatibility, I adjust the
handleUploadparameter to optionally accept a function. If the function returnsfalsethen it will not upload the images. We can then create the gallery block ourselves to avoid the complexity.Testing Instructions
Testing Instructions for Keyboard
Same as above. I'm not sure how to test dragging and dropping with keyboard though.
Screenshots or screencast
Kapture.2024-09-04.at.13.24.48.mp4