#40: Self review for PR

ElementX/Sources/Screens/Authentication/AuthenticationCoordinator.swift

@@ -54,19 +54,21 @@ class AuthenticationCoordinator: Coordinator {
             guard let self = self else { return }
             switch action {
             case .login:
-                self.startNewLoginFlow()
+                self.showLoginScreen()
             case .register:
                 fatalError("Not implemented")
             }
         }
         
-        add(childCoordinator: coordinator)
-        navigationRouter.setRootModule(coordinator)
-        
         coordinator.start()
+        add(childCoordinator: coordinator)
+        
+        navigationRouter.setRootModule(coordinator) { [weak self] in
+            self?.remove(childCoordinator: coordinator)
+        }
     }
     
-    private func startNewLoginFlow() {
+    private func showLoginScreen() {
         let parameters = LoginScreenCoordinatorParameters()
         let coordinator = LoginScreenCoordinator(parameters: parameters)
         
@@ -91,10 +93,12 @@ class AuthenticationCoordinator: Coordinator {
             }
         }
         
-        add(childCoordinator: coordinator)
-        navigationRouter.push(coordinator)
-        
         coordinator.start()
+        add(childCoordinator: coordinator)
+        
+        navigationRouter.push(coordinator) { [weak self] in
+            self?.remove(childCoordinator: coordinator)
+        }
     }
     
     private func login(username: String, password: String) async -> Result<UserSession, AuthenticationCoordinatorError> {

ElementX/Sources/Screens/Authentication/AuthenticationMetrics.swift

@@ -17,23 +17,12 @@
 import SwiftUI
 
 /// Metrics used across the entire onboarding flow.
-struct OnboardingMetrics {
+struct AuthenticationMetrics {
     static let maxContentWidth: CGFloat = 600
     static let maxContentHeight: CGFloat = 750
     
     /// The padding used between the top of the main content and the navigation bar.
     static let topPaddingToNavigationBar: CGFloat = 16
-    /// The padding used between the footer and the bottom of the view.
-    static let actionButtonBottomPadding: CGFloat = 24
     /// The width/height used for the main icon shown in most of the screens.
     static let iconSize: CGFloat = 90
-    
-    /// The padding used to the top of the view for breaker screens that don't have a navigation bar.
-    static let breakerScreenTopPadding: CGFloat = 80
-    static let breakerScreenIconBottomPadding: CGFloat = 42
-    
-    /// The height to use for top/bottom spacers to pad the views to fit the `maxContentHeight`.
-    static func spacerHeight(in geometry: GeometryProxy) -> CGFloat {
-        max(0, (geometry.size.height - maxContentHeight) / 2)
-    }
 }

ElementX/Sources/Screens/Onboarding/Common/String-Tinting.swift

@@ -1,29 +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 SwiftUI
-
-extension String {
-    func tinting(_ character: Character, color: Color = .accentColor) -> AttributedString {
-        var string = AttributedString(self)
-        let characterView = string.characters
-        for index in characterView.indices where characterView[index] == character {
-            string[index..<characterView.index(after: index)].foregroundColor = color
-        }
-        
-        return string
-    }
-}

ElementX/Sources/Screens/SplashScreen/SplashScreenModels.swift

@@ -65,7 +65,7 @@ struct SplashScreenViewState: BindableState, CustomDebugStringConvertible {
         let locale = Locale.current
         let page4Title = locale.identifier.hasPrefix("en") ? "Cut the slack from teams." : ElementL10n.ftueAuthCarouselWorkplaceTitle
         
-        self.content = [
+        content = [
             SplashScreenPageContent(title: ElementL10n.ftueAuthCarouselSecureTitle.tinting("."),
                                               message: ElementL10n.ftueAuthCarouselSecureBody,
                                               image: Asset.Images.splashScreenPage1,
@@ -83,7 +83,7 @@ struct SplashScreenViewState: BindableState, CustomDebugStringConvertible {
                                               image: Asset.Images.splashScreenPage4,
                                               gradient: Gradient(colors: [Constants.gradientColors[3], Constants.gradientColors[4]]))
         ]
-        self.bindings = SplashScreenBindings()
+        bindings = SplashScreenBindings()
     }
 }
 

ElementX/Sources/Screens/SplashScreen/View/SplashScreen.swift

@@ -90,7 +90,7 @@ struct SplashScreen: View {
                 
                 buttons
                     .padding(.horizontal, 16)
-                    .frame(maxWidth: OnboardingMetrics.maxContentWidth)
+                    .frame(maxWidth: AuthenticationMetrics.maxContentWidth)
                 Spacer()
             }
             .background(ViewFrameReader(frame: $overlayFrame))
@@ -109,33 +109,20 @@ struct SplashScreen: View {
     
     // MARK: - Animation
     
-    func nextPage() {
-        // Wrap back round to the first page index when reaching the end.
-        viewModel.pageIndex = (viewModel.pageIndex + 1) % viewModel.viewState.content.count
-    }
-    func previousPage() {
-        // Prevent the hidden page at index -1 from being shown.
-        viewModel.pageIndex = max(0, (viewModel.pageIndex - 1))
-    }
-    func hiddenPage() {
-        // Hidden page for a nicer animation when looping back to the start.
-        viewModel.pageIndex = -1
-    }
-    
     /// Starts the animation timer for an automatic carousel effect.
     private func startTimer() {
         guard pageTimer == nil else { return }
         
         pageTimer = Timer.scheduledTimer(withTimeInterval: 5, repeats: true) { _ in
             if viewModel.pageIndex == pageCount - 1 {
-                hiddenPage()
+                showHiddenPage()
                 
                 withAnimation(.easeInOut(duration: 0.7)) {
-                    nextPage()
+                    showNextPage()
                 }
             } else {
                 withAnimation(.easeInOut(duration: 0.7)) {
-                    nextPage()
+                    showNextPage()
                 }
             }
         }
@@ -149,6 +136,21 @@ struct SplashScreen: View {
         pageTimer.invalidate()
     }
     
+    private func showNextPage() {
+        // Wrap back round to the first page index when reaching the end.
+        viewModel.pageIndex = (viewModel.pageIndex + 1) % viewModel.viewState.content.count
+    }
+    
+    private func showPreviousPage() {
+        // Prevent the hidden page at index -1 from being shown.
+        viewModel.pageIndex = max(0, (viewModel.pageIndex - 1))
+    }
+    
+    private func showHiddenPage() {
+        // Hidden page for a nicer animation when looping back to the start.
+        viewModel.pageIndex = -1
+    }
+    
     // MARK: - Gestures
     
     /// Whether or not a drag gesture is valid or not.
@@ -188,9 +190,9 @@ struct SplashScreen: View {
         
         withAnimation(.easeInOut(duration: 0.2)) {
             if drag.predictedEndTranslation.width < -viewSize.width / 2 {
-                nextPage()
+                showNextPage()
             } else if drag.predictedEndTranslation.width > viewSize.width / 2 {
-                previousPage()
+                showPreviousPage()
             }
             
             dragOffset = 0

ElementX/Sources/Screens/SplashScreen/View/SplashScreenPage.swift

@@ -68,8 +68,8 @@ struct SplashScreenPage: View {
                 Spacer().frame(maxHeight: overlayHeight)
             }
             .padding(.horizontal, 16)
-            .frame(maxWidth: OnboardingMetrics.maxContentWidth,
-                   maxHeight: OnboardingMetrics.maxContentHeight)
+            .frame(maxWidth: AuthenticationMetrics.maxContentWidth,
+                   maxHeight: AuthenticationMetrics.maxContentHeight)
         }
         .frame(maxWidth: .infinity, maxHeight: .infinity)
         .background(backgroundGradient.ignoresSafeArea())