Drag and Drop: When dragging a mix of video, audio, and image blocks, create individual blocks as appropriate #65144
Conversation
This file contains 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
… create individual blocks as appropriate
andrewserong
added
[Type] Enhancement
andrewserong
requested review from
artemiomorales and
ajitbohra
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: +12 B (0%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
ramonjd
reviewed
Sep 9, 2024
There was a problem hiding this comment.
This is working great. I'd like to tentatively approve, but will wait for overnight testers in case I missed something.
I dragged the following files to the canvas
- mp3 and wav files create audio blocks
- mp4 files create video blocks
- pdf/other files creates file blocks
Just one question - what do you think should happen when dragging multiple unsupported files onto the image block? Currently it switches to a gallery block. Should it remain an image block?
2024-09-09.17.19.05.mp4
❤️ |
kevin940726
reviewed
Sep 9, 2024
There was a problem hiding this comment.
This works great! However, I think we might want to look at the implementation in onFilesDrop.
gutenberg/packages/block-editor/src/components/use-on-block-drop/index.js
Lines 176 to 182 in 33b11ac
The transformation only finds one for all files, instead of finding a transformation for each file. I wonder if we could (or should) iterate through each file and apply the transformations. 🤔 This should also check for canInsertBlockType and other conditions in each block's transformation.
You mean in addition to the existing lookup, as a fallback? I think that makes sense from a responsibility perspective. Transformations can happen through other ways as well though, right? Like copy/paste or even manually through the UI. |
|
Thanks for testing, folks! 🙇
Oh, interesting! With a little digging, it looks like with the current approach we'll still wind up being guarded by gutenberg/packages/block-editor/src/store/actions.js Lines 374 to 377 in 9cf3162 Or here for insertions: gutenberg/packages/block-editor/src/store/actions.js Lines 579 to 582 in 9cf3162 So I believe what's currently happening in this PR is that the drop will bail if anything can't be inserted. Were there any other checks that happen in I quite liked the idea (if possible) of keeping the logic within the transformation if we can, as we've already got a bit of complexity happening in the dropping behaviour (especially in the That said, do let me know if you think it still needs moving around, though! |
swissspidy
approved these changes
Sep 10, 2024
andrewserong
deleted the
try/file-block-transforms-for-mix-of-video-audio-and-image-files
branch
mtias
added
the
[Feature] Media
This was referenced Sep 26, 2024
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?
Fixes: #65074, and follow-up to: #65030
Allow dragging a mix of File, Audio, and Image blocks onto the editor canvas to create one of each block as appropriate.
This PR borrows the approach used by @swissspidy in the media-experiments plugin, as suggested in this comment: #65030 (comment)
The approach here differs slightly from that link, as here we can combine the logic with the fallback transform of the File block, since we're not working in a plugin.
Why?
Currently if you attempt to drag a mix of file types, you'll wind up with a File block for each file type. It's likely that a user dragging a mix of media files would expect media blocks to be in use rather than File blocks which are typically used to allow visitors to download the original file.
This PR proposes allowing image, video, and audio blocks to be created where appropriate, while still falling back to File blocks.
How?
Testing Instructions
trunkit'll just create a bunch of File blocksScreenshots or screencast
2024-09-09.16.26.56.mp4