-
Notifications
You must be signed in to change notification settings - Fork 671
Conversation
i guess there is a shorter or a better code :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fix the checkstyle and the small comments and this PR can be merged.
Thank you very much for your contribution @bluemix !
@@ -37,6 +37,7 @@ | |||
private final String buttonText; | |||
private final View.OnClickListener onButtonClickListener; | |||
private final Snackbar.Callback snackbarCallback; | |||
private int duration = 5_000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No need to give it a default value if it already has one in the builder
@@ -76,16 +80,26 @@ private SnackbarOnDeniedPermissionListener(ViewGroup rootView, String text, Stri | |||
private String buttonText; | |||
private View.OnClickListener onClickListener; | |||
private Snackbar.Callback snackbarCallback; | |||
private int duration = 5_000; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The previous default value was Snackbar.LENGTH_LONG
public static Builder with(ViewGroup rootView, String text) { | ||
return new Builder(rootView, text); | ||
} | ||
|
||
public static Builder | ||
with(ViewGroup rootView, String text, int duration) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Being duration
an optional parameter, I'd create a new non-static method withDuration
instead
Besides, there is a very similar implementation called |
I'm merging this PR in #141 and releasing a new version |
i guess there is a shorter or a better code :)