From 2bbc9e3114a36da7bd33f1818ee3663e662424f1 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Thu, 22 May 2025 14:59:09 +0100 Subject: [PATCH 1/7] chore: improve UI for password prompt --- .../Sources/Views/PasswordPromptView.swift | 44 ++++++++++++++----- 1 file changed, 33 insertions(+), 11 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift index c306d2f4336..29539ea4e38 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift @@ -1,3 +1,4 @@ +import FirebaseCore import SwiftUI struct PasswordPromptSheet { @@ -9,26 +10,47 @@ struct PasswordPromptSheet { extension PasswordPromptSheet: View { var body: some View { VStack(spacing: 20) { - SecureField(authService.string.passwordInputLabel, text: $password) - .textFieldStyle(.roundedBorder) + Text(authService.string.confirmPasswordInputLabel) + .font(.largeTitle) + .fontWeight(.bold) .padding() - HStack { - Button(authService.string.cancelButtonLabel) { - coordinator.cancel() - } - Spacer() - Button(authService.string.okButtonLabel) { + Divider() + + LabeledContent { + TextField(authService.string.passwordInputLabel, text: $password) + .textInputAutocapitalization(.never) + .disableAutocorrection(true) + .submitLabel(.next) + } label: { + Image(systemName: "lock") + }.padding(.vertical, 10) + .background(Divider(), alignment: .bottom) + .padding(.bottom, 4) + + Button(action: { + Task { coordinator.submit(password: password) } - .disabled(password.isEmpty) + }) { + Text(authService.string.okButtonLabel) + .padding(.vertical, 8) + .frame(maxWidth: .infinity) + } + .disabled(password.isEmpty) + .padding([.top, .bottom, .horizontal], 8) + .frame(maxWidth: .infinity) + .buttonStyle(.borderedProminent) + + Button(authService.string.cancelButtonLabel) { + coordinator.cancel() } - .padding(.horizontal) } .padding() } } #Preview { - PasswordPromptSheet(coordinator: PasswordPromptCoordinator()) + FirebaseOptions.dummyConfigurationForPreview() + return PasswordPromptSheet(coordinator: PasswordPromptCoordinator()).environment(AuthService()) } From 176f3eb23ced0deeafb91e8281cfe249e29d0bde Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Fri, 23 May 2025 12:17:38 +0100 Subject: [PATCH 2/7] chore: furhter UI tweaks --- .../Sources/Utils/StringUtils.swift | 7 ++ .../Sources/Views/AuthPickerView.swift | 76 ++++++++++--------- .../Sources/Views/PasswordRecoveryView.swift | 66 +++++++++++----- 3 files changed, 96 insertions(+), 53 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Utils/StringUtils.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Utils/StringUtils.swift index 1dbdc49a484..1f84a883cee 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Utils/StringUtils.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Utils/StringUtils.swift @@ -350,4 +350,11 @@ public class StringUtils { public var privacyPolicyLabel: String { return localizedString(for: "PrivacyPolicy") } + + /// Alert Error title + /// found in: + /// PasswordRecoveryView + public var alertErrorTitle: String { + return localizedString(for: "Error") + } } diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift index cb753567fbe..c78c064fc34 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift @@ -15,46 +15,50 @@ public struct AuthPickerView { extension AuthPickerView: View { public var body: some View { - VStack { - Text(authService.string.authPickerTitle) - .font(.largeTitle) - .fontWeight(.bold) - .padding() - if authService.authenticationState == .authenticated { - SignedInView() - } else if authService.authView == .passwordRecovery { - PasswordRecoveryView() - } else if authService.authView == .emailLink { - EmailLinkView() - } else { - if authService.emailSignInEnabled { - Text(authService.authenticationFlow == .login ? authService.string - .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) - Divider() - EmailAuthView() + ScrollView { + VStack { + if authService.authView == .authPicker { + Text(authService.string.authPickerTitle) + .font(.largeTitle) + .fontWeight(.bold) + .padding() } - VStack { - authService.renderButtons() - }.padding(.horizontal) - if authService.emailSignInEnabled { - Divider() - HStack { - Text(authService - .authenticationFlow == .login ? authService.string.dontHaveAnAccountYetLabel : - authService.string.alreadyHaveAnAccountLabel) - Button(action: { - withAnimation { - switchFlow() + if authService.authenticationState == .authenticated { + SignedInView() + } else if authService.authView == .passwordRecovery { + PasswordRecoveryView() + } else if authService.authView == .emailLink { + EmailLinkView() + } else { + if authService.emailSignInEnabled { + Text(authService.authenticationFlow == .login ? authService.string + .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) + Divider() + EmailAuthView() + } + VStack { + authService.renderButtons() + }.padding(.horizontal) + if authService.emailSignInEnabled { + Divider() + HStack { + Text(authService + .authenticationFlow == .login ? authService.string.dontHaveAnAccountYetLabel : + authService.string.alreadyHaveAnAccountLabel) + Button(action: { + withAnimation { + switchFlow() + } + }) { + Text(authService.authenticationFlow == .signUp ? authService.string + .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) + .fontWeight(.semibold) + .foregroundColor(.blue) } - }) { - Text(authService.authenticationFlow == .signUp ? authService.string - .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) - .fontWeight(.semibold) - .foregroundColor(.blue) } + PrivacyTOCsView(displayMode: .footer) + Text(authService.errorMessage).foregroundColor(.red) } - PrivacyTOCsView(displayMode: .footer) - Text(authService.errorMessage).foregroundColor(.red) } } } diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift index c29e157ee68..f197a26bbc8 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift @@ -1,18 +1,32 @@ import FirebaseCore import SwiftUI +enum PasswordRecoveryResult: Identifiable { + case success + case failure + + var id: String { + switch self { + case .success: return "success" + case .failure: return "failure" + } + } +} + public struct PasswordRecoveryView { @Environment(AuthService.self) private var authService @State private var email = "" - @State private var showModal = false + @State private var result: PasswordRecoveryResult? public init() {} private func sendPasswordRecoveryEmail() async { do { try await authService.sendPasswordRecoveryEmail(to: email) - showModal = true - } catch {} + result = .success + } catch { + result = .failure + } } } @@ -46,30 +60,48 @@ extension PasswordRecoveryView: View { .frame(maxWidth: .infinity) } .disabled(!CommonUtils.isValidEmail(email)) - .padding([.top, .bottom], 8) + .padding([.top, .bottom, .horizontal], 8) .frame(maxWidth: .infinity) .buttonStyle(.borderedProminent) - }.sheet(isPresented: $showModal) { + }.sheet(item: $result) { result in VStack { - Text(authService.string.passwordRecoveryEmailSentTitle) - .font(.largeTitle) - .fontWeight(.bold) - .padding() - Text(authService.string.passwordRecoveryHelperMessage) + switch result { + case .success: + Text(authService.string.passwordRecoveryEmailSentTitle) + .font(.largeTitle) + .fontWeight(.bold) + .padding() + Text(authService.string.passwordRecoveryHelperMessage) + .padding() + + Divider() + + Text(authService.string.passwordRecoveryEmailSentMessage) + .padding() + Button(authService.string.okButtonLabel) { + self.result = nil + } .padding() + case .failure: + Text(authService.string.alertErrorTitle) + .font(.title) + .fontWeight(.semibold) + .padding() - Divider() + Divider() - Text(authService.string.passwordRecoveryEmailSentMessage) + Text(authService.errorMessage) + .padding() + + Divider() + + Button(authService.string.okButtonLabel) { + self.result = nil + } .padding() - Button(authService.string.okButtonLabel) { - showModal = false } - .padding() } .padding() - }.onOpenURL { _ in - // move the user to email/password View } .navigationBarItems(leading: Button(action: { authService.authView = .authPicker From b4ea600140900c6be8fb9609a398095b9b9f09ca Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 27 May 2025 09:01:12 +0100 Subject: [PATCH 3/7] chore: auth picker title --- .../Sources/Views/AuthPickerView.swift | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift index c78c064fc34..d5462dd01bf 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift @@ -11,18 +11,23 @@ public struct AuthPickerView { authService.authenticationFlow = authService .authenticationFlow == .login ? .signUp : .login } + + @ViewBuilder + private var authPickerTitleView: some View { + if authService.authView == .authPicker { + Text(authService.string.authPickerTitle) + .font(.largeTitle) + .fontWeight(.bold) + .padding() + } + } } extension AuthPickerView: View { public var body: some View { ScrollView { VStack { - if authService.authView == .authPicker { - Text(authService.string.authPickerTitle) - .font(.largeTitle) - .fontWeight(.bold) - .padding() - } + authPickerTitleView if authService.authenticationState == .authenticated { SignedInView() } else if authService.authView == .passwordRecovery { From 03ebaf232ca7275d07f9c2e2f1f9e092a3f8ebea Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 27 May 2025 10:25:25 +0100 Subject: [PATCH 4/7] refactor: use SwiftUI Result --- .../Sources/Views/PasswordRecoveryView.swift | 111 +++++++++--------- 1 file changed, 56 insertions(+), 55 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift index f197a26bbc8..129270bcc69 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordRecoveryView.swift @@ -1,31 +1,26 @@ import FirebaseCore import SwiftUI -enum PasswordRecoveryResult: Identifiable { - case success - case failure - - var id: String { - switch self { - case .success: return "success" - case .failure: return "failure" - } - } +private struct ResultWrapper: Identifiable { + let id = UUID() + let value: Result } public struct PasswordRecoveryView { @Environment(AuthService.self) private var authService @State private var email = "" - @State private var result: PasswordRecoveryResult? + @State private var resultWrapper: ResultWrapper? public init() {} private func sendPasswordRecoveryEmail() async { + let recoveryResult: Result + do { try await authService.sendPasswordRecoveryEmail(to: email) - result = .success + resultWrapper = ResultWrapper(value: .success(())) } catch { - result = .failure + resultWrapper = ResultWrapper(value: .failure(error)) } } } @@ -47,9 +42,11 @@ extension PasswordRecoveryView: View { .submitLabel(.next) } label: { Image(systemName: "at") - }.padding(.vertical, 6) - .background(Divider(), alignment: .bottom) - .padding(.bottom, 4) + } + .padding(.vertical, 6) + .background(Divider(), alignment: .bottom) + .padding(.bottom, 4) + Button(action: { Task { await sendPasswordRecoveryEmail() @@ -63,45 +60,9 @@ extension PasswordRecoveryView: View { .padding([.top, .bottom, .horizontal], 8) .frame(maxWidth: .infinity) .buttonStyle(.borderedProminent) - }.sheet(item: $result) { result in - VStack { - switch result { - case .success: - Text(authService.string.passwordRecoveryEmailSentTitle) - .font(.largeTitle) - .fontWeight(.bold) - .padding() - Text(authService.string.passwordRecoveryHelperMessage) - .padding() - - Divider() - - Text(authService.string.passwordRecoveryEmailSentMessage) - .padding() - Button(authService.string.okButtonLabel) { - self.result = nil - } - .padding() - case .failure: - Text(authService.string.alertErrorTitle) - .font(.title) - .fontWeight(.semibold) - .padding() - - Divider() - - Text(authService.errorMessage) - .padding() - - Divider() - - Button(authService.string.okButtonLabel) { - self.result = nil - } - .padding() - } - } - .padding() + } + .sheet(item: $resultWrapper) { wrapper in + resultSheet(wrapper.value) } .navigationBarItems(leading: Button(action: { authService.authView = .authPicker @@ -112,6 +73,46 @@ extension PasswordRecoveryView: View { .foregroundColor(.blue) }) } + + @ViewBuilder + @MainActor + private func resultSheet(_ result: Result) -> some View { + VStack { + switch result { + case .success: + Text(authService.string.passwordRecoveryEmailSentTitle) + .font(.largeTitle) + .fontWeight(.bold) + .padding() + Text(authService.string.passwordRecoveryHelperMessage) + .padding() + + Divider() + + Text(String(format: authService.string.passwordRecoveryEmailSentMessage, email)) + .padding() + + case .failure: + Text(authService.string.alertErrorTitle) + .font(.title) + .fontWeight(.semibold) + .padding() + + Divider() + + Text(authService.errorMessage) + .padding() + } + + Divider() + + Button(authService.string.okButtonLabel) { + self.resultWrapper = nil + } + .padding() + } + .padding() + } } #Preview { From c013f3403cd19816750586432174831cd3dc2142 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 27 May 2025 11:58:21 +0100 Subject: [PATCH 5/7] refactor: use switch statement --- .../Sources/Views/AuthPickerView.swift | 59 ++++++++++--------- 1 file changed, 32 insertions(+), 27 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift index d5462dd01bf..45d74b62e43 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift @@ -30,39 +30,44 @@ extension AuthPickerView: View { authPickerTitleView if authService.authenticationState == .authenticated { SignedInView() - } else if authService.authView == .passwordRecovery { - PasswordRecoveryView() - } else if authService.authView == .emailLink { - EmailLinkView() } else { - if authService.emailSignInEnabled { - Text(authService.authenticationFlow == .login ? authService.string - .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) - Divider() - EmailAuthView() - } - VStack { - authService.renderButtons() - }.padding(.horizontal) - if authService.emailSignInEnabled { - Divider() - HStack { - Text(authService - .authenticationFlow == .login ? authService.string.dontHaveAnAccountYetLabel : - authService.string.alreadyHaveAnAccountLabel) - Button(action: { - withAnimation { - switchFlow() + switch authService.authView { + case .passwordRecovery: + PasswordRecoveryView() + case .emailLink: + EmailLinkView() + case .authPicker: + if authService.emailSignInEnabled { + Text(authService.authenticationFlow == .login ? authService.string + .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) + Divider() + EmailAuthView() + } + VStack { + authService.renderButtons() + }.padding(.horizontal) + if authService.emailSignInEnabled { + Divider() + HStack { + Text(authService + .authenticationFlow == .login ? authService.string.dontHaveAnAccountYetLabel : + authService.string.alreadyHaveAnAccountLabel) + Button(action: { + withAnimation { + switchFlow() + } + }) { + Text(authService.authenticationFlow == .signUp ? authService.string + .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) + .fontWeight(.semibold) + .foregroundColor(.blue) } - }) { - Text(authService.authenticationFlow == .signUp ? authService.string - .emailLoginFlowLabel : authService.string.emailSignUpFlowLabel) - .fontWeight(.semibold) - .foregroundColor(.blue) } } PrivacyTOCsView(displayMode: .footer) Text(authService.errorMessage).foregroundColor(.red) + default: + EmptyView() } } } From b738cf53381f82dacb5af64bc429f3ed0066bcd2 Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Tue, 27 May 2025 12:01:17 +0100 Subject: [PATCH 6/7] chore: rm Task wrapper --- .../Sources/Views/PasswordPromptView.swift | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift index 29539ea4e38..7d9d2bc8132 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/PasswordPromptView.swift @@ -29,9 +29,7 @@ extension PasswordPromptSheet: View { .padding(.bottom, 4) Button(action: { - Task { - coordinator.submit(password: password) - } + coordinator.submit(password: password) }) { Text(authService.string.okButtonLabel) .padding(.vertical, 8) From 03aeb6131a225b06fdc2736ef6dd07da3eebf83b Mon Sep 17 00:00:00 2001 From: russellwheatley Date: Wed, 28 May 2025 09:17:03 +0100 Subject: [PATCH 7/7] chore: leave TODO to refactor later --- .../FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift | 1 + 1 file changed, 1 insertion(+) diff --git a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift index 45d74b62e43..e4c85834eaa 100644 --- a/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift +++ b/FirebaseSwiftUI/FirebaseAuthSwiftUI/Sources/Views/AuthPickerView.swift @@ -67,6 +67,7 @@ extension AuthPickerView: View { PrivacyTOCsView(displayMode: .footer) Text(authService.errorMessage).foregroundColor(.red) default: + // TODO: - possibly refactor this, see: https://github.com/firebase/FirebaseUI-iOS/pull/1259#discussion_r2105473437 EmptyView() } }