refactor: email sign-in opt-in, remove providerButton closure #1252
Conversation
| if authService.emailSignInEnabled { | ||
| Text(authService.authenticationFlow == .login ? authService.string | ||
| .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) | ||
| VStack { Divider() } |
There was a problem hiding this comment.
This is kind of lazy of me to ask, but why does the divider need to be in another VStack?
There was a problem hiding this comment.
That's a good catch - I got this from the example app Peter provided a while ago. I tested using Preview here: d13aea6 (#1252)
and found that it made no discernible difference to the UI so I've removed them.
| @@ -127,7 +127,7 @@ extension EmailAuthView: View { | |||
| .frame(maxWidth: .infinity) | |||
| .buttonStyle(.borderedProminent) | |||
| Button(action: { | |||
| authService.authView = .passwordRecovery | |||
| authService.authView = .emailLink | |||
There was a problem hiding this comment.
Should these be two separate buttons?
There was a problem hiding this comment.
They are two separate buttons because they are two separate Views.
password recovery: https://github.com/firebase/FirebaseUI-iOS/blob/opt-in-email-provider/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/EmailAuthView.swift#L84
email link sign-in: https://github.com/firebase/FirebaseUI-iOS/blob/opt-in-email-provider/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/EmailAuthView.swift#L129
Which are rendered in the AuthPickerView here: https://github.com/firebase/FirebaseUI-iOS/blob/opt-in-email-provider/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift#L24-L27
If you prefer a different way then let me know 👍
There was a problem hiding this comment.
@morganchen12 Also - I have a question in the description regarding whether calling AuthService() .withEmailSignIn() should also hide email sign-in. I'm 99% sure that it should but just wanted confirmation.
There was a problem hiding this comment.
Do you mean the opposite, where not calling .withEmailSignIn() should hide it? That sounds good to me.
Question