Skip to content
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.

Sign up for GitHub

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 errors after failed upload #39178

Merged
merged 4 commits into from
Mar 8, 2022
Merged

Image: Display errors after failed upload #39178

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b363a3d
Image: Display errors after failed upload
Mamaduka Mar 3, 2022
8eee505
Set isTemp to false on error
Mamaduka Mar 4, 2022
de17283
Only reset temp URL state
Mamaduka Mar 4, 2022
1f26213
Reset attributes on error
Mamaduka Mar 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 2 additions & 6 deletions packages/block-library/src/image/edit.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,7 @@ export function ImageEdit( {
function onUploadError( message ) {
noticeOperations.removeAllNotices();
noticeOperations.createErrorNotice( message );
setTemporaryURL( undefined );
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realized that we only need to reset the state. The onSelectImage will remove attributes for us. It's called during the error but with an undefined media argument.

Copy link
Contributor

@glendaviesnz glendaviesnz Mar 6, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a weird one 🤔 - for me this change causes the php.ini upload limit error to be ignored, but only in the gallery:

img-error.mp4

Seems like there are a couple of gotchas with how gallery versus individual images blocks are behaving with these errors, which would be good to work out why and resolve, or at least document. I will try and get a closer look at it later today.

Copy link
Contributor

@glendaviesnz glendaviesnz Mar 7, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had another look at it, and for some reason it seems like the standalone Image block goes through an extra render cycle which causes image to be removed and error to display, but this doesn't happen if the Image block is in a Gallery, but I can't work out why, as the Gallery essentially just delegates the upload process to the Image block. I will take another look at it tomorrow if you don't have any ideas.

We could just put the setAttributes back, but it would be nice to know why this is happening.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for testing, @glendaviesnz. I will revert setAttributes removal.

}

function onSelectImage( media ) {
Expand Down Expand Up @@ -306,12 +307,7 @@ export function ImageEdit( {
allowedTypes: ALLOWED_MEDIA_TYPES,
onError: ( message ) => {
isTemp = false;
Mamaduka marked this conversation as resolved.
Show resolved Hide resolved
noticeOperations.createErrorNotice( message );
setAttributes( {
src: undefined,
id: undefined,
url: undefined,
} );
onUploadError( message );
},
} );
}
Expand Down