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.

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

Async notifications: media-only error notifications (part III) #6780

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Oct 24, 2017

Builds on top of #6763 (Post error / success notifications), continuing the implementation of the new design for foreground / error / success notifications for async uploads.

screen shot 2017-10-24 at 4 51 07 pm

Notes:

  • Please note that, differently to how Posts error notifications are handled (there is one error/final notification per Post, summarising what went well/wrong with a specific Post each time), the notification that covers errors for standalone media items (that is, media that is uploaded through the Media Browser without being included in a Post) is one notification for all.
  • Observe that the error notification for media items holds information for all failed standalone (i.e. not-Post related) media items.
    For example, if you are uploading one standalone picture (started uploading through the Media Browser), and it fails, the error notification will show not only the one item that just failed but will tell you all standalone media items that have failed for this site. If you tap RETRY on the notification, you are starting uploads for all of them.
    If you prefer to go one by one, you can always pick which to upload from the Media browser, by tapping on the failed items ( “retry” overlay).

To test:

CASE A:

  1. start a media upload from the media browser
  2. see the progress notification appears and starts progressing
  3. turn airplane mode ON
  4. see progress notification disappear and error notification come up

screenshot_20171024-164221

  1. turn airplane mode OFF
  2. tap RETRY on the notification / snackbar
  3. see the progress notification re-appear and the error notification is gone
  4. also see the media browser screen, it should show the same retried item as UPLOADING again.

CASE B:

  1. start a media upload from the media browser
  2. see the progress notification appears and starts progressing
  3. turn airplane mode ON
  4. see progress notification disappear and error notification come up
  5. turn airplane mode OFF
  6. upload yet another file
  7. see both the progress notification for the new file appears, and the error notification for the first file remains.
  8. turn airplane mode ON
  9. see the progress notification go away, and the error notification is updated now, showing 2 files have not been uploaded.
  10. turn airplane mode oFF
  11. tap on RETRY
  12. see the progress notification re-appear, showing that it’s uploading 2 files, and the error notification is gone
  13. also see the media browser screen, it should show the same retried items (2 items) as UPLOADING again.

CASE C: make a post error appear, and make a stand alone media error appear

  1. start a new draft, include some media item, exit the Editor
  2. turn airplane mode ON to trigger the error notification
  3. turn airplane mode off
  4. go to the media browser, and add a new media item
  5. when the media item is uploading, turn airplane mode ON
  6. see both error notifications coexist.

screen shot 2017-10-24 at 5 04 10 pm

_(The image above is the media error notification while the below one is the Post error notification)_
  1. Tap on RETRY on the media-only error notification, see the progress starts

screen shot 2017-10-24 at 5 06 15 pm

  1. Tap on RETRY on the post with media error notification, see the progress info is added to the already existing progress information notification.

CASE D:

  1. upload a new media item and turn airplane mode ON in the middle of its upload.
  2. turn airplane mode off and try uploading a new item, and again make it fail by turning airplane mode ON in the middle of its upload.
  3. you’ll see this second time the error notification is shown, it will read “2 media files not uploaded”, instead of “1 media file not uploaded”..
  4. turn airplane mode OFF and tap on RETRY
  5. wait until everything is uploaded
  6. check the Media Browser and verify your 2 items are uploaded.

cc @nbradbury

@nbradbury nbradbury self-assigned this Oct 25, 2017
@mzorz mzorz changed the base branch from mzorz/post-error-success-notifications to issue/6389-upload-service-notification October 25, 2017 15:11
@nbradbury
Copy link
Contributor

Can we skip the toast when a snackbar will be shown? Showing both a toast and a snackbar seems like overkill.

@@ -288,6 +288,7 @@
<!-- post uploads -->
<string name="upload_failed_param">Upload failed for \"%s\"</string>
<string name="media_files_not_uploaded">%d media files not uploaded</string>
<string name="media_file_not_uploaded">1 media file not uploaded</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't have strong feelings about this so we don't have to change it, but it feels wrong to start these sentences with a numeral. How about "Failed to upload media file" and "Failed to upload %d media files" instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm open to changing it - I think maybe let's do an editor copy in the end, with all the notifications in place and merged into #6713 so it's easier for people to weigh in and compare all cases. Sounds good?

@mzorz
Copy link
Contributor Author

mzorz commented Oct 25, 2017

Can we skip the toast when a snackbar will be shown? Showing both a toast and a snackbar seems like overkill.

removed the Toast in c6eae47

@nbradbury
Copy link
Contributor

:shipit:

@nbradbury nbradbury merged commit cd6b5f8 into issue/6389-upload-service-notification Oct 25, 2017
@mzorz mzorz deleted the mzorz/media-error-success-notifications branch November 6, 2017 19:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants