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

Retry failed posts from post list #6789

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Oct 25, 2017

Fixes #6407

Unhides and implements the Retry button in Posts List, by making use of the implementation of UploadService.

screen shot 2017-10-25 at 7 36 04 pm

To test:

  1. start a draft
  2. include several media items in it
  3. as the media started uploading, exit the editor
  4. see the progress notification show something like “1 post, xxxx media files”
  5. turn airplane mode ON
  6. observe the error is shown in the Posts list, in red, and the RETRY button becomes visible on that Post item, along with the snackbar and corresponding notification.
  7. turn airplane mode OFF
  8. once connectivity is recovered, tap on RETRY
  9. see the progress notification appear, the error disappears from the posts list, and the progress is shown.

cc @nbradbury

@mzorz mzorz changed the base branch from mzorz/media-success-notifications-write-post to issue/6389-upload-service-notification October 26, 2017 16:37
@nbradbury nbradbury self-assigned this Oct 26, 2017
@nbradbury
Copy link
Contributor

nbradbury commented Oct 26, 2017

If you run this on a device with a smaller screen, the retry button wraps to the second set of buttons (so it doesn't show up until you tap "More"). Should it be in the first set instead?

screenshot_1509042459

(This screenshot was taken from a Nexus 4 emulator running API 23)

default:
return 0;
}
}

public static int getTextColorResId(int buttonType) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Minor tip: I've started using the @StringRes, @DrawableRes and @ColorRes annotations for these type of routines. ie: public static @ColorRes int getTextColorResId(int buttonType)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Niiice! Added annotations in dd2caaf

@@ -1225,8 +1225,8 @@
<string name="error_media_load">Unable to load media</string>
<string name="error_media_save">Unable to save media</string>
<string name="error_media_canceled">Uploading media were canceled</string>
<string name="error_media_recover_post">We were unable to upload this post\'s media. Please edit the post to try again.</string>
<string name="error_media_recover_page">We were unable to upload this page\'s media. Please edit the page to try again.</string>
<string name="error_media_recover_post">We were unable to upload this post\'s media. Please tap retry or edit the post to try again.</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

This is really wordy, plus it uses the pronoun "we" which Google recommends avoiding. The user should be able to figure out they can edit or retry without us telling them, so how about simply, "Unable to upload this post's media?"

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 amazed about how you are so much into details. Thanks! Made the change as suggested in 8cba571

@mzorz
Copy link
Contributor Author

mzorz commented Oct 26, 2017

If you run this on a device with a smaller screen, the retry button wraps to the second set of buttons (so it doesn't show up until you tap "More"). Should it be in the first set instead?

I did this change in 74fc93a, but could you double check please? not sure if it's going to work correctly with the conditionals there are for showing publish or stats button

@nbradbury
Copy link
Contributor

I did this change in 74fc93a, but could you double check please?

There are quite a few problems here, such as the Publish button disappearing after you hit More > and then < Back. This is a pretty thorny bit of code, so I think we should revert this change and file a separate issue for it.

screenshot_1509054769

@mzorz
Copy link
Contributor Author

mzorz commented Oct 27, 2017

This is a pretty thorny bit of code, so I think we should revert this change and file a separate issue for it

Sounds good, reverted in 0c1b740 and opened new issue here #6793, I'll get on it ASAP (i.e. before anything goes to develop)

@nbradbury
Copy link
Contributor

:shipit:

@nbradbury nbradbury merged commit 524f8c4 into issue/6389-upload-service-notification Oct 27, 2017
@mzorz mzorz deleted the issue/6407-retry-failed-post-from-post-list 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