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 media: adding publishing Snackbars to MySiteFragment #6425

Merged
merged 5 commits into from
Jul 31, 2017

Conversation

mzorz
Copy link
Contributor

@mzorz mzorz commented Jul 28, 2017

Extends the functionality implemented in #6233 to the Main screen (My sites) where the floating edit button also appears.

To test: follow the instructions in all cases in #6233

posting

cc @aforcier

@aforcier aforcier mentioned this pull request Jul 28, 2017
12 tasks
@aforcier aforcier added the [Status] Needs Design Review A designer needs to sign off on the implemented design. label Jul 31, 2017
@aforcier
Copy link
Contributor

cc @iamthomasbishop

@iamthomasbishop
Copy link

Looks great! 👍

}

public static void showSnackbar(Activity activity, int messageRes, int buttonTitleRes, View.OnClickListener onClickListener) {
Snackbar.make(activity.findViewById(R.id.coordinator), messageRes, Snackbar.LENGTH_LONG)
Copy link
Contributor

Choose a reason for hiding this comment

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

UploadUtils knowing about the coordinator layout ID seems weird - how about we pass in the View to the static methods as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

(This also applies to the second showSnackbar() method below.)


public static void showSnackbar(Activity activity, String text) {
Snackbar.make(activity.findViewById(R.id.coordinator),
text, Snackbar.LENGTH_LONG).show();
Copy link
Contributor

Choose a reason for hiding this comment

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

Since we're doing this, let's make this one consistent and use the Snackbar.make() method that accepts a resource ID instead of a String (like the showSnackbar() above this one is using).

}
}

public static void showSnackbar(Activity activity, int messageRes, int buttonTitleRes, View.OnClickListener onClickListener) {
Copy link
Contributor

Choose a reason for hiding this comment

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

These two showSnackbar() methods can be private, I think it makes to hide them.

Copy link
Contributor

Choose a reason for hiding this comment

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

This line and several others exceed 120 chars.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

addressed in e37d98b

@mzorz
Copy link
Contributor Author

mzorz commented Jul 31, 2017

ready for another round @aforcier 🙇

public void onPostUploaded(PostStore.OnPostUploaded event) {
final PostModel post = event.post;
SiteModel site = getSelectedSite();
if (isAdded() && event.post != null && event.post.getLocalSiteId() == site.getId()) {
Copy link
Contributor

Choose a reason for hiding this comment

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

getSelectedSite() is @Nullable, so we need to null check before calling site.getId() in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oops good catch - did that check and also moved isAdded() check a bit up here 28a9d58

…s added, then checked for site != null to avoid NPE
@aforcier aforcier self-assigned this Jul 31, 2017
@aforcier aforcier removed the [Status] Needs Design Review A designer needs to sign off on the implemented design. label Jul 31, 2017
@aforcier
Copy link
Contributor

Looks good @mzorz! :shipit:

@aforcier aforcier merged commit 2483fef into feature/async-media Jul 31, 2017
@aforcier aforcier deleted the feature/add-post-snackbar-my-site branch July 31, 2017 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants