Use UserIndicatorPresenter for onboarding personalisation.

Remove the service from the display name screen to match the avatar screen.
Add a loading indicator to PhotoPickerPresenter.
Fix layout issue when selecting non-square avatar image.

RiotSwiftUI/Modules/Onboarding/DisplayName/Coordinator/OnboardingDisplayNameCoordinator.swift

@@ -15,6 +15,7 @@
 //
 
 import SwiftUI
+import CommonKit
 
 struct OnboardingDisplayNameCoordinatorParameters {
     let userSession: UserSession
@@ -31,6 +32,9 @@ final class OnboardingDisplayNameCoordinator: Coordinator, Presentable {
     private let onboardingDisplayNameHostingController: VectorHostingController
     private var onboardingDisplayNameViewModel: OnboardingDisplayNameViewModelProtocol
     
+    private var indicatorPresenter: UserIndicatorTypePresenterProtocol
+    private var waitingIndicator: UserIndicator?
+    
     // MARK: Public
 
     // Must be used only internally
@@ -41,25 +45,64 @@ final class OnboardingDisplayNameCoordinator: Coordinator, Presentable {
     
     init(parameters: OnboardingDisplayNameCoordinatorParameters) {
         self.parameters = parameters
-        let viewModel = OnboardingDisplayNameViewModel.makeOnboardingDisplayNameViewModel(onboardingDisplayNameService: OnboardingDisplayNameService(userSession: parameters.userSession))
+        
+        // Don't pre-fill the display name from the MXID to encourage the user to enter something
+        let viewModel = OnboardingDisplayNameViewModel()
+        
         let view = OnboardingDisplayNameScreen(viewModel: viewModel.context)
         onboardingDisplayNameViewModel = viewModel
         onboardingDisplayNameHostingController = VectorHostingController(rootView: view)
         onboardingDisplayNameHostingController.vc_removeBackTitle()
         onboardingDisplayNameHostingController.enableNavigationBarScrollEdgesAppearance = true
+        
+        indicatorPresenter = UserIndicatorTypePresenter(presentingViewController: onboardingDisplayNameHostingController)
     }
     
     // MARK: - Public
     func start() {
         MXLog.debug("[OnboardingDisplayNameCoordinator] did start.")
-        onboardingDisplayNameViewModel.completion = { [weak self] in
+        onboardingDisplayNameViewModel.completion = { [weak self] result in
             guard let self = self else { return }
             MXLog.debug("[OnboardingDisplayNameCoordinator] OnboardingDisplayNameViewModel did complete.")
-            self.completion?(self.parameters.userSession)
+            
+            switch result {
+            case .save(let displayName):
+                self.setDisplayName(displayName)
+            case .skip:
+                self.completion?(self.parameters.userSession)
+            }
         }
     }
     
     func toPresentable() -> UIViewController {
         return self.onboardingDisplayNameHostingController
     }
+    
+    // MARK: - Private
+    
+    private func startWaiting() {
+        waitingIndicator = indicatorPresenter.present(.loading(label: VectorL10n.saving, isInteractionBlocking: true))
+    }
+    
+    private func stopWaiting(error: Error? = nil) {
+        waitingIndicator?.cancel()
+        waitingIndicator = nil
+        
+        if let error = error {
+            onboardingDisplayNameViewModel.update(with: error)
+        }
+    }
+    
+    private func setDisplayName(_ displayName: String) {
+        startWaiting()
+        
+        parameters.userSession.account.setUserDisplayName(displayName) { [weak self] in
+            guard let self = self else { return }
+            self.stopWaiting()
+            self.completion?(self.parameters.userSession)
+        } failure: { [weak self] error in
+            guard let self = self else { return }
+            self.stopWaiting(error: error)
+        }
+    }
 }

RiotSwiftUI/Modules/Onboarding/DisplayName/MockOnboardingDisplayNameScreenState.swift

@@ -26,7 +26,6 @@ enum MockOnboardingDisplayNameScreenState: MockScreenState, CaseIterable {
     // mock that screen.
     case emptyTextField
     case filledTextField(displayName: String)
-    case operationInProgress(displayName: String)
     
     /// The associated screen
     var screenType: Any.Type {
@@ -37,28 +36,24 @@ enum MockOnboardingDisplayNameScreenState: MockScreenState, CaseIterable {
     static var allCases: [MockOnboardingDisplayNameScreenState] {
         [
             MockOnboardingDisplayNameScreenState.emptyTextField,
-            MockOnboardingDisplayNameScreenState.filledTextField(displayName: "Test User"),
-            MockOnboardingDisplayNameScreenState.operationInProgress(displayName: "Test User"),
+            MockOnboardingDisplayNameScreenState.filledTextField(displayName: "Test User")
         ]
     }
     
     /// Generate the view struct for the screen state.
     var screenView: ([Any], AnyView)  {
-        let service: MockOnboardingDisplayNameService
+        let viewModel: OnboardingDisplayNameViewModel
         switch self {
         case .emptyTextField:
-            service = MockOnboardingDisplayNameService()
+            viewModel = OnboardingDisplayNameViewModel()
         case .filledTextField(let displayName):
-            service = MockOnboardingDisplayNameService(displayName: displayName)
-        case .operationInProgress(let displayName):
-            service = MockOnboardingDisplayNameService(displayName: displayName, isWaiting: true)
+            viewModel = OnboardingDisplayNameViewModel(displayName: displayName)
         }
-        let viewModel = OnboardingDisplayNameViewModel.makeOnboardingDisplayNameViewModel(onboardingDisplayNameService: service)
         
         // can simulate service and viewModel actions here if needs be.
         
         return (
-            [service, viewModel], AnyView(OnboardingDisplayNameScreen(viewModel: viewModel.context))
+            [self, viewModel], AnyView(OnboardingDisplayNameScreen(viewModel: viewModel.context))
         )
     }
 }

RiotSwiftUI/Modules/Onboarding/DisplayName/OnboardingDisplayNameModels.swift

@@ -19,14 +19,19 @@ import Foundation
 // MARK: View model
 
 enum OnboardingDisplayNameViewModelResult {
-    // Can probably be removed
+    case save(String)
+    case skip
 }
 
 // MARK: View
 
 struct OnboardingDisplayNameViewState: BindableState {
-    var isWaiting = false
     var bindings: OnboardingDisplayNameBindings
+    var validationErrorMessage: String?
+    
+    var textFieldFooterMessage: String {
+        validationErrorMessage ?? VectorL10n.onboardingDisplayNameHint
+    }
 }
 
 struct OnboardingDisplayNameBindings {
@@ -35,6 +40,7 @@ struct OnboardingDisplayNameBindings {
 }
 
 enum OnboardingDisplayNameViewAction {
+    case validateDisplayName
     case save
     case skip
 }

RiotSwiftUI/Modules/Onboarding/DisplayName/OnboardingDisplayNameViewModel.swift

@@ -28,54 +28,43 @@ class OnboardingDisplayNameViewModel: OnboardingDisplayNameViewModelType, Onboar
 
     // MARK: Private
 
-    private let onboardingDisplayNameService: OnboardingDisplayNameServiceProtocol
-
     // MARK: Public
 
-    var completion: (() -> Void)?
+    var completion: ((OnboardingDisplayNameViewModelResult) -> Void)?
 
     // MARK: - Setup
-
-    static func makeOnboardingDisplayNameViewModel(onboardingDisplayNameService: OnboardingDisplayNameServiceProtocol) -> OnboardingDisplayNameViewModelProtocol {
-        return OnboardingDisplayNameViewModel(onboardingDisplayNameService: onboardingDisplayNameService)
-    }
-
-    private init(onboardingDisplayNameService: OnboardingDisplayNameServiceProtocol) {
-        self.onboardingDisplayNameService = onboardingDisplayNameService
-        super.init(initialViewState: Self.defaultState(onboardingDisplayNameService: onboardingDisplayNameService))
-    }
-
-    private static func defaultState(onboardingDisplayNameService: OnboardingDisplayNameServiceProtocol) -> OnboardingDisplayNameViewState {
-        // Start with a blank display name to encourage the user not to just use the first part of their MXID.
-        return OnboardingDisplayNameViewState(bindings: OnboardingDisplayNameBindings(displayName: ""))
+    
+    init(displayName: String = "") {
+        super.init(initialViewState: OnboardingDisplayNameViewState(bindings: OnboardingDisplayNameBindings(displayName: displayName)))
     }
     
     // MARK: - Public
 
     override func process(viewAction: OnboardingDisplayNameViewAction) {
         switch viewAction {
+        case .validateDisplayName:
+            validateDisplayName()
         case .save:
-            setDisplayName()
+            completion?(.save(state.bindings.displayName))
         case .skip:
-            completion?()
+            completion?(.skip)
+        }
+    }
+    
+    func update(with error: Error) {
+        if let error = error as NSError? {
+            state.bindings.alertInfo = AlertInfo(error: error)
         }
     }
     
     // MARK: - Private
     
-    private func setDisplayName() {
-        state.isWaiting = true
-        
-        onboardingDisplayNameService.setDisplayName(context.displayName) { [weak self] result in
-            guard let self = self else { return }
-            self.state.isWaiting = false
-            
-            switch result {
-            case .success(_):
-                self.completion?()
-            case .failure(let error):
-                self.state.bindings.alertInfo = AlertInfo(error: error as NSError)
-            }
+    private func validateDisplayName() {
+        if state.bindings.displayName.count > 256 {
+            guard state.validationErrorMessage == nil else { return }
+            state.validationErrorMessage = VectorL10n.onboardingDisplayNameMaxLength
+        } else if state.validationErrorMessage != nil {
+            state.validationErrorMessage = nil
         }
     }
 }

RiotSwiftUI/Modules/Onboarding/DisplayName/OnboardingDisplayNameViewModelProtocol.swift

@@ -18,9 +18,9 @@ import Foundation
 
 protocol OnboardingDisplayNameViewModelProtocol {
     
-    var completion: (() -> Void)? { get set }
-    @available(iOS 14, *)
-    static func makeOnboardingDisplayNameViewModel(onboardingDisplayNameService: OnboardingDisplayNameServiceProtocol) -> OnboardingDisplayNameViewModelProtocol
+    var completion: ((OnboardingDisplayNameViewModelResult) -> Void)? { get set }
     @available(iOS 14, *)
     var context: OnboardingDisplayNameViewModelType.Context { get }
+    
+    func update(with error: Error)
 }

RiotSwiftUI/Modules/Onboarding/DisplayName/Service/MatrixSDK/OnboardingDisplayNameService.swift

@@ -1,52 +0,0 @@
-// 
-// Copyright 2021 New Vector Ltd
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-import Foundation
-import Combine
-
-@available(iOS 14.0, *)
-class OnboardingDisplayNameService: OnboardingDisplayNameServiceProtocol {
-    
-    enum ServiceError: Error {
-        case unknown
-    }
-    
-    // MARK: - Properties
-    
-    // MARK: Private
-    
-    private let userSession: UserSession
-    
-    // MARK: Public
-    
-    var displayName: String? {
-        userSession.account.userDisplayName
-    }
-    
-    // MARK: - Setup
-    
-    init(userSession: UserSession) {
-        self.userSession = userSession
-    }
-    
-    func setDisplayName(_ displayName: String, completion: @escaping (Result<Bool, Error>) -> Void) {
-        userSession.account.setUserDisplayName(displayName) {
-            completion(.success(true))
-        } failure: { error in
-            completion(.failure(error ?? ServiceError.unknown))
-        }
-    }
-}

RiotSwiftUI/Modules/Onboarding/DisplayName/Service/Mock/MockOnboardingDisplayNameService.swift

@@ -1,35 +0,0 @@
-// 
-// Copyright 2021 New Vector Ltd
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-import Foundation
-import Combine
-
-@available(iOS 14.0, *)
-class MockOnboardingDisplayNameService: OnboardingDisplayNameServiceProtocol {
-    var displayName: String?
-    
-    #warning("isWaiting isn't handled.")
-    init(displayName: String? = nil, isWaiting: Bool = false) {
-        self.displayName = displayName
-    }
-    
-    func setDisplayName(_ displayName: String, completion: @escaping (Result<Bool, Error>) -> Void) {
-        DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(2)) {
-            self.displayName = displayName
-            completion(.success(true))
-        }
-    }
-}

RiotSwiftUI/Modules/Onboarding/DisplayName/Service/OnboardingDisplayNameServiceProtocol.swift

@@ -1,27 +0,0 @@
-// 
-// Copyright 2021 New Vector Ltd
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-import Foundation
-import Combine
-
-@available(iOS 14.0, *)
-protocol OnboardingDisplayNameServiceProtocol {
-    /// The user's current display name read from the `UserSession`.
-    var displayName: String? { get }
-    
-    /// Update the user's display name.
-    func setDisplayName(_ displayName: String, completion: @escaping (Result<Bool, Error>) -> Void)
-}

RiotSwiftUI/Modules/Onboarding/DisplayName/View/OnboardingDisplayNameScreen.swift

@@ -27,16 +27,7 @@ struct OnboardingDisplayNameScreen: View {
     
     @State private var isEditingTextField = false
     
-    #warning("Move these computed properties to the view model")
-    var textFieldFooterString: String {
-        if let errorMessage = viewModel.viewState.validationErrorMessage {
-            return errorMessage
-        }
-        
-        return VectorL10n.onboardingDisplayNameHint
-    }
-    
-    var textFieldFooterColor: Color {
+    private var textFieldFooterColor: Color {
         viewModel.viewState.validationErrorMessage == nil ? theme.colors.tertiaryContent : theme.colors.alert
     }
     
@@ -44,6 +35,8 @@ struct OnboardingDisplayNameScreen: View {
     
     @ObservedObject var viewModel: OnboardingDisplayNameViewModel.Context
     
+    // MARK: - Views
+    
     var body: some View {
         ScrollView {
             VStack(spacing: 0) {
@@ -62,7 +55,6 @@ struct OnboardingDisplayNameScreen: View {
         }
         .accentColor(theme.colors.accent)
         .background(theme.colors.background.ignoresSafeArea())
-        .waitOverlay(show: viewModel.viewState.isWaiting, allowUserInteraction: false)
         .alert(item: $viewModel.alertInfo) { $0.alert }
         .onChange(of: viewModel.displayName) { _ in
             viewModel.send(viewAction: .validateDisplayName)
@@ -98,7 +90,7 @@ struct OnboardingDisplayNameScreen: View {
                                                     isEditing: isEditingTextField,
                                                     isError: viewModel.viewState.validationErrorMessage != nil))
             
-            Text(textFieldFooterString)
+            Text(viewModel.viewState.textFieldFooterMessage)
                 .font(theme.fonts.footnote)
                 .foregroundColor(textFieldFooterColor)
                 .frame(maxWidth: .infinity, alignment: .leading)
@@ -112,7 +104,7 @@ struct OnboardingDisplayNameScreen: View {
                 viewModel.send(viewAction: .save)
             }
             .buttonStyle(PrimaryActionButtonStyle())
-            .disabled(viewModel.displayName.isEmpty || viewModel.viewState.isWaiting)
+            .disabled(viewModel.displayName.isEmpty)
             
             Button { viewModel.send(viewAction: .skip) } label: {
                 Text(VectorL10n.onboardingPersonalizationSkip)