Address PR comments

Add more docs.
Rename PhotoPickerPresenter to MediaPickerPresenter.
Use a Character for the placeholder avatar rather than a string.

Riot/Assets/en.lproj/Untranslated.strings

@@ -33,7 +33,6 @@
 
 "onboarding_avatar_title" = "Add a profile picture";
 "onboarding_avatar_message" = "You can change this anytime.";
-"onboarding_avatar_placeholder_accessibility_label" = "Profile picture, %@";
+"onboarding_avatar_accessibility_label" = "Profile picture";
-"onboarding_avatar_image_accessibility_label" = "Profile picture, image";
 
 "image_picker_action_files" = "Choose from files";

Riot/Assets/en.lproj/Vector.strings

@@ -74,6 +74,7 @@
 "ok" = "OK";
 "error" = "Error";
 "suggest" = "Suggest";
+"edit" = "Edit";
 
 // Activities
 "loading" = "Loading";

Riot/Generated/Strings.swift

@@ -1651,6 +1651,10 @@ public class VectorL10n: NSObject {
   public static var e2eRoomKeyRequestTitle: String { 
     return VectorL10n.tr("Vector", "e2e_room_key_request_title") 
   }
+  /// Edit
+  public static var edit: String { 
+    return VectorL10n.tr("Vector", "edit") 
+  }
   /// Activities
   public static var emojiPickerActivityCategory: String { 
     return VectorL10n.tr("Vector", "emoji_picker_activity_category") 

Riot/Generated/UntranslatedStrings.swift

@@ -14,18 +14,14 @@ public extension VectorL10n {
   static var imagePickerActionFiles: String { 
     return VectorL10n.tr("Untranslated", "image_picker_action_files") 
   }
-  /// Profile picture, image
+  /// Profile picture
-  static var onboardingAvatarImageAccessibilityLabel: String { 
+  static var onboardingAvatarAccessibilityLabel: String { 
-    return VectorL10n.tr("Untranslated", "onboarding_avatar_image_accessibility_label") 
+    return VectorL10n.tr("Untranslated", "onboarding_avatar_accessibility_label") 
   }
   /// You can change this anytime.
   static var onboardingAvatarMessage: String { 
     return VectorL10n.tr("Untranslated", "onboarding_avatar_message") 
   }
-  /// Profile picture, %@
-  public static func onboardingAvatarPlaceholderAccessibilityLabel(_ p1: String) -> String {
-    return VectorL10n.tr("Untranslated", "onboarding_avatar_placeholder_accessibility_label", p1)
-  }
   /// Add a profile picture
   static var onboardingAvatarTitle: String { 
     return VectorL10n.tr("Untranslated", "onboarding_avatar_title") 

Riot/Modules/MediaPickerV2/MediaPickerPresenter.swift

@@ -19,15 +19,18 @@ import PhotosUI
 import CommonKit
 
 @available(iOS 14.0, *)
-protocol PhotoPickerPresenterDelegate: AnyObject {
+protocol MediaPickerPresenterDelegate: AnyObject {
-    func photoPickerPresenter(_ presenter: PhotoPickerPresenter, didPickImage image: UIImage)
+    func mediaPickerPresenter(_ presenter: MediaPickerPresenter, didPickImage image: UIImage)
-    func photoPickerPresenterDidCancel(_ presenter: PhotoPickerPresenter)
+    func mediaPickerPresenterDidCancel(_ presenter: MediaPickerPresenter)
 }
 
 /// A picker for photos and videos from the user's photo library on iOS 14+ using the
 /// new `PHPickerViewController` that doesn't require permission to be granted.
+///
+/// **Note:** If you need to support iOS 12 & 13, then you will need to use the older
+/// `MediaPickerCoordinator`/`MediaPickerViewController` instead.
 @available(iOS 14.0, *)
-final class PhotoPickerPresenter: NSObject {
+final class MediaPickerPresenter: NSObject {
     
     // MARK: - Properties
     
@@ -40,7 +43,7 @@ final class PhotoPickerPresenter: NSObject {
     
     // MARK: Public
     
-    weak var delegate: PhotoPickerPresenterDelegate?
+    weak var delegate: MediaPickerPresenterDelegate?
     
     // MARK: - Public
     
@@ -77,11 +80,11 @@ final class PhotoPickerPresenter: NSObject {
 
 // MARK: - PHPickerViewControllerDelegate
 @available(iOS 14, *)
-extension PhotoPickerPresenter: PHPickerViewControllerDelegate {
+extension MediaPickerPresenter: PHPickerViewControllerDelegate {
     func picker(_ picker: PHPickerViewController, didFinishPicking results: [PHPickerResult]) {
         // TODO: Handle videos and multi-selection
         guard let provider = results.first?.itemProvider, provider.canLoadObject(ofClass: UIImage.self) else {
-            self.delegate?.photoPickerPresenterDidCancel(self)
+            self.delegate?.mediaPickerPresenterDidCancel(self)
             return
         }
         
@@ -93,14 +96,14 @@ extension PhotoPickerPresenter: PHPickerViewControllerDelegate {
             guard let image = image as? UIImage else {
                 DispatchQueue.main.async {
                     self.hideLoadingIndicator()
-                    self.delegate?.photoPickerPresenterDidCancel(self)
+                    self.delegate?.mediaPickerPresenterDidCancel(self)
                 }
                 return
             }
             
             DispatchQueue.main.async {
                 self.hideLoadingIndicator()
-                self.delegate?.photoPickerPresenter(self, didPickImage: image)
+                self.delegate?.mediaPickerPresenter(self, didPickImage: image)
             }
         }
     }

Riot/Modules/Onboarding/OnboardingCoordinator.swift

@@ -343,7 +343,7 @@ final class OnboardingCoordinator: NSObject, OnboardingCoordinatorProtocol {
     @available(iOS 14.0, *)
     private func congratulationsCoordinator(_ coordinator: OnboardingCongratulationsCoordinator, didCompleteWith result: OnboardingCongratulationsCoordinatorResult) {
         switch result {
-        case .personaliseProfile(let userSession):
+        case .personalizeProfile(let userSession):
             if shouldShowDisplayNameScreen {
                 showDisplayNameScreen(for: userSession)
                 return

RiotSwiftUI/Modules/Common/Avatar/View/PlaceholderAvatarImage.swift

@@ -17,6 +17,11 @@
 import SwiftUI
 
 @available(iOS 14.0, *)
+/// A reusable view that will show a standard placeholder avatar with the
+/// supplied character and colour index for the `namesAndAvatars` color array.
+///
+/// This view has a forced 1:1 aspect ratio but will appear very large until a `.frame`
+/// modifier is applied.
 struct PlaceholderAvatarImage: View {
     
     // MARK: - Private
@@ -25,7 +30,7 @@ struct PlaceholderAvatarImage: View {
     
     // MARK: - Public
     
-    let firstCharacter: String
+    let firstCharacter: Character
     let colorIndex: Int
     
     // MARK: - Views
@@ -34,7 +39,7 @@ struct PlaceholderAvatarImage: View {
         ZStack {
             theme.colors.namesAndAvatars[colorIndex]
             
-            Text(firstCharacter)
+            Text(String(firstCharacter))
                 .padding(4)
                 .foregroundColor(.white)
                 // Make the text resizable (i.e. Make it large and then allow it to scale down)

RiotSwiftUI/Modules/Common/Avatar/View/SpaceAvatarImage.swift

@@ -35,7 +35,7 @@ struct SpaceAvatarImage: View {
             case .empty:
                 ProgressView()
             case .placeholder(let firstCharacter, let colorIndex):
-                Text(firstCharacter)
+                Text(String(firstCharacter))
                     .padding(10)
                     .frame(width: CGFloat(size.rawValue), height: CGFloat(size.rawValue))
                     .foregroundColor(.white)

RiotSwiftUI/Modules/Common/Avatar/ViewModel/AvatarViewState.swift

@@ -19,6 +19,6 @@ import UIKit
 
 enum AvatarViewState {
     case empty
-    case placeholder(String, Int)
+    case placeholder(Character, Int)
     case avatar(UIImage)
 }

RiotSwiftUI/Modules/Common/Avatar/ViewModel/PlaceholderAvatarViewModel.swift

@@ -25,12 +25,9 @@ struct PlaceholderAvatarViewModel {
     /// The number of total colors available for the `stableColorIndex`.
     let colorCount: Int
     
-    /// Get the first character of the display name capitalized or else an empty string.
+    /// Get the first character of the display name capitalized or else a space character.
-    var firstCharacterCapitalized: String {
+    var firstCharacterCapitalized: Character {
-        guard let character = displayName?.first else {
+        return displayName?.capitalized.first ?? " "
-            return ""
-        }
-        return String(character).capitalized
     }
     
     /// Provides the same color each time for a specified matrixId

RiotSwiftUI/Modules/Onboarding/Avatar/Coordinator/OnboardingAvatarCoordinator.swift

@@ -41,8 +41,8 @@ final class OnboardingAvatarCoordinator: Coordinator, Presentable {
         return presenter
     }()
     
-    private lazy var photoPickerPresenter: PhotoPickerPresenter = {
+    private lazy var mediaPickerPresenter: MediaPickerPresenter = {
-        let presenter = PhotoPickerPresenter()
+        let presenter = MediaPickerPresenter()
         presenter.delegate = self
         return presenter
     }()
@@ -100,24 +100,29 @@ final class OnboardingAvatarCoordinator: Coordinator, Presentable {
     
     // MARK: - Private
     
+    /// Show a blocking activity indicator whilst saving.
     private func startWaiting() {
         waitingIndicator = indicatorPresenter.present(.loading(label: VectorL10n.saving, isInteractionBlocking: true))
     }
     
+    /// Hide the currently displayed activity indicator.
     private func stopWaiting() {
         waitingIndicator = nil
     }
     
+    /// Present an image picker for the device photo library.
     private func pickImage() {
         let controller = toPresentable()
-        photoPickerPresenter.presentPicker(from: controller, with: .images, animated: true)
+        mediaPickerPresenter.presentPicker(from: controller, with: .images, animated: true)
     }
     
+    /// Present a camera view to take a photo to use for the avatar.
     private func takePhoto() {
         let controller = toPresentable()
         cameraPresenter.presentCamera(from: controller, with: [.image], animated: true)
     }
     
+    /// Set the supplied image as user's avatar, completing the screen's display if successful.
     func setAvatar(_ image: UIImage?) {
         guard let image = image else {
             MXLog.error("[OnboardingAvatarCoordinator] setAvatar called with a nil image.")
@@ -160,16 +165,16 @@ final class OnboardingAvatarCoordinator: Coordinator, Presentable {
     }
 }
 
-// MARK: - PhotoPickerPresenterDelegate
+// MARK: - MediaPickerPresenterDelegate
 
 @available(iOS 14.0, *)
-extension OnboardingAvatarCoordinator: PhotoPickerPresenterDelegate {
+extension OnboardingAvatarCoordinator: MediaPickerPresenterDelegate {
-    func photoPickerPresenter(_ presenter: PhotoPickerPresenter, didPickImage image: UIImage) {
+    func mediaPickerPresenter(_ presenter: MediaPickerPresenter, didPickImage image: UIImage) {
         onboardingAvatarViewModel.updateAvatarImage(with: image)
         presenter.dismiss(animated: true, completion: nil)
     }
     
-    func photoPickerPresenterDidCancel(_ presenter: PhotoPickerPresenter) {
+    func mediaPickerPresenterDidCancel(_ presenter: MediaPickerPresenter) {
         presenter.dismiss(animated: true, completion: nil)
     }
 }

RiotSwiftUI/Modules/Onboarding/Avatar/OnboardingAvatarModels.swift

@@ -19,36 +19,45 @@ import UIKit
 // MARK: View model
 
 enum OnboardingAvatarViewModelResult {
+    /// The user would like to choose an image from their photo library.
     case pickImage
+    /// The user would like to take a photo to use as their avatar.
     case takePhoto
+    /// The user would like to set specified image as their avatar.
     case save(UIImage?)
+    /// Move on to the next screen in the flow without setting an avatar.
     case skip
 }
 
 // MARK: View
 
 struct OnboardingAvatarViewState: BindableState {
-    let placeholderAvatarLetter: String
+    /// The letter shown in the placeholder avatar.
+    let placeholderAvatarLetter: Character
+    /// The color index to use for the placeholder avatar's background.
     let placeholderAvatarColorIndex: Int
+    /// The image selected by the user to use as their avatar.
     var avatar: UIImage?
     var bindings: OnboardingAvatarBindings
     
+    /// The image shown in the avatar's button.
     var buttonImage: ImageAsset {
         avatar == nil ? Asset.Images.onboardingAvatarCamera : Asset.Images.onboardingAvatarEdit
     }
-    
-    var avatarAccessibilityLabel: String {
-        avatar == nil ? VectorL10n.onboardingAvatarPlaceholderAccessibilityLabel(placeholderAvatarLetter) : VectorL10n.onboardingAvatarImageAccessibilityLabel
-    }
 }
 
 struct OnboardingAvatarBindings {
+    /// The currently displayed alert's info value otherwise `nil`.
     var alertInfo: AlertInfo<Int>?
 }
 
 enum OnboardingAvatarViewAction {
+    /// The user would like to choose an image from their photo library.
     case pickImage
+    /// The user would like to take a photo to use as their avatar.
     case takePhoto
+    /// The user would like to save their chosen avatar image.
     case save
+    /// Move on to the next screen in the flow without setting an avatar.
     case skip
 }

RiotSwiftUI/Modules/Onboarding/Avatar/View/OnboardingAvatarScreen.swift

@@ -75,8 +75,8 @@ struct OnboardingAvatarScreen: View {
         .onTapGesture { isPresentingPickerSelection = true }
         .actionSheet(isPresented: $isPresentingPickerSelection) { pickerSelectionActionSheet }
         .accessibilityElement(children: .ignore)
-        .accessibilityLabel(viewModel.viewState.avatarAccessibilityLabel)
+        .accessibilityLabel(VectorL10n.onboardingAvatarAccessibilityLabel)
-        .accessibilityValue(VectorL10n.accessibilityButtonLabel)
+        .accessibilityValue(VectorL10n.edit)
     }
     
     /// The button to indicate the user can tap to select an avatar

RiotSwiftUI/Modules/Onboarding/Congratulations/Coordinator/OnboardingCongratulationsCoordinator.swift

@@ -17,12 +17,17 @@
 import SwiftUI
 
 struct OnboardingCongratulationsCoordinatorParameters {
+    /// The user session used to determine the user ID to display.
     let userSession: UserSession
+    /// When `true` the "Personalise Profile" button will be hidden, preventing the
+    /// user from setting a displayname or avatar.
     let personalizationDisabled: Bool
 }
 
 enum OnboardingCongratulationsCoordinatorResult {
-    case personaliseProfile(UserSession)
+    /// Show the display name and/or avatar screens for the user to personalize their profile.
+    case personalizeProfile(UserSession)
+    /// Continue the flow by skipping the display name and avatar screens.
     case takeMeHome(UserSession)
 }
 
@@ -64,8 +69,8 @@ final class OnboardingCongratulationsCoordinator: Coordinator, Presentable {
             MXLog.debug("[OnboardingCongratulationsCoordinator] OnboardingCongratulationsViewModel did complete with result: \(result).")
             
             switch result {
-            case .personaliseProfile:
+            case .personalizeProfile:
-                self.completion?(.personaliseProfile(self.parameters.userSession))
+                self.completion?(.personalizeProfile(self.parameters.userSession))
             case .takeMeHome:
                 self.completion?(.takeMeHome(self.parameters.userSession))
             }

RiotSwiftUI/Modules/Onboarding/Congratulations/OnboardingCongratulationsModels.swift

@@ -21,7 +21,7 @@ import Foundation
 // MARK: View model
 
 enum OnboardingCongratulationsViewModelResult {
-    case personaliseProfile
+    case personalizeProfile
     case takeMeHome
 }
 

RiotSwiftUI/Modules/Onboarding/Congratulations/OnboardingCongratulationsViewModel.swift

@@ -43,7 +43,7 @@ class OnboardingCongratulationsViewModel: OnboardingCongratulationsViewModelType
     override func process(viewAction: OnboardingCongratulationsViewAction) {
         switch viewAction {
         case .personaliseProfile:
-            completion?(.personaliseProfile)
+            completion?(.personalizeProfile)
         case .takeMeHome:
             completion?(.takeMeHome)
         }

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

@@ -80,14 +80,17 @@ final class OnboardingDisplayNameCoordinator: Coordinator, Presentable {
     
     // MARK: - Private
     
+    /// Show a blocking activity indicator whilst saving.
     private func startWaiting() {
         waitingIndicator = indicatorPresenter.present(.loading(label: VectorL10n.saving, isInteractionBlocking: true))
     }
     
+    /// Hide the currently displayed activity indicator.
     private func stopWaiting() {
         waitingIndicator = nil
     }
     
+    /// Set the supplied string as user's display name, completing the screen's display if successful.
     private func setDisplayName(_ displayName: String) {
         startWaiting()
         

RiotSwiftUI/Modules/Onboarding/DisplayName/OnboardingDisplayNameModels.swift

@@ -19,7 +19,9 @@ import Foundation
 // MARK: View model
 
 enum OnboardingDisplayNameViewModelResult {
+    /// The user would like to save the entered display name.
     case save(String)
+    /// Move on to the next screen in the flow without setting a display name.
     case skip
 }
 
@@ -27,20 +29,27 @@ enum OnboardingDisplayNameViewModelResult {
 
 struct OnboardingDisplayNameViewState: BindableState {
     var bindings: OnboardingDisplayNameBindings
+    /// Any error that occurred during display name validation otherwise `nil`.
     var validationErrorMessage: String?
     
+    /// The string to be displayed in the text field's footer.
     var textFieldFooterMessage: String {
         validationErrorMessage ?? VectorL10n.onboardingDisplayNameHint
     }
 }
 
 struct OnboardingDisplayNameBindings {
+    /// The display name string entered by the user.
     var displayName: String
+    /// The currently displayed alert's info value otherwise `nil`.
     var alertInfo: AlertInfo<Int>?
 }
 
 enum OnboardingDisplayNameViewAction {
+    /// The display name needs validation.
     case validateDisplayName
+    /// The user would like to save the entered display name.
     case save
+    /// Move on to the next screen in the flow without setting a display name.
     case skip
 }