WordPress.org

Make WordPress Core

Opened 19 months ago

Last modified 7 months ago

#49587 new defect (bug)

Add error handling for the media manager Ajax response

Reported by: johnbillion Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 3.5
Component: Media Keywords: needs-patch
Focuses: javascript, administration Cc:

Description

When the media manager modal performs an Ajax request to query attachments, if the Ajax call responds with an error then the user is presented with an eternal loading spinner and no indication there has been a problem.

I think there are two places which need a fail() handler for the Ajax promise:

  • wp.media.model.Attachments.more()
  • wp.media.model.Query.more()

I haven't done any testing yet though to confirm this.

Needs a decision around how best to present an error to a user. Probably needs UI work; maybe one of the UI pieces such as the upload failure message can be reused.

Attachments (3)

media-modal loading.png (17.1 KB) - added by ibdz 13 months ago.
media-modal error.png (19.1 KB) - added by ibdz 13 months ago.
Screenshot 2021-02-04 at 10.04.45.png (78.1 KB) - added by johnbillion 8 months ago.
Screenshot of existing error handler for drag and drop upload

Download all attachments as: .zip

Change History (21)

#1 @joemcgill
19 months ago

  • Milestone changed from Awaiting Review to 5.5

Thanks for the report @johnbillion, let's add this to 5.5 to confirm and get some design feedback on the best user experience for this.

#2 @johnbillion
18 months ago

  • Keywords needs-design added

#3 @johnbillion
18 months ago

  • Keywords needs-ux removed

#4 @estelaris
15 months ago

  • Keywords needs-screenshots added

This ticket was discussed in Design triage, we would like to see the error in a screenshot and see the "upload failure message" that could be used to solve the UI display.

This ticket was mentioned in Slack in #core-media by antpb. View the logs.


15 months ago

#6 @antpb
15 months ago

  • Milestone changed from 5.5 to 5.6

thanks @estelaris! @johnbillion what part of the modal do you see this happening in? I'm happy to get a screenshot of the error but I think the error here is that there is no error message to begin with.

I can force one in the place you're thinking if that would be helpful for the Design team. I think the confusion on my end is just where this is happening in the media modal. I'm not super confident this is a 5.5 task given we are in Beta 1. Going to move this to 5.6. Feel free to change back if that is incorrect.

#7 @johnbillion
15 months ago

The errors above occur when fetching the list of attachments via Ajax, either for the first load or for subsequent loads as part of infinite scrolling.

You can force this by calling wp_send_json_error() in wp_ajax_query_attachments().

You're right that the idea behind this ticket is that there is no error handling at all at the moment, and the user will see an eternally spinning spinner.

#8 @johnbillion
15 months ago

This also applies if there's a network problem and there's no response, or there's a fatal error during the Ajax call, etc.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


13 months ago

#10 @ibdz
13 months ago

  • Keywords needs-design-feedback added; needs-design needs-screenshots removed

I did some design for error message showing in Media Modal. When there's an error, the spinner should be stopped (replaced) and show the error message instead. The message might need more elaborate.

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


13 months ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


12 months ago

This ticket was mentioned in Slack in #design by karmatosed. View the logs.


12 months ago

#14 @hedgefield
12 months ago

  • Keywords needs-design-feedback removed

This looks like a fine solution to me @idbz. It fits nicely in the space where the spinner appears as well, with a clear and concise message. I'm not sure "HTTP error" is all that helpful, the thing is that for the user, the "request failed". The reason is less important, except maybe for troubleshooting if there are more ways in which the request can fail. But that probably appears in the console?

Either way, I'd say this design with "Request failed, try again" as copy is good to move ahead to dev. Is there a dev available who can write this patch?

#15 @johnbillion
12 months ago

  • Milestone changed from 5.6 to Future Release

Thanks for the feedback @ibdz @hedgefield !

At the code level we can pass through whatever error message comes from the failure, so we ought to be able to avoid generic messages such as HTTP error here and possibly append a suffix such as Try again, although depending on the error it might not be possible to try again.

I don't have time to write a patch for this so I'm bumping it for now. If someone else wants to pick it up, great!

@johnbillion
8 months ago

Screenshot of existing error handler for drag and drop upload

#16 @johnbillion
8 months ago

I discovered today there is already UI for an error in this view. If you attempt to drag and drop a file while you're on the Media Library tab and there is an error from the upload, the error message appears in the sidebar. Screenshot above.

We can reuse this existing UI for the error message, we just need to connect it up to the response from the Ajax call for attachment queries.

This ticket was mentioned in PR #1065 on WordPress/wordpress-develop by johnbillion.


7 months ago

  • Keywords has-patch added; needs-patch removed

#18 @johnbillion
7 months ago

  • Keywords needs-patch added; has-patch removed

I started working on this in https://github.com/WordPress/wordpress-develop/pull/1065 but I forgot how difficult it is to figure out the media manager views. If someone wants to pick this up and carry on with it that would be great.

Note: See TracTickets for help on using tickets.