-
Notifications
You must be signed in to change notification settings - Fork 663
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
Launch Link Eagerly with new ConfirmationDefinition #9940
base: master
Are you sure you want to change the base?
Conversation
Diffuse output:
APK
|
I think this makes sense as a confirmation definition since we are expecting this to be a payment flow that immediately completes A side note, I think this is the first case of a three-step confirmation flow we have in the repo. |
24cf02f
to
340facd
Compare
@Provides | ||
@NativeLinkScope | ||
fun provideLinkAccountManager( | ||
configuration: LinkConfiguration, | ||
linkRepository: LinkRepository, | ||
linkEventsReporter: LinkEventsReporter, | ||
errorReporter: ErrorReporter, | ||
linkAccount: LinkAccount? | ||
): LinkAccountManager { | ||
return DefaultLinkAccountManager( | ||
config = configuration, | ||
linkRepository = linkRepository, | ||
linkEventsReporter = linkEventsReporter, | ||
errorReporter = errorReporter | ||
).apply { | ||
setAccount(linkAccount) | ||
} | ||
} |
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.
This is a design question mark. I feel like it's safe but would love to discuss.
val isVerified: Boolean = consumerSession.containsVerifiedSMSSession() || | ||
consumerSession.isVerifiedForSignup() | ||
|
||
@IgnoredOnParcel |
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'm kinda surprised that all these fields are ignored on parcel -- how do we actually use the LinkAccount if none of the fields are parcelized? Or am I misunderstanding?
Summary (Proof of Concept)
This is PR adds
link_express
as a new confirmation option. This is the scenario where Link is launched eagerly as 2FA dialog (see screenshot below).link_express
will be launched with aLinkAccount
, and after the 2FA is done, this confirmation definition returns aNextStep
to launchnative_link
with an updatedLinkAccount
.Why are we passing
LinkAccount
around?LinkAccountManager
is scoped to the module that uses it, so the Link auth state inpaymentsheet
does not exist inNativeLink
. We initializeLinkAccountManager
with aLinkAccount
if provided.Why a new ConfirmationDefinition?
I chose to do it this way because I want to keep
LinkActivity
(native link) simple. This eager flow would add more complexity and require at least a small refactor because of the translucent background and other small things like navigation. I'm open to other ideas.Motivation
Testing
Screenshots
Screen.Recording.2025-01-21.at.9.30.41.AM.mov
Changelog