-
Notifications
You must be signed in to change notification settings - Fork 1.9k
Differentiate between user cancellation and actual errors #426
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
Changes from 9 commits
358e965
f3d9d72
ae29a5c
e39aaa1
093967d
ba4a1a4
197463e
da54dee
0837a6a
a2dc35e
a541fd8
761e83a
7482754
3fee673
19174f1
d56058b
e36c35d
b3c4892
22dfd43
d8d4f41
c497160
b49e43f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -14,6 +14,7 @@ | |
| import android.widget.Toast; | ||
|
|
||
| import com.bumptech.glide.Glide; | ||
| import com.firebase.ui.auth.ResultCodes; | ||
| import com.firebase.ui.storage.images.FirebaseImageLoader; | ||
| import com.firebase.uidemo.R; | ||
| import com.google.android.gms.tasks.OnCompleteListener; | ||
|
|
@@ -82,7 +83,7 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) { | |
| super.onActivityResult(requestCode, resultCode, data); | ||
|
|
||
| if (requestCode == RC_CHOOSE_PHOTO) { | ||
| if (resultCode == RESULT_OK) { | ||
| if (resultCode == ResultCodes.OK) { | ||
|
||
| Uri selectedImage = data.getData(); | ||
| uploadPhoto(selectedImage); | ||
| } else { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -204,8 +204,8 @@ startActivityForResult( | |
|
|
||
| #####Response codes | ||
| The authentication flow only provides three response codes: | ||
|
||
| `Activity.RESULT_OK` if a user is signed in, `Activity.RESULT_CANCELLED` if | ||
| sign in failed, and `ResultCodes.RESULT_NO_NETWORK` if sign in failed due to a lack of network connectivity. | ||
| `ResultCodes.OK` if a user is signed in, `ResultCodes.CANCELLED` if | ||
| sign in failed, and `ResultCodes.NO_NETWORK` if sign in failed due to a lack of network connectivity. | ||
| No further information on failure is provided as it is not | ||
| typically useful; the only recourse for most apps if sign in fails is to ask | ||
| the user to sign in again later, or proceed with anonymous sign-in if | ||
|
|
@@ -214,24 +214,30 @@ supported. | |
| ```java | ||
| protected void onActivityResult(int requestCode, int resultCode, Intent data) { | ||
| super.onActivityResult(requestCode, resultCode, data); | ||
| if (resultCode == RESULT_OK) { | ||
| if (resultCode == ResultCodes.OK) { | ||
| // user is signed in! | ||
| startActivity(new Intent(this, WelcomeBackActivity.class)); | ||
| finish(); | ||
| return; | ||
| } | ||
|
|
||
| // Sign in canceled | ||
| if (resultCode == RESULT_CANCELED) { | ||
| if (resultCode == ResultCodes.CANCELED) { | ||
| showSnackbar(R.string.sign_in_cancelled); | ||
| return; | ||
| } | ||
|
|
||
| // No network | ||
| if (resultCode == ResultCodes.RESULT_NO_NETWORK) { | ||
| if (resultCode == ResultCodes.NO_NETWORK) { | ||
| showSnackbar(R.string.no_internet_connection); | ||
| return; | ||
| } | ||
|
|
||
|
|
||
| if (resultCode == ResultCodes.UNKNOWN_ERROR) { | ||
| showSnackbar(R.string.unknown_error); | ||
| return; | ||
| } | ||
|
|
||
| // User is not signed in. Maybe just wait for the user to press | ||
| // "sign in" again, or show a message. | ||
|
|
@@ -250,7 +256,7 @@ Intent. | |
| ```java | ||
| protected void onActivityResult(int requestCode, int resultCode, Intent data) { | ||
| super.onActivityResult(requestCode, resultCode, data); | ||
| if (resultCode == RESULT_OK) { | ||
| if (resultCode == ResultCodes.OK) { | ||
| IdpResponse idpResponse = IdpResponse.fromResultIntent(data); | ||
| startActivity(new Intent(this, WelcomeBackActivity.class) | ||
| .putExtra("my_token", idpResponse.getIdpToken())); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,33 @@ | ||
| package com.firebase.ui.auth; | ||
|
|
||
| import android.app.Activity; | ||
|
|
||
| /** | ||
| * Result codes returned when using {@link AuthUI.SignInIntentBuilder#build()} with | ||
| * {@code startActivityForResult}. | ||
| */ | ||
| public final class ResultCodes { | ||
| private ResultCodes() { | ||
| // We don't want people to initialize this class | ||
| } | ||
|
|
||
| /** | ||
| * Sign in succeeded | ||
| **/ | ||
| public static final int OK = Activity.RESULT_OK; | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I decided to unify result codes and make I decided to use the existing activity result codes to not break anything. |
||
|
|
||
| /** | ||
| * Sign in canceled by user | ||
| **/ | ||
| public static final int CANCELED = Activity.RESULT_CANCELED; | ||
|
|
||
| /** | ||
| * Sign in failed due to lack of network connection | ||
| **/ | ||
| public static final int NO_NETWORK = 10; | ||
|
||
|
|
||
| /** | ||
| * An unknown error has occurred | ||
| **/ | ||
| public static final int UNKNOWN_ERROR = 20; | ||
|
||
| } | ||
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 library version should probably be bumped to
1.1.0since this is slight breakage.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.
I think this would technically require a major version (if you're going by semver) since old code would break when updated.
Could we keep the
.ui.ResultCodesclass but have it be all@Deprecatedconstants that are just symlinks to the new class? Then it's fully backwards compatible.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.
Awesome idea!
PS: how does deprecation work? When the library hits 2.0, will we remove the deprecated stuff or are we stuck with it for life?
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.
We can remove deprecated things after a major version change (if we want to). Or we can leave it for life. This will be the first example of deprecation in this library so there's no precedent for us, and every library does it a bit differently.
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.
Ok, thanks for the info! (I'm pretty sure we also deprecated
setProviders(String... providers))