Show an account provider picker on the server confirmation screen when required. (#4137)

* Add a picker mode to the ServerConfirmationScreen.

* Hook up the account provider picker in authentication the flow.

Simplify the authentication flow coordinator, removing the restricted state.

* UI/Snapshot tests.
This commit is contained in:
Doug
2025-05-20 17:21:08 +01:00
committed by GitHub
parent 18519c15dd
commit 5197e41928
31 changed files with 436 additions and 109 deletions
@@ -11,6 +11,7 @@ import SwiftUI
struct ServerConfirmationScreenCoordinatorParameters {
let authenticationService: AuthenticationServiceProtocol
let authenticationFlow: AuthenticationFlow
let appSettings: AppSettings
let userIndicatorController: UserIndicatorControllerProtocol
}
@@ -30,7 +31,14 @@ final class ServerConfirmationScreenCoordinator: CoordinatorProtocol {
}
init(parameters: ServerConfirmationScreenCoordinatorParameters) {
let mode = if parameters.appSettings.allowOtherAccountProviders {
ServerConfirmationScreenMode.confirmation(parameters.authenticationService.homeserver.value.address)
} else {
ServerConfirmationScreenMode.picker(parameters.appSettings.accountProviders)
}
viewModel = ServerConfirmationScreenViewModel(authenticationService: parameters.authenticationService,
mode: mode,
authenticationFlow: parameters.authenticationFlow,
userIndicatorController: parameters.userIndicatorController)
}
@@ -16,9 +16,16 @@ enum ServerConfirmationScreenViewModelAction {
case changeServer
}
enum ServerConfirmationScreenMode: Equatable {
/// The user is confirming the displayed account provider (or can enter their own).
case confirmation(String)
/// The user is only allowed to pick from a list of account providers.
case picker([String])
}
struct ServerConfirmationScreenViewState: BindableState {
/// The homeserver address input by the user.
var homeserverAddress: String
/// Whether the screen is configured to confirm a single account provider or pick one from a list of many.
var mode: ServerConfirmationScreenMode
/// The flow being attempted on the selected homeserver.
let authenticationFlow: AuthenticationFlow
/// The presentation anchor used for OIDC authentication.
@@ -28,17 +35,24 @@ struct ServerConfirmationScreenViewState: BindableState {
/// The screen's title.
var title: String {
switch authenticationFlow {
case .login:
return L10n.screenServerConfirmationTitleLogin(homeserverAddress)
case .register:
return L10n.screenServerConfirmationTitleRegister(homeserverAddress)
switch mode {
case .confirmation(let accountProvider):
switch authenticationFlow {
case .login:
L10n.screenServerConfirmationTitleLogin(accountProvider)
case .register:
L10n.screenServerConfirmationTitleRegister(accountProvider)
}
case .picker:
L10n.screenServerConfirmationTitlePickerMode
}
}
/// The message shown beneath the title.
var message: String {
switch authenticationFlow {
var message: String? {
guard case let .confirmation(homeserverAddress) = mode else { return nil }
return switch authenticationFlow {
case .login:
if homeserverAddress == "matrix.org" {
L10n.screenServerConfirmationMessageLoginMatrixDotOrg
@@ -54,6 +68,8 @@ struct ServerConfirmationScreenViewState: BindableState {
}
struct ServerConfirmationScreenBindings {
/// The chosen server when in `.picker` mode, otherwise `nil`.
var pickerSelection: String?
/// Information describing the currently displayed alert.
var alertInfo: AlertInfo<ServerConfirmationScreenAlert>?
}
@@ -22,21 +22,29 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
}
init(authenticationService: AuthenticationServiceProtocol,
mode: ServerConfirmationScreenMode,
authenticationFlow: AuthenticationFlow,
userIndicatorController: UserIndicatorControllerProtocol) {
self.authenticationService = authenticationService
self.authenticationFlow = authenticationFlow
self.userIndicatorController = userIndicatorController
let homeserver = authenticationService.homeserver.value
super.init(initialViewState: ServerConfirmationScreenViewState(homeserverAddress: homeserver.address,
authenticationFlow: authenticationFlow))
let pickerSelection: String? = switch mode {
case .picker(let providers): providers[0]
case .confirmation: nil
}
authenticationService.homeserver
.receive(on: DispatchQueue.main)
.map(\.address)
.weakAssign(to: \.state.homeserverAddress, on: self)
.store(in: &cancellables)
super.init(initialViewState: ServerConfirmationScreenViewState(mode: mode,
authenticationFlow: authenticationFlow,
bindings: .init(pickerSelection: pickerSelection)))
if case .confirmation = mode {
authenticationService.homeserver
.receive(on: DispatchQueue.main)
.map { .confirmation($0.address) }
.weakAssign(to: \.state.mode, on: self)
.store(in: &cancellables)
}
}
override func process(viewAction: ServerConfirmationScreenViewAction) {
@@ -45,7 +53,10 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
guard state.window != window else { return }
Task { state.window = window }
case .confirm:
Task { await confirmHomeserver() }
switch state.mode {
case .confirmation: Task { await confirmServer() }
case .picker: Task { await pickServer() }
}
case .changeServer:
actionsSubject.send(.changeServer)
}
@@ -53,16 +64,18 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
// MARK: - Private
private func confirmHomeserver() async {
private func confirmServer() async {
let homeserver = authenticationService.homeserver.value
// If the login mode is unknown, the service hasn't be configured and we need to do it now.
// If the login mode is unknown, the service hasn't been configured and we need to do it now.
// Otherwise we can continue the flow as server selection has been performed and succeeded.
guard homeserver.loginMode == .unknown || authenticationService.flow != authenticationFlow else {
await fetchLoginURLIfNeededAndContinue()
return
}
// Note: We don't show the spinner until now as it isn't needed if the service is already
// configured and we're about to use password based login
startLoading()
defer { stopLoading() }
@@ -87,6 +100,32 @@ class ServerConfirmationScreenViewModel: ServerConfirmationScreenViewModelType,
}
}
private func pickServer() async {
guard let accountProvider = state.bindings.pickerSelection else {
fatalError("It shouldn't be possible to confirm without a selection.")
}
// Don't bother reconfiguring the service if it has already been done for the selected server.
let homeserver = authenticationService.homeserver.value
guard homeserver.loginMode == .unknown || homeserver.address != accountProvider else {
await fetchLoginURLIfNeededAndContinue()
return
}
// Note: We don't show the spinner until now as it isn't needed if the service is already
// configured and we're about to use password based login
startLoading()
defer { stopLoading() }
switch await authenticationService.configure(for: accountProvider, flow: authenticationFlow) {
case .success:
await fetchLoginURLIfNeededAndContinue()
case .failure:
// When the servers are hard-coded they should have a valid configuration, so show a generic error.
displayError(.unknownError)
}
}
private func fetchLoginURLIfNeededAndContinue() async {
guard authenticationService.homeserver.value.loginMode.supportsOIDCFlow else {
actionsSubject.send(.continueWithPassword)
@@ -11,14 +11,38 @@ import SwiftUI
struct ServerConfirmationScreen: View {
@Bindable var context: ServerConfirmationScreenViewModel.Context
private var backgroundColor: Color {
switch context.viewState.mode {
case .confirmation: .compound.bgCanvasDefault
case .picker: .compound.bgSubtleSecondaryLevel0
}
}
private var headerIcon: KeyPath<CompoundIcons, Image> {
switch context.viewState.mode {
case .confirmation: \.userProfileSolid
case .picker: \.homeSolid
}
}
private var headerIconStyle: BigIcon.Style {
switch context.viewState.mode {
case .confirmation: .defaultSolid
case .picker: .default
}
}
var body: some View {
FullscreenDialog(topPadding: UIConstants.iconTopPaddingToNavigationBar) {
header
VStack(spacing: 36) {
header
mainContent
}
} bottomContent: {
buttons
}
.background()
.backgroundStyle(.compound.bgCanvasDefault)
.backgroundStyle(backgroundColor)
.alert(item: $context.alertInfo)
.introspect(.window, on: .supportedVersions) { window in
context.send(viewAction: .updateWindow(window))
@@ -28,8 +52,7 @@ struct ServerConfirmationScreen: View {
/// The main content of the view to be shown in a scroll view.
var header: some View {
VStack(spacing: 8) {
Image(systemSymbol: .personCropCircleFill)
.bigIcon()
BigIcon(icon: headerIcon, style: headerIconStyle)
.padding(.bottom, 8)
Text(context.viewState.title)
@@ -38,14 +61,26 @@ struct ServerConfirmationScreen: View {
.foregroundColor(.compound.textPrimary)
.fixedSize(horizontal: false, vertical: true)
Text(context.viewState.message)
.font(.compound.bodyMD)
.multilineTextAlignment(.center)
.foregroundColor(.compound.textSecondary)
if let message = context.viewState.message {
Text(message)
.font(.compound.bodyMD)
.multilineTextAlignment(.center)
.foregroundColor(.compound.textSecondary)
}
}
.padding(.horizontal, 16)
}
@ViewBuilder
var mainContent: some View {
if case .picker(let accountProviders) = context.viewState.mode {
FakeInlinePicker(items: accountProviders,
icon: \.host,
selection: $context.pickerSelection)
.accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.serverPicker)
}
}
/// The action buttons shown at the bottom of the view.
var buttons: some View {
VStack(spacing: 16) {
@@ -55,21 +90,53 @@ struct ServerConfirmationScreen: View {
.buttonStyle(.compound(.primary))
.accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.continue)
Button { context.send(viewAction: .changeServer) } label: {
Text(L10n.screenServerConfirmationChangeServer)
.font(.compound.bodyLGSemibold)
.padding(14)
if case .confirmation = context.viewState.mode {
Button { context.send(viewAction: .changeServer) } label: {
Text(L10n.screenServerConfirmationChangeServer)
.font(.compound.bodyLGSemibold)
.padding(14)
}
.accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.changeServer)
}
.accessibilityIdentifier(A11yIdentifiers.serverConfirmationScreen.changeServer)
}
}
}
// This is such a hack. I hate it!
// But… We're not in a List/Form, the compound picker doesn't
// support icons and this screen's design might change so 🤷‍♂️.
private struct FakeInlinePicker: View {
let items: [String]
let icon: KeyPath<CompoundIcons, Image>
@Binding var selection: String?
var body: some View {
VStack(spacing: 0) {
ForEach(items, id: \.self) { item in
ListRow(label: .default(title: item, icon: icon),
kind: .selection(isSelected: selection == item) {
selection = item
})
.overlay(alignment: .bottom) {
if item != items.last {
Divider()
.hidden()
.overlay(Color.compound._borderInteractiveSecondaryAlpha)
.padding(.leading, 54)
}
}
}
}
.clipShape(RoundedRectangle(cornerRadius: 10))
}
}
// MARK: - Previews
struct ServerConfirmationScreen_Previews: PreviewProvider, TestablePreview {
static let loginViewModel = makeViewModel(flow: .login)
static let registerViewModel = makeViewModel(flow: .register)
static let loginViewModel = makeViewModel(mode: .confirmation("matrix.org"), flow: .login)
static let registerViewModel = makeViewModel(mode: .confirmation("matrix.org"), flow: .register)
static let pickerViewModel = makeViewModel(mode: .picker(["dept1.company.com", "dept2.company.com", "dept3.company.com"]), flow: .login)
static var previews: some View {
NavigationStack {
@@ -83,10 +150,17 @@ struct ServerConfirmationScreen_Previews: PreviewProvider, TestablePreview {
.toolbar(.visible, for: .navigationBar)
}
.previewDisplayName("Register")
NavigationStack {
ServerConfirmationScreen(context: pickerViewModel.context)
.toolbar(.visible, for: .navigationBar)
}
.previewDisplayName("Picker")
}
static func makeViewModel(flow: AuthenticationFlow) -> ServerConfirmationScreenViewModel {
static func makeViewModel(mode: ServerConfirmationScreenMode, flow: AuthenticationFlow) -> ServerConfirmationScreenViewModel {
ServerConfirmationScreenViewModel(authenticationService: AuthenticationService.mock,
mode: mode,
authenticationFlow: flow,
userIndicatorController: UserIndicatorControllerMock())
}