Post publish upload media dialog: fix silent failure #64741
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
sgomes
added
[Type] Bug
|
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: +25 B (0%) Total Size: 1.77 MB
ℹ️ View Unchanged
|
|
Thanks, @sgomes! This only partially solves the issue. If fetch fails with a Example blocks
ScreencastCleanShot.2024-08-23.at.16.46.10.mp4 |
|
Thank you for taking a look, @Mamaduka! 🙌
Yes, we should fix that as well 👍 But to clarify, this PR is intended to fix a bug in the updating of the blocks, assuming that the upload works correctly. What do you think would be the best way to handle errors in the upload process? Displaying an error message and changing the button text to "Retry"? Also, would it be best to include those changes in this PR, or to open a new one for them? |
I think that should work. @jameskoster or @jasmussen might have better suggestions :)
Yeah, it can be done in follow-up PR. Don't want to block this. I think we could fix one minor thing here. If you watch the screencast closely, you'll see that the "Upload" button becomes active for a second before removing the uploaded image from the list. This can result in duplicate uploads on some devices if users click it again. Maybe we should optimistically remove uploaded images. What do you think? |
Perhaps the simplest way to start is to allow the "Upload" button to remai as-is, but indeed output, to the right of it, or below it, "Upload failed, try again." |
|
Thank you again, @Mamaduka! I added some code to wait until the animation ends before showing the button again: Wait.for.animation.movI'll follow up with the UI changes for handling upload errors in a new PR. |
sgomes
force-pushed
the
fix/upload-media-dialog-silently-fails
branch
from
105a9d6
to
aa2188e
Compare
|
I started getting a new error when testing this, in a portion of the code that hasn't changed. Please hold off on re-testing while I investigate. |
|
False alarm, the error was due to a local issue in my machine. Please take another look when you get a chance! |
Mamaduka
approved these changes
Aug 26, 2024
|
Thank you again for the review, @Mamaduka! 👍 @jasmussen : Thank you for the suggestion, I'll open a new PR 👍 |
This was referenced Aug 27, 2024
bph
pushed a commit
to bph/gutenberg
that referenced
this pull request
Aug 31, 2024
* Post publish upload media dialog: fix silent failure * Wait until animation ends before activating button
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?
Attempt to fix intermittent silent failures in the post publish upload media dialog.
Why?
The upload process sometimes silently fails when using the upload media dialog:
Upload.fails.mov
The files do get uploaded, but the blocks never get their attributes updated, causing the upload suggestion to remain.
When debugging this, I noticed that the
clientIds that are being updated don't seem to exist in the block editor store. My knowledge is extremely limited here, but I believe this may be due to a mismatch inclientIds between the editor store and the block editor store.This PR may fix #63956.
How?
This PR switches from using the editor store
getEditorBlocks()selector to the block editor storegetBlocks()selector. This seems like a reasonable fix, given that theupdateBlockAttributes()action that we use to update blocks belongs to the block editor store, but again, my knowledge is extremely limited here.Testing Instructions
I don't know how to reliably reproduce the failure state 😞 It sometimes happens when using patterns that include image blocks, and which point to theme images (which are considered external images).I figured out how to reproduce the failure state:
https://s.w.org/style/images/about/WordPress-logotype-simplified.png)This should produce a similar failure to the one in the video above before applying this PR. The same sequence of steps should work correctly after the PR.
Testing Instructions for Keyboard
N/A
Screenshots or screencast