Implement SignIn MVP #2783
Implement SignIn MVP #2783
Conversation
Coverage ReportAffected SDKs
Test Logs
NotesHTML coverage reports can be produced locally with Head commit (7fecfcd2) is created by Prow via merging commits: 460fd3f 46f5f2d. |
Binary Size ReportAffected SDKs
Test Logs
NotesHead commit (7fecfcd2) is created by Prow via merging commits: 460fd3f 46f5f2d. |
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Outdated
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
| /** Signs in the App Distribution tester. Presents the tester with a Google sign in UI */ | ||
| @NonNull | ||
| public Task<Void> signInTester() { | ||
| return Tasks.forResult(null); |
There was a problem hiding this comment.
Remove the comment here.
Also should we consider adding some logic to handle the case where a previous signInTaskCompletionSource is not yet complete. Maybe if the previoussignInTaskCompleteionSource is not complete, we just cancel it with an error and then proceed forward?
There was a problem hiding this comment.
Forgot to add cancellation logic, will do that in the next push.
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
| if (supportsCustomTabs(firebaseApp.getApplicationContext())) { | ||
| // If we can launch a chrome view, try that. | ||
| CustomTabsIntent.Builder builder = new CustomTabsIntent.Builder(); | ||
|
|
There was a problem hiding this comment.
Out of curiosity, do we need this animation? How does it behave if we don't set any animation?
Its not a super big deal but unless we really need it, we should not have it. Animations can be a bit wonky on older devices.
There was a problem hiding this comment.
Without the animation the transition looks like how it does for a normal browser where it looks like it's popping up a new app. It's not needed so I'll remove them
...tribution/src/test/java/com/google/firebase/appdistribution/FirebaseAppDistributionTest.java
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Outdated
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
...-distribution/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistribution.java
Show resolved
Hide resolved
...tion/src/main/java/com/google/firebase/appdistribution/FirebaseAppDistributionRegistrar.java
Outdated
Show resolved
Hide resolved
...app-distribution/src/main/java/com/google/firebase/appdistribution/SignInResultActivity.java
Show resolved
Hide resolved
...tribution/src/test/java/com/google/firebase/appdistribution/FirebaseAppDistributionTest.java
Outdated
Show resolved
Hide resolved
| public void onSuccess(String fid) { | ||
| Uri uri = | ||
| Uri.parse( | ||
| String.format( |
There was a problem hiding this comment.
This should change to https://appdistribution.firebase.google.com/pub/apps/" + "%s/installations/%s/buildalerts?appName=%s&packageName=%s
|
/test smoke-tests |
Uh oh!
There was an error while loading. Please reload this page.