search

wordpress-mobile / WordPress-Android

WordPress for Android
http://android.wordpress.org
GNU General Public License v2.0
2.92k stars 1.3k forks source link

UploadService foreground notification #6713

Closed mzorz closed 6 years ago

mzorz commented 6 years ago

Fixes #6389 (use one foreground notification for the UploadService) and #6546 (have a progress notification for media uploads triggered from the MediaBrowser)

IMPORTANT don't merge until #6735 is complete: This is the basis for all the things listed in https://github.com/wordpress-mobile/WordPress-Android/issues/6735 (Async media success / error notifications master thread) and all other branches will be merged into this one once this is reviewed.

TEST CASE A: single media upload within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/1 media item”.
  4. when the upload finishes, the notification should be gone.
  5. exit the editor
  6. see the foreground progress notification appears again to upload the draft post
  7. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

TEST CASE B: multiple media upload within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 2 media item (do this by using the long press functionality, which turns the media picker into the “multi-select” mode)
  3. see the foreground progress notification starts and remains there for as long as it takes for the media items to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/2 media items”.
  4. when the upload for both items finishes, the notification should be gone.
  5. exit the editor
  6. see the foreground progress notification appears again to upload the draft post
  7. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

You can try this for 3, 4, 5, etc items as well.

TEST CASE C: single media upload, exiting the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/1 media item”.
  4. while the upload is still happening (make sure to choose a large enough file), exit the editor.
  5. Observe the notification is still there, but now reads “Uploading 1/1 media item, 1/1 Post”.
  6. once the media item is finished uploading, see the foreground progress notification changes its text to “Uploading 1/1 Post”.
  7. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

TEST CASE D: multiple media upload, exiting the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 2 media item (do this by using the long press functionality, which turns the media picker into the “multi-select” mode)
  3. see the foreground progress notification starts and remains there for as long as it takes for the media items to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/2 media items”.
  4. while the upload is still happening (make sure to choose 2 large enough files), exit the editor.
  5. Observe the notification is still there, but now reads “Uploading 1/2 media items, 1/1 Post”, and then successively to “Uploading 2/2 media items, 1/1 Post”.
  6. once the media items are finished uploading, see the foreground progress notification changes its text to “Uploading 1/1 Post”.
  7. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

TEST CASE E: single media upload & error, within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/1 media item”.
  4. before the upload finishes, turn airplane mode ON.
  5. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it).
  6. exit the editor
  7. the error notification should remain there, and the Posts list shows the error cause in red for the failed Post.

TEST CASE F: single media upload & error & recover, within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/1 media item”.
  4. before the upload finishes, turn airplane mode ON.
  5. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it).
  6. turn airplane mode off
  7. tap on the image to retry
  8. the foreground notification should come up again, and read “Uploading 1/1 media item”.
  9. when the upload finishes, the notification should be gone.
  10. exit the editor
  11. see the foreground progress notification appears again to upload the draft post
  12. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

TEST CASE G: single media upload & error & recover, outside the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/1 media item”.
  4. exit the editor.
  5. you should see the progress notification there, and now it reads “Uploading 1/1 media item, 1/1 Post”.
  6. before the upload finishes, turn airplane mode ON.
  7. the progress notification should disappear, and instead an error notification is shown. Also, the Posts list shows the error in the corresponding Post item in the list, in red.
  8. tap on such Post to enter the Editor again. Also, dismiss the Error notification.
  9. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it).
  10. turn airplane mode off
  11. tap on the image to retry
  12. the foreground notification should come up again, and read “Uploading 1/1 media item”.
  13. when the upload finishes, the notification should be gone.
  14. exit the editor
  15. see the foreground progress notification appears again to upload the draft post
  16. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

TEST CASE H: multiple media upload & selective error & recover, outside the Editor (fully async)

For this one you’ll need to go through Charles proxy as I want to intentionally abort specific uploads.

  1. go to the Posts list and start a new draft
  2. open the media picker to include several media items
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “Uploading 1/x media item”. (with x being the number of items you included in the Post)
  4. exit the editor.
  5. you should see the progress notification there, and now it reads “Uploading 1/x media item, 1/1 Post”.
  6. before the upload finishes, cancel one of the uploads. You can do so by setting a breakpoint in Charles for the endpoint https://public-api.wordpress.com/rest/v1.1/sites/<site_id>/media/new/
  7. an error notification is shown for that specific item, but the foreground progress notification should continue to show. It should read “Uploading 1/x media item, 1/1 Post”.
  8. wait until everything else finishes, and open the Editor on that Post again.
  9. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it).
  10. tap on the image to retry
  11. the foreground notification should come up again, and read “Uploading 1/1 media item”.
  12. when the upload finishes, the notification should be gone.
  13. exit the editor
  14. see the foreground progress notification appears again to upload the draft post
  15. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

TEST CASE I: media browser single media upload

  1. go to the Media Browser
  2. start an upload for a single item
  3. see the notification appears with text “Uploading…” “1/1 media item”, and a new placeholder with a spinner appears in the grid
  4. wait for it to complete
  5. the foreground notification disappears

TEST CASE J: media browser multiple media upload

  1. go to the Media Browser
  2. start an upload for several items
  3. see the notification appears with text “Uploading…” “1/x media item”, with x being the number of items chosen in step 2, and as many placeholders with a spinner appear in the grid
  4. as each one of them completes, see the text change as in “2/x media items”, “3/x media items”, etc. and a successful notification is shown for each new item.
  5. once everything is completed, the foreground notification disappears

TEST CASE K: media browser multiple media upload & selective fail

For this one you’ll need to go through Charles proxy as I want to intentionally abort specific uploads.

  1. go to the Media Browser
  2. start an upload for several items
  3. see the notification appears with text “Uploading…” “1/x media item”, with x being the number of items chosen in step 2, and as many placeholders with a spinner appear in the grid
  4. for a specific one, abort the connection. You should see an error notification appear.
  5. as each one of the remaining ones completes, see the text change as in “2/x media items”, “3/x media items”, etc. No notifications are shown separately for each successfully uploaded item.
  6. once everything is completed, the foreground notification disappears
nbradbury commented 6 years ago

The wording of these notifications could use some work, I think. Since the majority of the time users will be uploading a single post, is it necessary to say "1/1 post"? When uploading two posts, the "Uploading 1/2 post" reads like only half a post is being uploaded. And the phrase "media items" seems kind of geeky (although maybe I'm being nitpicky about that?).

I'm not sure what would work best, but maybe something like this:

"Uploading 1 post and 3 images" "Uploading 2 posts, 1 image and 2 videos" etc.

screenshot_1508272257 screenshot_1508272325 screenshot_1508272340

mzorz commented 6 years ago

The wording of these notifications could use some work,

Good idea! - I'm wondering, what do you think about discussing it through with Design / Editorial once this passes code review?

nbradbury commented 6 years ago

what do you think about discussing it through with Design / Editorial once this passes code review?

Sounds good, assuming you mean talk with them before we merge into develop.

nbradbury commented 6 years ago

When I upload an image from the media browser and then put the device (emulator) in airplane, the notification never changes to one of failure - even after a few minutes it just stays there as though the upload is still happening.

screenshot_1508275992

mzorz commented 6 years ago

When I upload an image from the media browser and then put the device (emulator) in airplane, the notification never changes to one of failure - even after a few minutes it just stays there as though the upload is still happening.

I couldn't repro on the real handset (Google Pixel, Android 8.0) - the foreground notification is dismissed every time 1 or more uploads are happening and I turn airplane mode ON

For the error and success notifications, while we do have one "error" notification for Posts in place in develop (and therefore still here), I'm planning to do the implementation of the new design separately and is tracked in this other issue here: https://github.com/wordpress-mobile/WordPress-Android/issues/6735

mzorz commented 6 years ago

The wording of these notifications could use some work, I think.

what do you think about discussing it through with Design / Editorial once this passes code review?

Sounds good, assuming you mean talk with them before we merge into develop.

I think I'm going to implement the texts as per the designs, i.e.

screen shot 2017-10-18 at 10 41 37 am

1 post, 3 of 12 media files remaining

I know uploading 2 posts simultaneously is mostly a corner case, but it needs be prepared for that too, yet I agree showing 1/2 sounds like a half (I didn't realize until you said so, and now I can't think anything other than a half - lol)

Variants would be: 1 post, 1 page, 3 of 12 media files remaining 2 posts, 1 page, 3 of 12 media files remaining 1 post, 2 pages, 3 of 12 media files remaining

Tracked this to be tackled separately as well in https://github.com/wordpress-mobile/WordPress-Android/issues/6735

Sounds good?

nbradbury commented 6 years ago

I think I'm going to implement the texts as per the designs...Sounds good?

I'm good with that!

nbradbury commented 6 years ago

@mzorz This looks good. Once all the separate feature branches have been merged into one, we should run through the test again just to make sure it's all working, then merge into develop.

mzorz commented 6 years ago

original comment by @daniloercoli

CASE B: remove error Post notification

Media that were attached to the post, and not yet uploaded are in the media library with the failed status. Don't think it's related to this PR, but I think we should remove it from the media library.

This is a bit tricky, we need to keep them in order for Retry to be something we can do, and even before the Retry exists, it is needed so the user can edit the Post and see the items in there as well. If what makes noise for you is the fact that they are shown in the Media Browser, maybe we can do some check in there (MediaBrowser) and only show those failed (and only for the case of failed) MediaModels for which getLocalPostId == 0, thus effectively only ever letting the user retry images attached to a Post from wihtin a Post, but making them available in the Media Library as usual once they're uploaded successfully. Sounds good?

mzorz commented 6 years ago

original report here by @daniloercoli

I was testing this PR and found the following case.

  • start a draft
  • include several media items in the Post
  • exit the editor
  • you should see the progress notification read “1 post, xxx media files remaining"
  • Close the app from the system manager
  • You will see double notification. One with the error and one with the ongoing uploading

image

That's an interesting one, and I think I know why that happens. Will tackle this one here 👍

mzorz commented 6 years ago

Revisited test cases

With all of the feature implementation in #6735 done by now, I re-wrote the test cases here to make sure they include the new functionality and behavior such as:

  1. WRITE POST for newly uploaded media files
  2. RETRY, both on notifications and on the Post list
  3. showing success notifications only when the app is in the background

Marking [FAILED] on those cases that I didn't manage to make work. Will work on these right next.

TEST CASE A: single media upload within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “1 media file remaining”.
  4. when the upload finishes, the notification should be gone.
  5. exit the editor
  6. see the foreground progress notification appears again to upload the draft post
  7. once the draft post is finished uploading, the foreground notification is dismissed automatically.

TEST CASE B: multiple media upload within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker and include 2 media items
  3. see the foreground progress notification starts and remains there for as long as it takes for the media items to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “2 of 2 media files remaining”.
  4. when the upload for both items finishes, the notification should be gone.
  5. exit the editor
  6. see the foreground progress notification appears again to upload the draft post
  7. once the draft post is finished uploading, the foreground notification is dismissed automatically.

You can try this for 3, 4, 5, etc items as well.

TEST CASE C.1: single media upload, exiting the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “1 media file remaining”.
  4. while the upload is still happening (make sure to choose a large enough file), exit the editor.
  5. Observe the notification is still there, but now reads “Uploading…” and “1 post, 1 media file remaining”.
  6. once the draft post is finished uploading, the foreground notification is dismissed automatically, and a snackbar appears in the Posts list screen which reads “Draft saved online - PUBLISH”.
  7. if you tap on PUBLISH, the post item greys out as it uploads (to prevent editing while uploading), a new progress notification is shown, and finally a new snackbar is shown that reads “Post published - VIEW”. Tapping on VIEW takes you to the site’s preview screen for the Post.

TEST CASE C.2: single media upload, exiting the Editor, app in the background

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “1 media file remaining”.
  4. while the upload is still happening (make sure to choose a large enough file), exit the editor.
  5. Observe the notification is still there, but now reads “Uploading…” and “1 post, 1 media file remaining”.
  6. tap the HOME button to send the app to the background. Observe the progress notification is still there.
  7. once the draft post is finished uploading, the foreground notification is dismissed automatically, and a success notification appears, which reads: “ draft uploaded”, and subtext the name of the site. If you draft the lower edge of the notification down, the quick action “PUBLISH” is discovered. Tapping on it will trigger a new progress notification for a brief amount of time, and then again one another final success notification that reads “ Post published”, subtext name of site. The quick action here is now “SHARE”. Tap on SHARE to share the Post link to other apps.

TEST CASE D.1: multiple media upload, exiting the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 2 media items
  3. see the foreground progress notification starts and remains there for as long as it takes for the media items to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “2 of 2 media files remaining”.
  4. while the upload is still happening (make sure to choose 2 large enough files), exit the editor.
  5. Observe the notification is still there, but now reads “1 post, 2 of 2 media files remaining”, and then successively to “1 post, 1 of 2 media files remaining”.
  6. once the draft post is finished uploading, the foreground notification is dismissed automatically and the snackbar is shown “Draft saved online - PUBLISH”.
  7. Same things should happen as in step 7 in test case C.1.

TEST CASE D.2: multiple media upload, exiting the Editor, app in the background

  1. go to the Posts list and start a new draft
  2. open the media picker to include 2 media items
  3. see the foreground progress notification starts and remains there for as long as it takes for the media items to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “2 of 2 media files remaining”.
  4. while the upload is still happening (make sure to choose 2 large enough files), exit the editor.
  5. tap the HOME button to send the app to the background. Observe the progress notification is still there.
  6. Observe the notification is still there, but now reads “1 post, 2 of 2 media files remaining”, and then successively to “1 post, 1 of 2 media files remaining”.
  7. Same things should happen as in step 7 in test case C.2 (i.e. the success notification should be shown).

TEST CASE E: single media upload & error, within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “1 media file remaining”.
  4. before the upload finishes, turn airplane mode ON.
  5. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it). The error notification reads: “Upload failed for ”. Subtitle: “1 media file not uploaded”.
  6. exit the editor
  7. the error notification should remain there, and the Posts list shows the error cause in red for the failed Post, along with the “RETRY” button.

[FAILED] TEST CASE E.1: RETRY within the Editor

  1. Repeat test case E.
  2. turn airplane mode OFF.
  3. on the Error notification, tap the RETRY quick action.
  4. The progress notification should be shown again, and this time the process should end successfully. If you sent the app to the background before retrying (or at least before the retried upload finishes), the success notification should be shown as well.

TEST CASE F: single media upload & error & recover, within the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “1 media file remaining”.
  4. before the upload finishes, turn airplane mode ON.
  5. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it). The error notification reads: “Upload failed for ”. Subtitle: “1 media file not uploaded”.
  6. turn airplane mode off
  7. tap on the image to retry
  8. the foreground notification should come up again, and read “1 media file remaining”.
  9. when the upload finishes, the notification should be gone.
  10. exit the editor
  11. see the foreground progress notification appears again to upload the draft post
  12. once the draft post is finished uploading, the foreground notification is dismissed automatically and the snackbar is shown “Draft saved online - PUBLISH”.

TEST CASE G: single media upload & error & recover, outside the Editor

  1. go to the Posts list and start a new draft
  2. open the media picker to include 1 media item
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading media…” and subtext “1 media file remaining”.
  4. exit the editor.
  5. you should see the progress notification there, and now it reads “Uploading…” and “1 post, 1 media file remaining”.
  6. before the upload finishes, turn airplane mode ON.
  7. the progress notification should disappear, and instead an error notification is shown “1 post, 1 media file not uploaded”. Also, the Posts list shows the error in the corresponding Post item in the list, in red.
  8. tap on such Post to enter the Editor again. Also, dismiss the Error notification.
  9. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it).
  10. turn airplane mode off
  11. tap on the image to retry
  12. the foreground notification should come up again, and read “Uploading media…” and “1 media file remaining”.
  13. when the upload finishes, the progress notification should be gone.
  14. exit the editor
  15. see the foreground progress notification appears again to upload the draft post
  16. once the draft post is finished uploading, the foreground notification is dismissed automatically and the snackbar is shown “Draft saved online - PUBLISH”.

You can also repeat this test case and exit the editor and send the app to the background after tapping on the retry overlay (step 11), to see the success notification come up as in test case C.2, step 7.

[FAILED] TEST CASE H: multiple media upload & selective error & recover, outside the Editor (fully async)

For this one you’ll need to go through Charles proxy as I want to intentionally abort specific uploads.

  1. go to the Posts list and start a new draft
  2. open the media picker to include several media items
  3. see the foreground progress notification starts and remains there for as long as it takes for the media item to upload. If you drag the drawer down, you should see the Notification shows title “Uploading…” and subtext “x of x media files remaining”. (with x being the number of items you included in the Post)
  4. exit the editor.
  5. you should see the progress notification there, and now it reads “1 post, x of x media files remaining”.
  6. before the upload finishes, cancel one of the uploads. You can do so by setting a breakpoint in Charles for the endpoint https://public-api.wordpress.com/rest/v1.1/sites/<site_id>/media/new/
  7. an error notification is shown for that specific item, but the foreground progress notification should continue to show. It should read “Uploading 1/x media item, 1/1 Post”.
  8. wait until everything else finishes, and open the Editor on that Post again.
  9. you should see the media item failed overlay and retry icon on the image, the foreground progress notification disappears, and an error notification appears (this last one remains there until user dismisses it).
  10. tap on the image to retry
  11. the foreground notification should come up again, and read “Uploading 1/1 media item”.
  12. when the upload finishes, the notification should be gone.
  13. exit the editor
  14. see the foreground progress notification appears again to upload the draft post
  15. once the draft post is finished uploading, the foreground notification is dismissed automatically and a final, successful notification is shown, along with the snackbar.

TEST CASE I: media browser single media upload

  1. go to the Media Browser
  2. start an upload for a single item
  3. see the notification appears with text “Uploading…” “1 media file remaining”, and a new placeholder with a spinner appears in the grid
  4. wait for it to complete
  5. the foreground notification disappears

TEST CASE J: media browser multiple media upload

  1. go to the Media Browser
  2. start an upload for several items
  3. see the notification appears with text “Uploading…” “x of x media files remaining”, with x being the number of items chosen in step 2, and as many placeholders with a spinner appear in the grid
  4. as each one of them completes, see the text change as in “x-1 of x media files remaining”, “x-2 of x media files remaining”, etc.
  5. once everything is completed, the foreground notification disappears.

[FAILING] TEST CASE K: media browser multiple media upload & selective fail

[TODO - not able to test this due to an issue with including media from device] For this one you’ll need to go through Charles proxy as I want to intentionally abort specific uploads.

  1. go to the Media Browser
  2. start an upload for several items
  3. see the notification appears with text “Uploading…” “1/x media item”, with x being the number of items chosen in step 2, and as many placeholders with a spinner appear in the grid
  4. for a specific one, abort the connection. You should see an error notification appear.
  5. as each one of the remaining ones completes, see the text change as in “2/x media items”, “3/x media items”, etc. No notifications are shown separately for each successfully uploaded item.
  6. once everything is completed, the foreground notification disappears

TEST CASE L: WRITE POST functionality

  1. go to the Media Browser
  2. start an upload for several items
  3. see the notification appears with text “Uploading…” “x of x media files remaining”, with x being the number of items chosen in step 2, and as many placeholders with a spinner appear in the grid
  4. as each one of them completes, see the text change as in “x-1 of x media files remaining”, “x-2 of x media files remaining”, etc.
  5. Tap the HOME button to send the app to the background
  6. wait until every item is uploaded, you should see a success notification appear. This notification also has the quick action WRITE POST.
  7. Tap on WRITE POST, the Editor should appear, with a pre-populated draft post that has the media files you just uploaded in it.

SUMMARY of failed cases:

mzorz commented 6 years ago
mzorz commented 6 years ago

TEST CASE K: media browser multiple media upload & selective fail unable to test due to adding pictures in the emulator not working alright for now. Will work on it later.

Works alright after merging #6827

mzorz commented 6 years ago

@nbradbury once #6842 is merged, all tests described here are passing alright for me. With your OK, we'll request the design / editor review. I can put together some videos showing the feature as well 👍

nbradbury commented 6 years ago

@mzorz This is ready for design/editor review 🎉

mzorz commented 6 years ago

Video captures for design/copy review:

Posts list view:

Part 1: normal publishing, async https://cloudup.com/c9OZ33tBaZ5

Part 2: same as before, but sending the app to the background (to trigger the success notification, tapping PUBLISH on it) https://cloudup.com/c2C02ARkej5

Part 3: same as before, but now with a focus on snackbars (tapping on PUBLISH and VIEW) https://cloudup.com/czp8saStqO9

Part 4: starts a draft with media, goes back to Posts list, error happens: see both the error notification and the snackbar below. https://cloudup.com/cGPrTDxx1Ox

Part 4-1: Error in the Posts list, and tapping retry on the posts list itself https://cloudup.com/ccamodcKhkf

Part 5: Error notification, tapping on retry on the notification. https://cloudup.com/cxLeJoe6Qej

Part 6: error snackbar and RETRY on the snackbar (here I have to tap on RETRY many times to be able to test tapping on it, as I don’t want the snackbar to disappear before the connection is reestablished). https://cloudup.com/ckN8k03mP64

Media browser view:

Part 7: media browser normal upload and WRITE POST, snackbar https://cloudup.com/cBrezBAHs-j

Part 8: media browser background upload and WRITE POST, notification https://cloudup.com/czu64wXKu-g

Part 9: media browser & error while uploading https://cloudup.com/csT0bc-ircY

cc @folletto

folletto commented 6 years ago

Video captures for design/copy review

Awesome, and massive thanks for doing this extensive visual record! :)

Design wise, the notifications are good to go for me 👍

mzorz commented 6 years ago

Thank you @folletto ! 🙇 💯

Waiting for Editor copy review now 👍

mzorz commented 6 years ago

Waiting for Editor copy review now 👍

cc'ing @michelleweber Check this comment to see the videos showing all texts - let me know if you prefer a text dump / screenshots

Thanks in advance! 🙇

michelleweber commented 6 years ago

Whoa, I posted a bunch of notes yesterday afternoon; where'd they go? I'll reconstruct them.

mzorz commented 6 years ago

Whoa, I posted a bunch of notes yesterday afternoon; where'd they go? I'll reconstruct them.

oh wow! 😱 thank you!

michelleweber commented 6 years ago

Okay, I think these were all the points/suggestions I wanted to share!

Question: do we have to say "media files"? Can we not just say "files"? I agree that "media files" sounds a little nerdy/jargony, but the clearer options, like "photo/video files" are clunky (and probably not inclusive enough). It seems like plain old "files" would work here. That'd mean you'd also have to update "Uploading..." messages to just say "files."

In the media library, can the header say "4 files selected" instead of just "4 selected"? The addition of the word "files" here helps make it super clear what the other notifications refer to when they say things like "1 of 5 files remaining." (You can never be too clear!)

In messaging like "1 post, 1 of 5 media files remaining," I wonder whether it would be clearer to change it to "1 post with 1 or 5 files remaining" or "1 post saved, and 4 of 5 files uploaded." Making it more of a sentence makes it a little easier to read quickly.

In error messaging like "1 post, 3 media files not uploaded" -- does this mean that nothing at all was uploaded, or that just the files weren't uploaded? Ie, will there be an empty post draft, or nothing at all? Assuming there's nothing at all, changing to "1 post with X files" will be clearer here as well. If there's a post draft, it should be something like "3 files were not uploaded / Post draft saved" (on two separate lines).

In error messaging like "1 post, 1 media file not uploaded (5 files successfully uploaded)" it's a little overwhelming to have all that info on one line -- the length and all the punctuation make it not quickly scannable. I'd take the second half of the message out of parentheses and drop it to another line.

Also, in this case, I have the same question re: post and file uploads. Will there be a draft post saved? If not, where did the five uploaded files go?

mzorz commented 6 years ago

Thank you for your feedback @michelleweber ! 🙇

Question: do we have to say "media files"? Can we not just say "files"?

Totally, addressing that in d74d12c

In the media library, can the header say "4 files selected" instead of just "4 selected"?

Oh, I'm afraid that's how the platform handles it - it is that way for all multi-selection contexts (be it a grid view, a list of things, etc.) either within or out of the app.

In messaging like "1 post, 1 of 5 media files remaining," I wonder whether it would be clearer to change it to "1 post with 1 or 5 files remaining" or "1 post saved, and 4 of 5 files uploaded." Making it more of a sentence makes it a little easier to read quickly.

I see your point! There's a possibility here (not super frequent though) that the user would try and upload more than 1 post at the same time, and we would have to make a a distinction between which posts's has which media files on the notiication, making it too long and would probably be capped. For example, following this structure you could potentially have "1 post with 1 of 5 files remaining, 1 post with 3 of 4 files remaining". This would most probably be ellipsized and unclear; that's why the more generic "2 posts, 1 of 9 files remaining" seemed to fit better. Also, if we used several lines in this case, the user would have to drag the notification down from the bottom edge to expand it, so enough information would never be available at a glance, but rather the user would have to expand the notification to see details about what's going on. But I think we could do with the addition of "and" right after comma as per the proposed change up here, like "1 post, and 1 of 5 media files remaining" (addressed in f1a3c96).

screen shot 2017-11-13 at 10 29 07 am

Looks good? We could also add further specific information if the user wanted to drag the notification down to open more detail on it (although this requires extra work, we'd be able to do it on another PR).

In error messaging like "1 post, 3 media files not uploaded" -- does this mean that nothing at all was uploaded, or that just the files weren't uploaded? Ie, will there be an empty post draft, or nothing at all? Assuming there's nothing at all, changing to "1 post with X files" will be clearer here as well. If there's a post draft, it should be something like "3 files were not uploaded / Post draft saved" (on two separate lines).

👍 Changed the wording in favor of the "1 post with X files not uploaded" form in

screen shot 2017-11-13 at 10 24 17 am

In error messaging like "1 post, 1 media file not uploaded (5 files successfully uploaded)" it's a little overwhelming to have all that info on one line -- the length and all the punctuation make it not quickly scannable. I'd take the second half of the message out of parentheses and drop it to another line.

Ok I did that but by doing so the second line goes hidden and the user needs to drag the notification down to expand it. This is how it looks:

image ˆˆ(unexpanded)

image ˆ^^ expanded

Are we sure we want it like that? the info in parenthesis makes it be all readable at a glance instead IMO. I'll wait for your confirmation though.

Also, in this case, I have the same question re: post and file uploads. Will there be a draft post saved? If not, where did the five uploaded files go?

I think this is intrinsic: each error notification corresponds to a failed Post (if something went wrong with it in any way). The local draft is always kept, tapping on the notification takes you to the position within the Posts list where this failed Post is, so you can read more / edit / take action about it, and there you can see the successfully uploaded media is there in the draft (as expected).

folletto commented 6 years ago

Are we sure we want it like that? the info in parenthesis makes it be all readable at a glance instead IMO. I'll wait for your confirmation though.

That's ok, it's a detail to confirm that some went through.

mzorz commented 6 years ago

That's ok, it's a detail to confirm that some went through

Cool! implemented @michelleweber's suggestion in e634cae :) Thanks for confirming @folletto

@michelleweber let me know if all is good or anything else needs be addressed first 👍 (and thank you so much!!)

michelleweber commented 6 years ago

Thanks for all the explanation! Super helpful.

On the line with the parenthesis, we can do this to get it all one one line: 1 post with 3 files not uploaded, 1 file successfully uploaded

(Now that there's no comma in the first part of the sentence, you don't need the parens any more to create separation.)

Everything else looks good!

mzorz commented 6 years ago

On the line with the parenthesis, we can do this to get it all one one line: 1 post with 3 files not uploaded, 1 file successfully uploaded

(Now that there's no comma in the first part of the sentence, you don't need the parens any more to create separation.)

addressed in 38c1abd

Everything else looks good!

Thank you @michelleweber !

Removing the needs copy review label now.

@nbradbury do you want to check the latest commits before merging? https://github.com/wordpress-mobile/WordPress-Android/commit/d74d12c359770f86d6e9eea841a149753720bb1a https://github.com/wordpress-mobile/WordPress-Android/commit/f1a3c96fdd723240c3ee5b9c0fdbf6005c06e5b7 https://github.com/wordpress-mobile/WordPress-Android/commit/e634caed346a67d25a64ed11db0a1a69b3035bee and https://github.com/wordpress-mobile/WordPress-Android/pull/6713/commits/38c1abdb37b94547ef14085abb68ffae54823420

🙇 thanks once more!

nbradbury commented 6 years ago

Looks good 🎉 :shipit:

akirk commented 6 years ago

I know this is a huge PR but there is a lot of string concattenation going on which just doesn't work for translation. Translators need to be able to shuffle around words. They have tools that help them to deal with repeated translations, so don't worry about that, especially from a dev standpoint it can be really painful to see the same text repeated with only a slight difference but it's necessary for translators to be able to accommodate the peculiarities (compared to English) of their languages.

mzorz commented 6 years ago

@akirk agreed on all your points. It's a pitty the PR (and I'm sure this has happened before) is already merged by the time you're able to raise the flag here. I think it'd be great to have a label like [Status] needs pre-translator review so these things are checked before the merge actually happens. Or, maybe this could be part of the Editorial review, as after all it's something that's about communication? Thoughts welcome @folletto @akirk @michelleweber

FWIW I'll produce another PR to address the issues raised in your comments @akirk 👍 and will have this in mind next time I write or need to review a PR