-
Notifications
You must be signed in to change notification settings - Fork 4.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Post publish upload media dialog: handle more block types #65122
Post publish upload media dialog: handle more block types #65122
Conversation
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: +1.18 kB (+0.07%) Total Size: 1.78 MB
ℹ️ View Unchanged
|
Flaky tests detected in 874c5f5. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10774199552
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Some initial comments after a quick look, will do some in-depth testing later
packages/editor/src/components/post-publish-panel/media-util.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/media-util.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/media-util.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/media-util.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/media-util.js
Outdated
Show resolved
Hide resolved
packages/editor/src/components/post-publish-panel/media-util.js
Outdated
Show resolved
Hide resolved
Thank you for the review, @swissspidy! 🙌 I believe I addressed all your comments, so please go ahead and take another look when you get a chance 👍 |
I think I'd expect only two images in the list ( |
packages/editor/src/components/post-publish-panel/maybe-upload-media.js
Outdated
Show resolved
Hide resolved
Yes, I agree that would be the most user-friendly approach, in general. That said, I think changes to the UI go somewhat beyond the scope of this PR. Furthermore, I'm not sure if there would be any a11y concerns, since each image can have its own alt text (the demo HTML I provided does this, to verify everything is plumbed correctly), which means it could be important to display them separately. |
Thank you for the review, @swissspidy! 🙌 |
What?
This PR adds new block types to the post publish upload media dialog, so that external images in them are also considered for upload.
As part of this work, it also deduplicates URLs and generates unique filenames before upload.
This PR fixes #64899.
Why?
The post publish upload media dialog suggests uploading external media to the media library, for both performance and reliability reasons. However, it currently only does so for Image blocks, which limits its usefulness.
How?
Custom logic was added to handle
media-text
andcover
blocks, as well as to generate adequate filenames before uploading.Testing Instructions
Post contents
/wp-content/uploads/
)WordPress-logotype-standard.png
andWordPress-logotype-standard-<some UUID>.png
. The second file is expected because we used a different URL (via a query parameter) on the last block, and it's expected to have the UUID for deduplication of the filename.Testing Instructions for Keyboard
N/A, no UI changes.