Kill BaseHelper and friends #777
Conversation
Change-Id: I07599baa47fba0c0415ebec2a37b7a8ed011e0a2
Change-Id: Ie4e60c4346f5ff5cea2ea7474765c6857bd85e03
Change-Id: I216e2e6d605fb0b3750ddd198e954597db89c1d2
Change-Id: Id3c6e12f3da3fde438aa6da27e9033b30233e6d0
Change-Id: I9d7a4545a171274d5d704d9ac9f704cbaf364238
samtstern
changed the title
Change-Id: I850923fe00efd0be31c75196e917440ca6602708
Change-Id: I3b126f4e08cd359f7e1910cd086176159097650b
| @@ -5,24 +5,38 @@ | |||
| import android.support.annotation.Nullable; | |||
| import android.support.annotation.RestrictTo; | |||
| import android.support.v4.app.DialogFragment; | |||
| import android.view.ContextThemeWrapper; | |||
|
|
|||
| @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) | |||
| public class DialogBase extends DialogFragment { | |||
There was a problem hiding this comment.
@samtstern Could we just get rid of this class? The only dialog using it is the RecoveryEmailSentDialog and it doesn't use any of this. I also noticed that there's dead code in the RecoveryEmailSentDialog:
public static void show(String email, FlowParameters parameters, FragmentManager manager) {
// We never use the flow params
bundle.putParcelable(ExtraConstants.EXTRA_FLOW_PARAMS, parameters);
}|
|
||
| @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) | ||
| public class FragmentBase extends Fragment { | ||
| protected FragmentHelper mHelper; | ||
|
|
||
| protected FlowParameters mFlowParameters; |
There was a problem hiding this comment.
Shouldn't the flow params be private since we have a getter to handle initialization?
| @SuppressWarnings("Registered") | ||
| @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) | ||
| public class HelperActivityBase extends AppCompatActivity { | ||
|
|
||
| protected ActivityHelper mActivityHelper; | ||
| protected FlowParameters mFlowParameters; |
| @SuppressWarnings("Registered") | ||
| @RestrictTo(RestrictTo.Scope.LIBRARY_GROUP) | ||
| public class HelperActivityBase extends AppCompatActivity { | ||
|
|
||
| protected ActivityHelper mActivityHelper; | ||
| protected FlowParameters mFlowParameters; | ||
| protected ProgressDialogHolder mProgressDialogHolder; |
There was a problem hiding this comment.
It looks like the dialog can also be private since you have a getter. Or we could remove the getter, seems easier and would be more consistent with the other code you've written.
| RecoveryEmailSentDialog result = new RecoveryEmailSentDialog(); | ||
| Bundle bundle = new Bundle(); | ||
| bundle.putParcelable(ExtraConstants.EXTRA_FLOW_PARAMS, parameters); |
There was a problem hiding this comment.
Oh, just noticed you added the flow params. Yeah, lol. We should just kill DialogBase since we aren't using any of it and it's forcing us to add dead code to the RecoveryEmailSentDialog.
| return Auth.CredentialsApi; | ||
| } | ||
|
|
||
| public static FirebaseUser getCurrentUser(FlowParameters parameters) { |
There was a problem hiding this comment.
@samtstern We should probably mark this as @Nullable.
Change-Id: Ib07cea70703f6f7fdc2b47e5c9aab43f764350cf
|
@SUPERCILEX thanks for the comments! Think I fixed them all. |
|
@samtstern LGTM! 😄 |
See #776
Steps:
ActivityHelperandFragmentHelper, make everything aBaseHelperFlowParametersandProgressDialoggetFirebaseAuth(), etc) into a dedicated class for that.