Add an explicit error message if an image block fails to load the image #40982
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
glendaviesnz
added
[Type] Enhancement
|
Size Change: +78 B (0%) Total Size: 1.24 MB
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Great idea, Glen!
This is looking good for me for a single image block, but if I test it in a Gallery block, the screen real estate for the error message isn't very compatible with an error notice. I wasn't sure what the best solution for this would be — if we should conditionally show the notice only if there's enough room?
What do you think?
ah, nice catch @andrewserong, I should have thought to test that
Hmm, not sure, will ponder that. I did think about a snackbar, but decided it was too disconnected from where the block is, particularly if more then one Image block on a page. |
Good point. Maybe we almost need an error notice icon or something in smaller spaces, with the error message itself available in a tooltip when you hover over the notice? I'll leave it your pondering! 😄 |
|
@andrewserong I have reduced the notice text, which was actually way more verbose than it needed to be - it helps a bit, but isn't perfect - will have a look tomorrow if there is any way of adding a link or tooltip in the existing notices. |
|
@andrewserong currently the withNotices HOC does not allow the adding of additional user actions. The createNotice action from the notices store does, but that only adds a notice to the top of the editor which suffers from the same issue as the snackbar it that it disconnects the error from the affected image block. I think with the shorter message, and with the fact that this is probably not a super common problem in galleries, and it is better than no error at all, that we go with this approach for now ... but it would be good to have the ability to pass actions to the Notices HOC at some point, or create a useNotices hook to replace the HOC that allows this ... at which point we could refactor this? |
andrewserong
approved these changes
May 12, 2022
There was a problem hiding this comment.
Thanks for digging in @glendaviesnz, I agree, with the shortened error message that's much more manageable and it's better to have an error message than no notice at all.
Here's how it's looking for me now:
LGTM! ✨
|
@glendaviesnz I reverted these changes as part of #41221 due to the errors reported in #41161 A new PR that encompasses all reverted error handling work, including this PR's, over at #41220 |
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 an error message of an Image block fails to load the attached image
Why?
Currently if an existing Image block points to an image that no longer exists, when it is loaded in the editor it shows an empty placeholder, with no message to inform the user why that is the case
How?
Adds an explicit error notice with details about the url of the image that could not be loaded.
Testing Instructions
Screenshots or screencast
Before:
image-error-before.mp4
After:
image-error-after.mp4