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
Add support for prioritizing SSO Apps #145
Conversation
@@ -138,7 +151,7 @@ private Uri createSsoUri() { | |||
@Override | |||
public boolean isSupported() { | |||
return appProtocol.isInstalled(activity, UBER, MIN_UBER_RIDES_VERSION_SUPPORTED) | |||
|| appProtocol.isInstalled(activity, UBER_EATS, MIN_UBER_EATS_VERSION_SUPPORTED); | |||
|| appProtocol.isInstalled(activity, UBER_EATS, MIN_UBER_EATS_VERSION_SUPPORTED); |
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 has to fork based on productFlowPriority. Otherwise, you could get false positives. For example if the user has rides installed, but their productFlowPriority contains eats but not rides, then the deeplink would say SSO is supported even though it is not.
|
||
if(!validatedPackages.isEmpty()) { | ||
if (!validatedPackages.isEmpty()) { | ||
intent.setPackage(validatedPackages.get(0).packageName); | ||
} | ||
activity.startActivityForResult(intent, requestCode); |
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 know this was here before, do we still want this behavior? My intuition says that we should not be executing the intent if we didn't manage to validate that they have the expected app versions installed
@tyvsmith any thoughts on this?
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.
If there are two and we don't specify one, it'll show the intent picker which is a bad UX. If there are 0, it would have failed at the beginning of the method.
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.
👍
|
||
if(!validatedPackages.isEmpty()) { | ||
if (!validatedPackages.isEmpty()) { | ||
intent.setPackage(validatedPackages.get(0).packageName); | ||
} | ||
activity.startActivityForResult(intent, requestCode); |
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.
If there are two and we don't specify one, it'll show the intent picker which is a bad UX. If there are 0, it would have failed at the beginning of the method.
Description: Allows SDK configuration to specify the desired application(s) to launch.
Related issue(s): #130 and #144