Prepublish: suggest uploading external images #46014
Merged
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
|
|
|
Size Change: +825 B (0%) Total Size: 1.42 MB
ℹ️ View Unchanged
|
mtias
added
[Type] Enhancement
mtias
reviewed
Nov 23, 2022
ellatrix
commented
Nov 23, 2022
packages/editor/src/components/post-publish-panel/maybe-upload-media.js
Outdated
Show resolved
Hide resolved
mtias
reviewed
Nov 24, 2022
ellatrix
force-pushed
the
try/image-upload-prepublish
branch
from
2a6b3a3
to
ab2e2f0
Compare
mtias
reviewed
Dec 8, 2022
mtias
reviewed
Dec 8, 2022
packages/editor/src/components/post-publish-panel/maybe-upload-media.js
Outdated
Show resolved
Hide resolved
github-actions
bot
added
the
[Status] In Progress
|
Nice one! Haven't yet had a chance to try this: what happens if you press "Publish" before you press "Upload all"? Should there be some warning or should it happen automatically? Like, could it be a checkbox: "upload all images" which then gets batched and part of the publish flow? |
They remain external images?
Why should there be a warning?
Not sure I understand what you mean 😅 |
|
Just thinking through the flow here, and it seems such a nice feature that it might be easy to overlook if a) it isn't given attention, or b) it's automatic. In the case of the latter, I mean there could be a checkbox: "Upload external images". You'd check it (or it would be checked by default), and when you press publish, it first uploads all external images and then publishes. The button would be candy striped for a bit longer, but it could be an integrated part of the flow. Mainly a thought for now. |
|
What happens if there are many external images, and you click Publish before the upload process is complete? Should the loading spinner overlay the entire panel to prevent this? Or perhaps the upload process takes place in the background, kind of like Joen suggested. |
ntsekouras
reviewed
Dec 21, 2022
| @@ -6,7 +6,7 @@ | |||
| // Ensure the post-publish panel accounts for the header and footer height. | |||
| min-height: calc(100% - #{ $header-height + 84px }); | |||
|
|
|||
| .components-spinner { | |||
| > .components-spinner { | |||
There was a problem hiding this comment.
This seems quite specific here and this component is public API. Should we instead style your new Spinner instead? 🤔
There was a problem hiding this comment.
So this is styling another spinner in the same pannel and we want the new spinner to have no extra styling. Changed it here so this extra CSS only affects the existing spinner.
ntsekouras
reviewed
Dec 21, 2022
packages/editor/src/components/post-publish-panel/maybe-upload-media.js
Outdated
Show resolved
Hide resolved
ellatrix
force-pushed
the
try/image-upload-prepublish
branch
from
cf704f9
to
900ccc8
Compare
|
Flaky tests detected in 245fcd082308a2f761eb0ed6957a416680db03b4. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5343957310
|
|
I tested this on 6.2.2 on a site created with InstaWP. It generally worked pretty well and I think this is a good enhancement. I'll note two things:
I noticed that if I had clicked on the image block previously (and seen the upload external image button), then this button still showed up when clicking the image again, even after uploading the images and publishing the page. It seems like if the image block hasn't been previously clicked, then the button does not show up. Sometimes when clicking the upload button, the image works fine. Other times, it appeared to get stuck (with the loading icon) while trying to upload the external image. Refreshing the page solved this. Upload.button.still.shows.up.mov
This should possibly be a separate follow up issue, but I'll note that the option to upload external images only shows up on publish. On an existing, published page, I tried copying in blocks with an external image linked. I then updated the page and did not see the prompt to upload the images. I think the UX of this might be harder, though, because we may not want to prompt someone each time they update a page. No.upload.on.page.update.mov |
|
One other piece of UX feedback -- I wasn't sure what clicking on the image thumbnail would do. It seemed like it would focus on the image on the page. However, clicking on the image again will take the focus off (and clicking again won't add it back). At first, I thought this was a way for me to select only certain images to be uploaded, but then realized it was not. One suggestion would be to remove the click option on the image thumbnails completely. Screen.Recording.2023-06-21.at.3.42.22.PM.mov |
|
@clubkert I fixed the lingering button. Could you file issues for the rest? We're short on time and we need to get this in. |
ellatrix
added
the
[Release] Do Not Punt
Co-authored-by: Timothy Jacobs <timothy@ironbounddesigns.com>
tellthemachines
force-pushed
the
try/image-upload-prepublish
branch
from
245fcd0
to
00df9ec
Compare
|
Thanks for picking this one up again and not letting it fall through the cracks! |
sethrubenstein
pushed a commit
to pewresearch/gutenberg
that referenced
this pull request
Jul 13, 2023
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?
Adds a prepublish panel suggesting to upload external images.
I'm not great at making a good UI, so feel free to tweak. :)
As for the technical bit: I think ideally we need a slot where block types can hook in and add custom logic, but this is a good start.
Why?
Make it more discoverable that you can upload them.
How?
Testing Instructions
Go to the demo content and publish.
Screenshots or screencast