Merge branch 'fix-modal-changelog'

4 files changed, 111 insertions, 76 deletions

diff --git a/ios/MullvadVPN/Coordinators/App/ApplicationCoordinator.swift b/ios/MullvadVPN/Coordinators/App/ApplicationCoordinator.swift
index 848dffe9af..e19aa34181 100644
--- a/ios/MullvadVPN/Coordinators/App/ApplicationCoordinator.swift
+++ b/ios/MullvadVPN/Coordinators/App/ApplicationCoordinator.swift
@@ -88,11 +88,6 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
         self.apiProxy = apiProxy
         self.devicesProxy = devicesProxy
 
-        /*
-         Uncomment if you'd like to test TOS again
-         TermsOfService.unsetAgreed()
-         */
-
         super.init()
 
         primaryNavigationContainer.delegate = self
@@ -143,7 +138,7 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
             presentLogin(animated: animated, completion: completion)
 
         case .changelog:
-            presentChangeLog(completion: completion)
+            presentChangeLog(animated: animated, completion: completion)
 
         case .tos:
             presentTOS(animated: animated, completion: completion)
@@ -172,8 +167,11 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
                 endHorizontalFlow(animated: context.isAnimated, completion: completion)
                 context.dismissedRoutes.forEach { $0.coordinator.removeFromParent() }
 
-            case .selectLocation, .account, .settings:
-                let coordinator = dismissedRoute.coordinator as! Presentable
+            case .selectLocation, .account, .settings, .changelog:
+                guard let coordinator = dismissedRoute.coordinator as? Presentable else {
+                    completion()
+                    return assertionFailure("Expected presentable coordinator for \(dismissedRoute.route)")
+                }
 
                 coordinator.dismiss(animated: context.isAnimated, completion: completion)
             }
@@ -278,39 +276,59 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
      Continues application flow by evaluating what route to present next.
      */
     private func continueFlow(animated: Bool) {
-        let next = evaluateNextRoute()
+        var nextRoutes = evaluateNextRoutes()
 
-        /*
-         On iPad the main route is always visible as it's a part of root controller hence we never
-         ask router to navigate to it. Instead this is when we hide the primary horizontal
-         navigation.
-         */
-        if isPad, next == .main {
-            router.dismissAll(.primary, animated: animated)
-        } else {
-            router.present(next, animated: animated)
+        if isPad {
+            /*
+             On iPad the main route is always visible as it's a part of root controller hence we never
+             ask router to navigate to it. Instead this is when we hide the primary horizontal
+             navigation.
+             */
+            if nextRoutes.contains(.main) {
+                router.dismissAll(.primary, animated: animated)
+            }
+
+            nextRoutes.removeAll { $0 == .main }
+        }
+
+        for nextRoute in nextRoutes {
+            router.present(nextRoute, animated: animated)
         }
     }
 
-    private func evaluateNextRoute() -> AppRoute {
+    /**
+     Evaluates conditions and returns the routes that need to be presented next.
+     */
+    private func evaluateNextRoutes() -> [AppRoute] {
+        // Show TOS alone blocking all other routes.
         guard TermsOfService.isAgreed else {
-            return .tos
+            return [.tos]
         }
 
-        guard ChangeLog.isSeen else {
-            return .changelog
-        }
+        var routes = [AppRoute]()
 
+        // Pick the primary route to present
         switch tunnelManager.deviceState {
         case .revoked:
-            return .revoked
+            routes.append(.revoked)
 
         case .loggedOut:
-            return .login
+            routes.append(.login)
 
         case let .loggedIn(accountData, _):
-            return accountData.isExpired ? (accountData.isNew ? .welcome : .outOfTime) : .main
+            if accountData.isExpired {
+                routes.append(accountData.isNew ? .welcome : .outOfTime)
+            } else {
+                routes.append(.main)
+            }
+        }
+
+        // Changelog can be presented simultaneously with other routes.
+        if !ChangeLog.isSeen {
+            routes.append(.changelog)
         }
+
+        return routes
     }
 
     private func logoutRevokedDevice() {
@@ -492,14 +510,20 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
         }
     }
 
-    private func presentChangeLog(completion: @escaping (Coordinator) -> Void) {
-        let coordinator = ChangeLogCoordinator(navigationController: primaryNavigationContainer)
+    private func presentChangeLog(animated: Bool, completion: @escaping (Coordinator) -> Void) {
+        let coordinator = ChangeLogCoordinator()
 
-        addChild(coordinator)
-        coordinator.start(animated: false)
+        coordinator.didFinish = { [weak self] in
+            ChangeLog.markAsSeen()
 
-        continueFlow(animated: false)
-        completion(coordinator)
+            self?.router.dismiss(.changelog, animated: true)
+        }
+
+        coordinator.start()
+
+        presentChild(coordinator, animated: animated) {
+            completion(coordinator)
+        }
     }
 
     private func presentMain(animated: Bool, completion: @escaping (Coordinator) -> Void) {
diff --git a/ios/MullvadVPN/Coordinators/App/ApplicationRouter.swift b/ios/MullvadVPN/Coordinators/App/ApplicationRouter.swift
index c83bdf3fe1..5a8b8ecd7d 100644
--- a/ios/MullvadVPN/Coordinators/App/ApplicationRouter.swift
+++ b/ios/MullvadVPN/Coordinators/App/ApplicationRouter.swift
@@ -36,6 +36,11 @@ enum AppRouteGroup: Comparable, Equatable, Hashable {
     case settings
 
     /**
+     Changelog group.
+     */
+    case changelog
+
+    /**
      Returns `true` if group is presented modally, otherwise `false` if group is a part of root view
      controller.
      */
@@ -44,7 +49,7 @@ enum AppRouteGroup: Comparable, Equatable, Hashable {
         case .primary:
             return UIDevice.current.userInterfaceIdiom == .pad
 
-        case .selectLocation, .account, .settings:
+        case .selectLocation, .account, .settings, .changelog:
             return true
         }
     }
@@ -53,7 +58,7 @@ enum AppRouteGroup: Comparable, Equatable, Hashable {
         switch self {
         case .primary:
             return 0
-        case .settings, .account, .selectLocation:
+        case .settings, .account, .selectLocation, .changelog:
             return 1
         }
     }
@@ -83,16 +88,21 @@ enum AppRoute: Equatable, Hashable {
     case selectLocation
 
     /**
+     Changelog route.
+     */
+    case changelog
+
+    /**
      Routes that are part of primary horizontal navigation group.
      */
-    case tos, changelog, login, main, revoked, outOfTime, welcome, setupAccountCompleted
+    case tos, login, main, revoked, outOfTime, welcome, setupAccountCompleted
 
     /**
      Returns `true` when only one route of a kind can be displayed.
      */
     var isExclusive: Bool {
         switch self {
-        case .selectLocation, .account, .settings:
+        case .selectLocation, .account, .settings, .changelog:
             return true
         default:
             return false
@@ -115,8 +125,10 @@ enum AppRoute: Equatable, Hashable {
      */
     var routeGroup: AppRouteGroup {
         switch self {
-        case .tos, .changelog, .login, .main, .revoked, .outOfTime, .welcome, .setupAccountCompleted:
+        case .tos, .login, .main, .revoked, .outOfTime, .welcome, .setupAccountCompleted:
             return .primary
+        case .changelog:
+            return .changelog
         case .selectLocation:
             return .selectLocation
         case .account:
diff --git a/ios/MullvadVPN/Coordinators/App/ChangeLogCoordinator.swift b/ios/MullvadVPN/Coordinators/App/ChangeLogCoordinator.swift
index 3b4f106205..fe42cd9fbe 100644
--- a/ios/MullvadVPN/Coordinators/App/ChangeLogCoordinator.swift
+++ b/ios/MullvadVPN/Coordinators/App/ChangeLogCoordinator.swift
@@ -9,15 +9,44 @@
 import MullvadLogging
 import UIKit
 
-final class ChangeLogCoordinator: Coordinator {
+final class ChangeLogCoordinator: Coordinator, Presentable {
     private let logger = Logger(label: "ChangeLogCoordinator")
-    private let navigationController: UIViewController
+
+    private var alertController: CustomAlertViewController?
 
     var presentedViewController: UIViewController {
-        return navigationController
+        return alertController!
+    }
+
+    var didFinish: (() -> Void)?
+
+    func start() {
+        alertController = CustomAlertViewController(
+            header: Bundle.main.shortVersion,
+            title: NSLocalizedString(
+                "CHANGE_LOG_TITLE",
+                tableName: "Account",
+                value: "Changes in this version:",
+                comment: ""
+            ),
+            attributedMessage: readChangeLogFromFile()
+        )
+
+        alertController?.addAction(
+            title: NSLocalizedString(
+                "CHANGE_LOG_OK_ACTION",
+                tableName: "Account",
+                value: "Got it!",
+                comment: ""
+            ),
+            style: .default,
+            handler: { [weak self] in
+                self?.didFinish?()
+            }
+        )
     }
 
-    private var changeLogText: NSAttributedString? {
+    private func readChangeLogFromFile() -> NSAttributedString? {
         guard let changeLogText = try? ChangeLog.readFromFile() else {
             logger.error("Cannot read changelog from bundle.")
             return nil
@@ -43,39 +72,4 @@ final class ChangeLogCoordinator: Coordinator {
             ]
         )
     }
-
-    init(navigationController: UIViewController) {
-        self.navigationController = navigationController
-    }
-
-    func start(animated: Bool) {
-        ChangeLog.markAsSeen()
-
-        guard let changeLogText else {
-            return
-        }
-
-        let alertController = CustomAlertViewController(
-            header: Bundle.main.shortVersion,
-            title: NSLocalizedString(
-                "CHANGE_LOG_TITLE",
-                tableName: "Account",
-                value: "Changes in this version:",
-                comment: ""
-            ),
-            attributedMessage: changeLogText
-        )
-
-        alertController.addAction(
-            title: NSLocalizedString(
-                "CHANGE_LOG_OK_ACTION",
-                tableName: "Account",
-                value: "Got it!",
-                comment: ""
-            ),
-            style: .default
-        )
-
-        presentedViewController.present(alertController, animated: animated)
-    }
 }
diff --git a/ios/MullvadVPN/Coordinators/Base/ModalPresentationConfiguration.swift b/ios/MullvadVPN/Coordinators/Base/ModalPresentationConfiguration.swift
index e2a5271d39..10581cdfba 100644
--- a/ios/MullvadVPN/Coordinators/Base/ModalPresentationConfiguration.swift
+++ b/ios/MullvadVPN/Coordinators/Base/ModalPresentationConfiguration.swift
@@ -14,6 +14,7 @@ import UIKit
 struct ModalPresentationConfiguration {
     var preferredContentSize: CGSize?
     var modalPresentationStyle: UIModalPresentationStyle?
+    var modalTransitionStyle: UIModalTransitionStyle?
     var isModalInPresentation: Bool?
     var transitioningDelegate: UIViewControllerTransitioningDelegate?
     var presentationControllerDelegate: UIAdaptivePresentationControllerDelegate?
@@ -25,6 +26,10 @@ struct ModalPresentationConfiguration {
             vc.modalPresentationStyle = modalPresentationStyle
         }
 
+        if let modalTransitionStyle {
+            vc.modalTransitionStyle = modalTransitionStyle
+        }
+
         if let preferredContentSize {
             vc.preferredContentSize = preferredContentSize
         }