refactor: improve UI for password prompt & password recovery Views#1259
Conversation
russellwheatley
changed the title
| /// Alert Error title | ||
| /// found in: | ||
| /// PasswordRecoveryView | ||
| public var alertErrorTitle: String { |
There was a problem hiding this comment.
Used existing "Error" title for alerts/modal.
| EmailAuthView() | ||
| ScrollView { | ||
| VStack { | ||
| if authService.authView == .authPicker { |
There was a problem hiding this comment.
I made the "Welcome" title only appear when on authPicker view, other wise it appeared in the others, e.g. PasswordRecoveryView.
There was a problem hiding this comment.
If you intend on having a title for the forgot password view later on, you may want to abstract this into an @ViewBuilder header property or something.
| VStack(spacing: 20) { | ||
| SecureField(authService.string.passwordInputLabel, text: $password) | ||
| .textFieldStyle(.roundedBorder) | ||
| Text(authService.string.confirmPasswordInputLabel) |
There was a problem hiding this comment.
Improved the UI for PasswordPromptSheet:
| import FirebaseCore | ||
| import SwiftUI | ||
|
|
||
| enum PasswordRecoveryResult: Identifiable { |
There was a problem hiding this comment.
Improved logic for password recovery so modal pops up telling user to check email for recovery link, or for failure, modal pops up with error.
There was a problem hiding this comment.
Instead of creating a custom type, you can initialize a Swift standard library Result from the authService.sendPasswordRecoveryEmail() function, use it in the switch statements below, and then also omit the do...catch in sendPasswordRecoveryEmail().
There was a problem hiding this comment.
I used Result but still need an Identifiable so that we could control the sheet: 03ebaf2 (#1259)
Also - I still needed to use do/catch as Result expected a synchronous closure and the operation is async
russellwheatley
changed the title
| EmailAuthView() | ||
| ScrollView { | ||
| VStack { | ||
| if authService.authView == .authPicker { |
There was a problem hiding this comment.
If you intend on having a title for the forgot password view later on, you may want to abstract this into an @ViewBuilder header property or something.
| Button(action: { | ||
| withAnimation { | ||
| switchFlow() | ||
| if authService.authenticationState == .authenticated { |
There was a problem hiding this comment.
You could do this slightly more elegantly with a
switch authService.authView {
case .passwordRecovery:
// ...
case .emailLink:
// ...
}There was a problem hiding this comment.
Using switch statement: c013f34 (#1259)
The only thing I don't like is including the default case at the end to satisfy compiler as we're not using updatePassword View in AuthPickerView.
There was a problem hiding this comment.
You can put break in the switch instead of EmptyView
There was a problem hiding this comment.
Doesn't seem possible. I can leave a TODO to refactor this.
| .padding(.bottom, 4) | ||
|
|
||
| Button(action: { | ||
| Task { |
There was a problem hiding this comment.
Why does this need to be wrapped in a Task?
| import FirebaseCore | ||
| import SwiftUI | ||
|
|
||
| enum PasswordRecoveryResult: Identifiable { |
There was a problem hiding this comment.
Instead of creating a custom type, you can initialize a Swift standard library Result from the authService.sendPasswordRecoveryEmail() function, use it in the switch statements below, and then also omit the do...catch in sendPasswordRecoveryEmail().
2 participants
See notes in PR.