-
Notifications
You must be signed in to change notification settings - Fork 739
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
FTUE Auth - I already have an account #4668
Conversation
I'm not super happy~ about having to the copy all of the login fragments, unfortunately there's a lot of inheritance in the area and the fragments are coupled directly to view models would recommend reviewing by commits! |
0dc4d38
to
74594d8
Compare
6846cf2
to
4d02621
Compare
closing to extract the fragment copying to its own PR |
854953e
to
801a02e
Compare
9ccad11
to
603c2fa
Compare
- supports switching between a copied legacy flow (DefaultFTUE) and the WIP variant - this will allow us to make iterative changes to the default ftue flow without affecting the legacy flow/forks
…verriding and merge login2 behaviour
… already have an account
- the onboarding uses the icon and it can be displayed for developer mode users
801a02e
to
6fbf8fa
Compare
@@ -36,4 +38,5 @@ interface VectorFeatures { | |||
|
|||
class DefaultVectorFeatures : VectorFeatures { | |||
override fun onboardingVariant(): VectorFeatures.OnboardingVariant = BuildConfig.ONBOARDING_VARIANT | |||
override fun isAlreadyHaveAccountSplashEnabled() = true |
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 I already have an account
is enabled by default when the onboarding variant is set to FTUE_AUTH
, it's currently set to LEGACY
, will raise a dedicated PR to enable the FTUE_AUTH
onboarding as the default variant
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.
LGTM
@@ -28,16 +28,25 @@ class DebugFeaturesStateFactory @Inject constructor( | |||
return FeaturesState(listOf( | |||
createEnumFeature( | |||
label = "Onboarding variant", | |||
selection = debugFeatures.onboardingVariant(), | |||
override = debugFeatures.onboardingVariant(), |
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 am surprised override
is not a reserved Kotlin keyword. Maybe a bit confusing to use it anyway.
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.
yeah I was surprised as well, will prefix the names to avoid confusion with the language keywords
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.
Fixes #4382 Part of #4584
I already have an account
button and flow behind the FTUE Auth feature flag