Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

MBL-1233: Consolidate login and signup buttons when OAuth is enabled #1956

Conversation

amy-at-kickstarter
Copy link
Contributor

@amy-at-kickstarter amy-at-kickstarter commented Feb 23, 2024

馃摬 What

Remove the sign up button (and change the login button text) when OAuth is turned on.

馃 Why

Users who want to sign up will go through the web-based sign up flow, using the same web view as login.
Screenshot 2024-02-23 at 12 14 38 PM

馃憖 See

OAuth off:
Screenshot 2024-02-23 at 12 12 52 PM

OAuth on:
Screenshot 2024-02-23 at 12 34 10 PM

@amy-at-kickstarter amy-at-kickstarter requested review from a team and scottkicks and removed request for a team February 23, 2024 17:13
@amy-at-kickstarter amy-at-kickstarter force-pushed the feat/adyer/MBL-1233/consolidate-signup-and-login-buttons branch 2 times, most recently from 898c661 to 6bdaad5 Compare February 23, 2024 17:39

/// True if the feature flag for OAuth login is true.
/// Note that this is not a signal, because we don't want it to ever change after the screen is loaded.
var loginWithOAuthEnabled: Bool { get }
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is perhaps a bit of a paranoid move, but since login is something you might do right after launch, I didn't want there to ever be a situation where the remote config reloads after the login screen has already been presented.

This way, this property is set when the view controller is initialized, and never changes.

Strings.login_tout_back_intent_traditional_login_button()
}
if self.viewModel.outputs.loginWithOAuthEnabled {
// TODO: Add and translate a new version of this string for this page.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will be a separate web PR + a follow-up to this PR once the strings are finished.

@amy-at-kickstarter amy-at-kickstarter marked this pull request as ready for review February 23, 2024 17:41
@amy-at-kickstarter amy-at-kickstarter self-assigned this Feb 23, 2024
@amy-at-kickstarter amy-at-kickstarter force-pushed the feat/adyer/MBL-1233/consolidate-signup-and-login-buttons branch from 6bdaad5 to 9251263 Compare February 23, 2024 19:10
@amy-at-kickstarter amy-at-kickstarter force-pushed the feat/adyer/MBL-1233/consolidate-signup-and-login-buttons branch from 9251263 to ab726c8 Compare February 26, 2024 14:27
@amy-at-kickstarter amy-at-kickstarter merged commit 9f6e16a into main Feb 26, 2024
5 checks passed
@amy-at-kickstarter amy-at-kickstarter deleted the feat/adyer/MBL-1233/consolidate-signup-and-login-buttons branch February 26, 2024 14:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants