-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Image: Display upload error notices using snackbars #43767
Conversation
@@ -374,4 +372,4 @@ export function ImageEdit( { | |||
); | |||
} | |||
|
|||
export default withNotices( ImageEdit ); |
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.
We no longer need this HoC.
Size Change: -57 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
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.
Beautiful! Instead of a notice that is likely to get cropped or wrap weirdly when #38990 is a thing, we get this handy snackbar that explains and announces what's up:
This is a substantially better experience, and the amount of red in this PR is beautiful. I'd appreciate a code check but this is great.
Thank you for fixing this 🙏
Do you think we should update other media blocks to use Snackbar notifications for errors? |
If you mean ensure they don't show up in placeholders, then yes. Placeholders are great, but as patterns and templating opportunities have improved, it's become clear how often the placeholders will be shown in narrow/small contexts, making it all the more necessary that critical information be extracted and shown elsewhere, so it doesn't just get hidden by responsive rules. |
Correct. I will try to update the remaining blocks. |
Thanks for fixing this! Agree snackbars are better and |
What?
A follow-up to #43180 (comment).
PR updates image block
onUploadError
method to use snackbar error notices.Why?
The new placeholder state doesn't display Notice UI. Matches behavior of Site Logo and Post Feature Image blocks, which also use alternative placeholder states.
Testing Instructions
touch empty.png
Screenshots or screencast
CleanShot.2022-09-01.at.14.06.21.mp4