From caf5d2fd41c01dcad2c2211be22ca8aee30a03f7 Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Fri, 1 Sep 2023 15:15:03 +0200 Subject: [PATCH] Refactor alerts to be presented on their respective coordinator --- ios/.swiftlint.yml | 2 + ios/MullvadVPN.xcodeproj/project.pbxproj | 6 --- .../xcshareddata/swiftpm/Package.resolved | 22 +++++++++++ ios/MullvadVPN/Classes/AppRoutes.swift | 2 +- .../Coordinators/AccountCoordinator.swift | 23 ++++++----- .../Coordinators/ApplicationCoordinator.swift | 38 +++++++++---------- .../InAppPurchaseCoordinator.swift | 6 ++- .../Coordinators/LoginCoordinator.swift | 11 ++++-- .../Coordinators/OutOfTimeCoordinator.swift | 8 +++- .../Coordinators/SettingsCoordinator.swift | 8 +--- .../Coordinators/TunnelCoordinator.swift | 10 ++++- .../Coordinators/WelcomeCoordinator.swift | 10 ++--- ios/MullvadVPN/SceneDelegate.swift | 9 ++++- .../Account/PaymentAlertPresenter.swift | 10 +++-- .../Alert/AlertPresentation.swift | 10 ++++- .../Alert/AlertPresenter.swift | 18 ++++++++- .../Alert/AlertViewController.swift | 2 +- .../DeviceManagementViewController.swift | 10 +++-- .../PreferencesViewController.swift | 4 +- .../ProblemReportViewController.swift | 10 +++-- ios/Operations/AsyncOperation.swift | 3 -- ios/Routing/Coordinator.swift | 14 +++++++ ios/Routing/Router/ApplicationRouter.swift | 12 ++++-- .../Router/ApplicationRouterDelegate.swift | 2 +- .../Router/ApplicationRouterTypes.swift | 29 +++++++++++++- ios/RoutingTests/RouterBlockDelegate.swift | 6 +-- 26 files changed, 199 insertions(+), 86 deletions(-) create mode 100644 ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved diff --git a/ios/.swiftlint.yml b/ios/.swiftlint.yml index 9e7704961d8c..5a66346daa4f 100644 --- a/ios/.swiftlint.yml +++ b/ios/.swiftlint.yml @@ -31,6 +31,8 @@ line_length: ignores_interpolated_strings: true warning: 120 error: 300 +cyclomatic_complexity: + ignores_case_statements: true type_name: min_length: 4 diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index 0f414a07c159..66c7eddfa0a0 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -395,12 +395,9 @@ 7A21DACF2A30AA3700A787A9 /* UITextField+Appearance.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A21DACE2A30AA3700A787A9 /* UITextField+Appearance.swift */; }; 7A307AD92A8CD8DA0017618B /* Duration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A307AD82A8CD8DA0017618B /* Duration.swift */; }; 7A307ADB2A8F56DF0017618B /* Duration+Extensions.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A307ADA2A8F56DF0017618B /* Duration+Extensions.swift */; }; -<<<<<<< HEAD 7A3353972AAA0F8600F0A71C /* OperationBlockObserverSupport.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A3353962AAA0F8600F0A71C /* OperationBlockObserverSupport.swift */; }; -======= 7A2960F62A963F7500389B82 /* AlertCoordinator.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A2960F52A963F7500389B82 /* AlertCoordinator.swift */; }; 7A2960FD2A964BB700389B82 /* AlertPresentation.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A2960FC2A964BB700389B82 /* AlertPresentation.swift */; }; ->>>>>>> 010d1f888 (Coordinate alert presentation by integrating it into routing system) 7A42DEC92A05164100B209BE /* SettingsInputCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A42DEC82A05164100B209BE /* SettingsInputCell.swift */; }; 7A42DECD2A09064C00B209BE /* SelectableSettingsCell.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A42DECC2A09064C00B209BE /* SelectableSettingsCell.swift */; }; 7A7AD28D29DC677800480EF1 /* FirstTimeLaunch.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */; }; @@ -1317,12 +1314,9 @@ 7A21DACE2A30AA3700A787A9 /* UITextField+Appearance.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = "UITextField+Appearance.swift"; sourceTree = ""; }; 7A307AD82A8CD8DA0017618B /* Duration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = Duration.swift; sourceTree = ""; }; 7A307ADA2A8F56DF0017618B /* Duration+Extensions.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Duration+Extensions.swift"; sourceTree = ""; }; -<<<<<<< HEAD 7A3353962AAA0F8600F0A71C /* OperationBlockObserverSupport.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = OperationBlockObserverSupport.swift; sourceTree = ""; }; -======= 7A2960F52A963F7500389B82 /* AlertCoordinator.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlertCoordinator.swift; sourceTree = ""; }; 7A2960FC2A964BB700389B82 /* AlertPresentation.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AlertPresentation.swift; sourceTree = ""; }; ->>>>>>> 010d1f888 (Coordinate alert presentation by integrating it into routing system) 7A42DEC82A05164100B209BE /* SettingsInputCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SettingsInputCell.swift; sourceTree = ""; }; 7A42DECC2A09064C00B209BE /* SelectableSettingsCell.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SelectableSettingsCell.swift; sourceTree = ""; }; 7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FirstTimeLaunch.swift; sourceTree = ""; }; diff --git a/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved new file mode 100644 index 000000000000..02691892fed1 --- /dev/null +++ b/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -0,0 +1,22 @@ +{ + "pins" : [ + { + "identity" : "swift-log", + "kind" : "remoteSourceControl", + "location" : "https://github.com/apple/swift-log.git", + "state" : { + "revision" : "173f567a2dfec11d74588eea82cecea555bdc0bc", + "version" : "1.4.0" + } + }, + { + "identity" : "wireguard-apple", + "kind" : "remoteSourceControl", + "location" : "https://github.com/mullvad/wireguard-apple.git", + "state" : { + "revision" : "11a00c20dc03f2751db47e94f585c0778c7bde82" + } + } + ], + "version" : 2 +} diff --git a/ios/MullvadVPN/Classes/AppRoutes.swift b/ios/MullvadVPN/Classes/AppRoutes.swift index cf81ddf4445d..c86022f7073e 100644 --- a/ios/MullvadVPN/Classes/AppRoutes.swift +++ b/ios/MullvadVPN/Classes/AppRoutes.swift @@ -94,7 +94,7 @@ enum AppRoute: AppRouteProtocol { /** Alert route. */ - case alert(AlertPresentation) + case alert(String) /** Routes that are part of primary horizontal navigation group. diff --git a/ios/MullvadVPN/Coordinators/AccountCoordinator.swift b/ios/MullvadVPN/Coordinators/AccountCoordinator.swift index efc82800f8d7..a5f55fe28085 100644 --- a/ios/MullvadVPN/Coordinators/AccountCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/AccountCoordinator.swift @@ -29,10 +29,6 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting { navigationController } - var presentationContext: UIViewController { - navigationController - } - var didFinish: ((AccountCoordinator, AccountDismissReason) -> Void)? init( @@ -48,7 +44,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting { let accountController = AccountViewController( interactor: interactor, - errorPresenter: PaymentAlertPresenter(coordinator: self) + errorPresenter: PaymentAlertPresenter(alertContext: self) ) accountController.actionHandler = handleViewControllerAction @@ -133,18 +129,25 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting { // MARK: - Alerts private func logOut() { - let presentation = AlertPresentation(icon: .spinner, message: nil, buttons: []) + let presentation = AlertPresentation( + id: "account-logout-alert", + icon: .spinner, + message: nil, + buttons: [] + ) + + let alertPresenter = AlertPresenter(context: self) interactor.logout { DispatchQueue.main.asyncAfter(deadline: .now() + .seconds(1)) { [weak self] in guard let self else { return } - applicationRouter?.dismiss(.alert(presentation), animated: true) + alertPresenter.dismissAlert(presentation: presentation, animated: true) self.didFinish?(self, .userLoggedOut) } } - applicationRouter?.present(.alert(presentation)) + alertPresenter.showAlert(presentation: presentation, animated: true) } private func showAccountDeviceInfo() { @@ -164,6 +167,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting { ) let presentation = AlertPresentation( + id: "account-device-info-alert", message: message, buttons: [AlertAction( title: NSLocalizedString( @@ -176,6 +180,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting { )] ) - applicationRouter?.present(.alert(presentation), animated: true) + let presenter = AlertPresenter(context: self) + presenter.showAlert(presentation: presentation, animated: true) } } diff --git a/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift b/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift index e8787c453664..83f47b4812cb 100644 --- a/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift @@ -16,8 +16,7 @@ import UIKit Application coordinator managing split view and two navigation contexts. */ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewControllerDelegate, - UISplitViewControllerDelegate, ApplicationRouterDelegate, - NotificationManagerDelegate { + UISplitViewControllerDelegate, ApplicationRouterDelegate, NotificationManagerDelegate { typealias RouteType = AppRoute /** @@ -120,11 +119,11 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo func applicationRouter( _ router: ApplicationRouter, - route: AppRoute, + presentWithContext context: RoutePresentationContext, animated: Bool, completion: @escaping (Coordinator) -> Void ) { - switch route { + switch context.route { case .account: presentAccount(animated: animated, completion: completion) @@ -155,8 +154,8 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo case .welcome: presentWelcome(animated: animated, completion: completion) - case let .alert(presentation): - presentAlert(presentation: presentation, animated: animated, completion: completion) + case .alert: + presentAlert(animated: animated, context: context, completion: completion) } } @@ -564,11 +563,10 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo ) coordinator.didFinishPayment = { [weak self] _ in - guard let self else { return } + guard let self = self else { return } if shouldDismissOutOfTime() { router.dismiss(.outOfTime, animated: true) - continueFlow(animated: true) } } @@ -589,7 +587,7 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo ) coordinator.didFinish = { [weak self] _ in - guard let self else { return } + guard let self = self else { return } appPreferences.isShownOnboarding = true router.dismiss(.welcome, animated: false) continueFlow(animated: false) @@ -607,10 +605,7 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo !(tunnelManager.deviceState.accountData?.isExpired ?? false) } - private func presentSelectLocation( - animated: Bool, - completion: @escaping (Coordinator) -> Void - ) { + private func presentSelectLocation(animated: Bool, completion: @escaping (Coordinator) -> Void) { let coordinator = makeSelectLocationCoordinator(forModalPresentation: true) coordinator.start() @@ -642,19 +637,24 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo } private func presentAlert( - presentation: AlertPresentation, animated: Bool, + context: RoutePresentationContext, completion: @escaping (Coordinator) -> Void ) { - let coordinator = AlertCoordinator(presentation: presentation) + guard let metadata = context.metadata as? AlertMetadata else { + assertionFailure("Could not get AlertMetadata from RoutePresentationContext.") + return + } + + let coordinator = AlertCoordinator(presentation: metadata.presentation) coordinator.didFinish = { [weak self] in - self?.router.dismiss(.alert(presentation)) + self?.router.dismiss(context.route) } coordinator.start() - presentChild(coordinator, animated: animated) { + metadata.context.presentChild(coordinator, animated: animated) { completion(coordinator) } } @@ -989,8 +989,6 @@ fileprivate extension AppPreferencesDataSource { mutating func markChangeLogSeen() { self.lastSeenChangeLogVersion = Bundle.main.shortVersion } - - // swiftlint:disable:next file_length } private protocol Poppable: Presentable { @@ -998,4 +996,6 @@ private protocol Poppable: Presentable { animated: Bool, completion: () -> Void ) + + // swiftlint:disable:next file_length } diff --git a/ios/MullvadVPN/Coordinators/InAppPurchaseCoordinator.swift b/ios/MullvadVPN/Coordinators/InAppPurchaseCoordinator.swift index 7d3a65df5bed..e2ccb578df87 100644 --- a/ios/MullvadVPN/Coordinators/InAppPurchaseCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/InAppPurchaseCoordinator.swift @@ -11,7 +11,7 @@ import Routing import StoreKit import UIKit -class InAppPurchaseCoordinator: Coordinator, Presentable { +class InAppPurchaseCoordinator: Coordinator, Presenting, Presentable { private let navigationController: RootContainerViewController private let interactor: InAppPurchaseInteractor @@ -50,6 +50,7 @@ class InAppPurchaseCoordinator: Coordinator, Presentable { case let .failure(failure): let presentation = AlertPresentation( + id: "in-app-purchase-error-alert", icon: .alert, message: failure.error.localizedDescription, buttons: [ @@ -69,7 +70,8 @@ class InAppPurchaseCoordinator: Coordinator, Presentable { ] ) - applicationRouter?.present(.alert(presentation), animated: true) + let presenter = AlertPresenter(context: self) + presenter.showAlert(presentation: presentation, animated: true) } } } diff --git a/ios/MullvadVPN/Coordinators/LoginCoordinator.swift b/ios/MullvadVPN/Coordinators/LoginCoordinator.swift index af989dc628be..9f01af0f77c7 100644 --- a/ios/MullvadVPN/Coordinators/LoginCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/LoginCoordinator.swift @@ -12,7 +12,7 @@ import Operations import Routing import UIKit -final class LoginCoordinator: Coordinator, DeviceManagementViewControllerDelegate { +final class LoginCoordinator: Coordinator, Presenting, DeviceManagementViewControllerDelegate { private let tunnelManager: TunnelManager private let devicesProxy: REST.DevicesProxy @@ -22,6 +22,10 @@ final class LoginCoordinator: Coordinator, DeviceManagementViewControllerDelegat var didFinish: ((LoginCoordinator) -> Void)? var didCreateAccount: (() -> Void)? + var presentationContext: UIViewController { + navigationController + } + let navigationController: RootContainerViewController init( @@ -107,11 +111,12 @@ final class LoginCoordinator: Coordinator, DeviceManagementViewControllerDelegat ) let controller = DeviceManagementViewController( interactor: interactor, - alertPresenter: AlertPresenter(coordinator: self) + alertPresenter: AlertPresenter(context: self) ) controller.delegate = self + controller.fetchDevices(animateUpdates: false) { [weak self] result in - guard let self else { return } + guard let self = self else { return } switch result { case .success: diff --git a/ios/MullvadVPN/Coordinators/OutOfTimeCoordinator.swift b/ios/MullvadVPN/Coordinators/OutOfTimeCoordinator.swift index 137fbcf44665..d26cb5d2a7bc 100644 --- a/ios/MullvadVPN/Coordinators/OutOfTimeCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/OutOfTimeCoordinator.swift @@ -9,13 +9,17 @@ import Routing import UIKit -class OutOfTimeCoordinator: Coordinator, OutOfTimeViewControllerDelegate { +class OutOfTimeCoordinator: Coordinator, Presenting, OutOfTimeViewControllerDelegate { let navigationController: RootContainerViewController let storePaymentManager: StorePaymentManager let tunnelManager: TunnelManager var didFinishPayment: ((OutOfTimeCoordinator) -> Void)? + var presentationContext: UIViewController { + navigationController + } + private(set) var isMakingPayment = false private var viewController: OutOfTimeViewController? @@ -42,7 +46,7 @@ class OutOfTimeCoordinator: Coordinator, OutOfTimeViewControllerDelegate { let controller = OutOfTimeViewController( interactor: interactor, - errorPresenter: PaymentAlertPresenter(coordinator: self) + errorPresenter: PaymentAlertPresenter(alertContext: self) ) controller.delegate = self diff --git a/ios/MullvadVPN/Coordinators/SettingsCoordinator.swift b/ios/MullvadVPN/Coordinators/SettingsCoordinator.swift index 1cd368fc7944..88a32826b5e1 100644 --- a/ios/MullvadVPN/Coordinators/SettingsCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/SettingsCoordinator.swift @@ -32,10 +32,6 @@ final class SettingsCoordinator: Coordinator, Presentable, Presenting, SettingsV navigationController } - var presentationContext: UIViewController { - navigationController - } - var willNavigate: (( _ coordinator: SettingsCoordinator, _ from: SettingsNavigationRoute?, @@ -159,13 +155,13 @@ final class SettingsCoordinator: Coordinator, Presentable, Presenting, SettingsV case .preferences: return PreferencesViewController( interactor: interactorFactory.makePreferencesInteractor(), - alertPresenter: AlertPresenter(coordinator: self) + alertPresenter: AlertPresenter(context: self) ) case .problemReport: return ProblemReportViewController( interactor: interactorFactory.makeProblemReportInteractor(), - alertPresenter: AlertPresenter(coordinator: self) + alertPresenter: AlertPresenter(context: self) ) case .faq: diff --git a/ios/MullvadVPN/Coordinators/TunnelCoordinator.swift b/ios/MullvadVPN/Coordinators/TunnelCoordinator.swift index 5f59a6e3bfd8..255728ccef1b 100644 --- a/ios/MullvadVPN/Coordinators/TunnelCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/TunnelCoordinator.swift @@ -9,12 +9,16 @@ import Routing import UIKit -class TunnelCoordinator: Coordinator { +class TunnelCoordinator: Coordinator, Presenting { private let tunnelManager: TunnelManager private let controller: TunnelViewController private var tunnelObserver: TunnelObserver? + var presentationContext: UIViewController { + controller + } + var rootViewController: UIViewController { controller } @@ -59,6 +63,7 @@ class TunnelCoordinator: Coordinator { private func showCancelTunnelAlert() { let presentation = AlertPresentation( + id: "main-cancel-tunnel-alert", icon: .alert, message: NSLocalizedString( "CANCEL_TUNNEL_ALERT_MESSAGE", @@ -91,6 +96,7 @@ class TunnelCoordinator: Coordinator { ] ) - applicationRouter?.present(.alert(presentation), animated: true) + let presenter = AlertPresenter(context: self) + presenter.showAlert(presentation: presentation, animated: true) } } diff --git a/ios/MullvadVPN/Coordinators/WelcomeCoordinator.swift b/ios/MullvadVPN/Coordinators/WelcomeCoordinator.swift index 7185321f009d..97a7c8e5413b 100644 --- a/ios/MullvadVPN/Coordinators/WelcomeCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/WelcomeCoordinator.swift @@ -26,10 +26,6 @@ final class WelcomeCoordinator: Coordinator, Presentable, Presenting { navigationController } - var presentationContext: UIViewController { - navigationController - } - init( navigationController: RootContainerViewController, storePaymentManager: StorePaymentManager, @@ -102,6 +98,7 @@ extension WelcomeCoordinator: WelcomeViewControllerDelegate { ) let presentation = AlertPresentation( + id: "welcome-device-name-alert", icon: .info, message: message, buttons: [ @@ -117,7 +114,8 @@ extension WelcomeCoordinator: WelcomeViewControllerDelegate { ] ) - applicationRouter?.present(.alert(presentation), animated: true) + let presenter = AlertPresenter(context: self) + presenter.showAlert(presentation: presentation, animated: true) } func didRequestToPurchaseCredit(controller: WelcomeViewController, accountNumber: String, product: SKProduct) { @@ -150,7 +148,7 @@ extension WelcomeCoordinator: WelcomeViewControllerDelegate { ) coordinator.didCancel = { [weak self] coordinator in - guard let self else { return } + guard let self = self else { return } navigationController.popViewController(animated: true) coordinator.removeFromParent() } diff --git a/ios/MullvadVPN/SceneDelegate.swift b/ios/MullvadVPN/SceneDelegate.swift index d07345c73e47..59a3bd37f0fe 100644 --- a/ios/MullvadVPN/SceneDelegate.swift +++ b/ios/MullvadVPN/SceneDelegate.swift @@ -186,7 +186,13 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate, SettingsMigrationUIHand // MARK: - SettingsMigrationUIHandler func showMigrationError(_ error: Error, completionHandler: @escaping () -> Void) { + guard let appCoordinator else { + completionHandler() + return + } + let presentation = AlertPresentation( + id: "settings-migration-error-alert", title: NSLocalizedString( "ALERT_TITLE", tableName: "SettingsMigrationUI", @@ -205,7 +211,8 @@ class SceneDelegate: UIResponder, UIWindowSceneDelegate, SettingsMigrationUIHand ] ) - appCoordinator?.router.present(.alert(presentation), animated: true) ?? completionHandler() + let presenter = AlertPresenter(context: appCoordinator) + presenter.showAlert(presentation: presentation, animated: true) } private static func migrationErrorReason(_ error: Error) -> String { diff --git a/ios/MullvadVPN/View controllers/Account/PaymentAlertPresenter.swift b/ios/MullvadVPN/View controllers/Account/PaymentAlertPresenter.swift index 8ca856688d16..0192f3fdd306 100644 --- a/ios/MullvadVPN/View controllers/Account/PaymentAlertPresenter.swift +++ b/ios/MullvadVPN/View controllers/Account/PaymentAlertPresenter.swift @@ -10,7 +10,7 @@ import MullvadREST import Routing struct PaymentAlertPresenter { - let coordinator: Coordinator + let alertContext: any Presenting func showAlertForError( _ error: StorePaymentManagerError, @@ -18,6 +18,7 @@ struct PaymentAlertPresenter { completion: (() -> Void)? = nil ) { let presentation = AlertPresentation( + id: "payment-error-alert", title: context.errorTitle, message: error.displayErrorDescription, buttons: [ @@ -31,7 +32,8 @@ struct PaymentAlertPresenter { ] ) - coordinator.applicationRouter?.present(.alert(presentation), animated: true) + let presenter = AlertPresenter(context: alertContext) + presenter.showAlert(presentation: presentation, animated: true) } func showAlertForResponse( @@ -45,6 +47,7 @@ struct PaymentAlertPresenter { } let presentation = AlertPresentation( + id: "payment-response-alert", title: response.alertTitle(context: context), message: response.alertMessage(context: context), buttons: [ @@ -58,7 +61,8 @@ struct PaymentAlertPresenter { ] ) - coordinator.applicationRouter?.present(.alert(presentation), animated: true) + let presenter = AlertPresenter(context: alertContext) + presenter.showAlert(presentation: presentation, animated: true) } private func okButtonTextForKey(_ key: String) -> String { diff --git a/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift b/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift index 8a1993311096..9fcaac842906 100644 --- a/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift +++ b/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift @@ -7,6 +7,12 @@ // import Foundation +import Routing + +struct AlertMetadata { + let presentation: AlertPresentation + let context: Presenting +} struct AlertAction { let title: String @@ -15,7 +21,7 @@ struct AlertAction { } struct AlertPresentation: Identifiable, CustomDebugStringConvertible { - let id = UUID() + let id: String var header: String? var icon: AlertIcon? @@ -24,7 +30,7 @@ struct AlertPresentation: Identifiable, CustomDebugStringConvertible { let buttons: [AlertAction] var debugDescription: String { - id.uuidString + return id } } diff --git a/ios/MullvadVPN/View controllers/Alert/AlertPresenter.swift b/ios/MullvadVPN/View controllers/Alert/AlertPresenter.swift index dc4dbd23b090..0875e2d0fc35 100644 --- a/ios/MullvadVPN/View controllers/Alert/AlertPresenter.swift +++ b/ios/MullvadVPN/View controllers/Alert/AlertPresenter.swift @@ -9,9 +9,23 @@ import Routing struct AlertPresenter { - let coordinator: Coordinator + let context: any Presenting func showAlert(presentation: AlertPresentation, animated: Bool) { - coordinator.applicationRouter?.present(.alert(presentation), animated: animated) + context.applicationRouter?.presentAlert( + route: .alert(presentation.id), + animated: true, + metadata: AlertMetadata(presentation: presentation, context: context) + ) + } + + func dismissAlert(presentation: AlertPresentation, animated: Bool) { + context.applicationRouter?.dismiss(.alert(presentation.id), animated: animated) + } +} + +extension ApplicationRouter { + func presentAlert(route: RouteType, animated: Bool, metadata: AlertMetadata) { + present(route, animated: animated, metadata: metadata) } } diff --git a/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift b/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift index 3241a0bd0164..0a3fced6716f 100644 --- a/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift +++ b/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift @@ -1,5 +1,5 @@ // -// CustomAlertController.swift +// AlertViewController.swift // MullvadVPN // // Created by Jon Petersson on 2023-05-19. diff --git a/ios/MullvadVPN/View controllers/DeviceList/DeviceManagementViewController.swift b/ios/MullvadVPN/View controllers/DeviceList/DeviceManagementViewController.swift index e9f38ea54f1f..7675490333c2 100644 --- a/ios/MullvadVPN/View controllers/DeviceList/DeviceManagementViewController.swift +++ b/ios/MullvadVPN/View controllers/DeviceList/DeviceManagementViewController.swift @@ -89,7 +89,7 @@ class DeviceManagementViewController: UIViewController, RootContainment { completionHandler: ((Result) -> Void)? = nil ) { interactor.getDevices { [weak self] result in - guard let self else { return } + guard let self = self else { return } if let devices = result.value { setDevices(devices, animated: animateUpdates) @@ -130,7 +130,9 @@ class DeviceManagementViewController: UIViewController, RootContainment { return } - deleteDevice(identifier: device.id) { error in + deleteDevice(identifier: device.id) { [weak self] error in + guard let self = self else { return } + if let error { self.showErrorAlert( title: NSLocalizedString( @@ -158,6 +160,7 @@ class DeviceManagementViewController: UIViewController, RootContainment { private func showErrorAlert(title: String, error: Error) { let presentation = AlertPresentation( + id: "delete-device-error-alert", title: title, message: getErrorDescription(error), buttons: [ @@ -181,6 +184,7 @@ class DeviceManagementViewController: UIViewController, RootContainment { completion: @escaping (_ shouldDelete: Bool) -> Void ) { let presentation = AlertPresentation( + id: "logout-confirmation-alert", icon: .alert, message: String( format: NSLocalizedString( @@ -223,7 +227,7 @@ class DeviceManagementViewController: UIViewController, RootContainment { private func deleteDevice(identifier: String, completionHandler: @escaping (Error?) -> Void) { interactor.deleteDevice(identifier) { [weak self] completion in - guard let self else { return } + guard let self = self else { return } switch completion { case .success: diff --git a/ios/MullvadVPN/View controllers/Preferences/PreferencesViewController.swift b/ios/MullvadVPN/View controllers/Preferences/PreferencesViewController.swift index 83ab9afb0a14..2a267142aeaf 100644 --- a/ios/MullvadVPN/View controllers/Preferences/PreferencesViewController.swift +++ b/ios/MullvadVPN/View controllers/Preferences/PreferencesViewController.swift @@ -79,6 +79,7 @@ class PreferencesViewController: UITableViewController, PreferencesDataSourceDel private func showContentBlockerInfo(with message: String) { let presentation = AlertPresentation( + id: "preferences-content-blockers-alert", icon: .info, message: message, buttons: [ @@ -153,8 +154,7 @@ class PreferencesViewController: UITableViewController, PreferencesDataSourceDel case .wireGuardPorts: let portsString = humanReadablePortRepresentation( - interactor.cachedRelays?.relays.wireguard - .portRanges ?? [] + interactor.cachedRelays?.relays.wireguard.portRanges ?? [] ) message = String( diff --git a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift index b851cf828779..0911c8185e3e 100644 --- a/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift +++ b/ios/MullvadVPN/View controllers/ProblemReport/ProblemReportViewController.swift @@ -45,8 +45,9 @@ final class ProblemReportViewController: UIViewController, UITextFieldDelegate { "SUBHEAD_LABEL", tableName: "ProblemReport", value: """ - To help you more effectively, your app’s log file will be attached to this message. \ - Your data will remain secure and private, as it is anonymised before being sent over an encrypted channel. + To help you more effectively, your app’s log file will be attached to \ + this message. Your data will remain secure and private, as it is anonymised \ + before being sent over an encrypted channel. """, comment: "" ) @@ -87,8 +88,8 @@ final class ProblemReportViewController: UIViewController, UITextFieldDelegate { "DESCRIPTION_TEXTVIEW_PLACEHOLDER", tableName: "ProblemReport", value: """ - To assist you better, please write in English or Swedish and include \ - which country you are connecting from. + To assist you better, please write in English or Swedish and \ + include which country you are connecting from. """, comment: "" ) @@ -510,6 +511,7 @@ final class ProblemReportViewController: UIViewController, UITextFieldDelegate { private func presentEmptyEmailConfirmationAlert(completion: @escaping (Bool) -> Void) { let presentation = AlertPresentation( + id: "problem-report-alert", icon: .alert, message: NSLocalizedString( "EMPTY_EMAIL_ALERT_MESSAGE", diff --git a/ios/Operations/AsyncOperation.swift b/ios/Operations/AsyncOperation.swift index 638d6ff415eb..034f17f199cf 100644 --- a/ios/Operations/AsyncOperation.swift +++ b/ios/Operations/AsyncOperation.swift @@ -71,7 +71,6 @@ open class AsyncOperation: Operation { get { stateLock.lock() defer { stateLock.unlock() } - return _state } set(newState) { @@ -88,7 +87,6 @@ open class AsyncOperation: Operation { get { stateLock.lock() defer { stateLock.unlock() } - return __isCancelled } set { @@ -182,7 +180,6 @@ open class AsyncOperation: Operation { public final var conditions: [OperationCondition] { operationLock.lock() defer { operationLock.unlock() } - return _conditions } diff --git a/ios/Routing/Coordinator.swift b/ios/Routing/Coordinator.swift index c111b76ee58a..b69659ab7d34 100644 --- a/ios/Routing/Coordinator.swift +++ b/ios/Routing/Coordinator.swift @@ -100,6 +100,15 @@ public protocol Presenting: Coordinator { var presentationContext: UIViewController { get } } +extension Presenting where Self: Presentable { + /** + View controller providing modal presentation context. + */ + public var presentationContext: UIViewController { + return presentedViewController + } +} + extension Presenting { /** Present child coordinator. @@ -112,6 +121,11 @@ extension Presenting { configuration: ModalPresentationConfiguration = ModalPresentationConfiguration(), completion: (() -> Void)? = nil ) { + assert( + presentationContext.presentedViewController == nil, + "Presenting context (\(presentationContext)) is already presenting another controller." + ) + var configuration = configuration configuration.notifyInteractiveDismissal { [weak child] in diff --git a/ios/Routing/Router/ApplicationRouter.swift b/ios/Routing/Router/ApplicationRouter.swift index 957398f77451..00e65e258802 100644 --- a/ios/Routing/Router/ApplicationRouter.swift +++ b/ios/Routing/Router/ApplicationRouter.swift @@ -53,10 +53,11 @@ public final class ApplicationRouter { /** Enqueue route for presetnation. */ - public func present(_ route: RouteType, animated: Bool = true) { + public func present(_ route: RouteType, animated: Bool = true, metadata: Any? = nil) { enqueue(PendingRoute( operation: .present(route), - animated: animated + animated: animated, + metadata: metadata )) } @@ -93,6 +94,7 @@ public final class ApplicationRouter { private func presentRoute( _ route: RouteType, animated: Bool, + metadata: Any?, completion: @escaping (PendingPresentationResult) -> Void ) { /** @@ -145,7 +147,9 @@ public final class ApplicationRouter { Consult with delegate whether the route should still be presented. */ if delegate.applicationRouter(self, shouldPresent: route) { - delegate.applicationRouter(self, route: route, animated: animated) { coordinator in + let context = RoutePresentationContext(route: route, isAnimated: animated, metadata: metadata) + + delegate.applicationRouter(self, presentWithContext: context, animated: animated) { coordinator in /* Synchronize router when modal controllers are removed by swipe. */ @@ -276,7 +280,7 @@ public final class ApplicationRouter { switch pendingRoute.operation { case let .present(route): - presentRoute(route, animated: pendingRoute.animated) { result in + presentRoute(route, animated: pendingRoute.animated, metadata: pendingRoute.metadata) { result in switch result { case .success, .drop: self.finishPendingRoute(pendingRoute) diff --git a/ios/Routing/Router/ApplicationRouterDelegate.swift b/ios/Routing/Router/ApplicationRouterDelegate.swift index ecccb154158d..a98870d30383 100644 --- a/ios/Routing/Router/ApplicationRouterDelegate.swift +++ b/ios/Routing/Router/ApplicationRouterDelegate.swift @@ -19,7 +19,7 @@ public protocol ApplicationRouterDelegate: AnyObject { */ func applicationRouter( _ router: ApplicationRouter, - route: RouteType, + presentWithContext context: RoutePresentationContext, animated: Bool, completion: @escaping (Coordinator) -> Void ) diff --git a/ios/Routing/Router/ApplicationRouterTypes.swift b/ios/Routing/Router/ApplicationRouterTypes.swift index 78bef90da48c..7be142adaf5d 100644 --- a/ios/Routing/Router/ApplicationRouterTypes.swift +++ b/ios/Routing/Router/ApplicationRouterTypes.swift @@ -11,9 +11,16 @@ import Foundation /** Struct describing a routing request for presentation or dismissal. */ -struct PendingRoute: Equatable { +struct PendingRoute { var operation: RouteOperation var animated: Bool + var metadata: Any? +} + +extension PendingRoute: Equatable { + static func == (lhs: PendingRoute, rhs: PendingRoute) -> Bool { + lhs.operation == rhs.operation + } } /** @@ -161,6 +168,26 @@ public struct RouteDismissalContext { public var isAnimated: Bool } +/** + Struct holding information used by delegate to perform presentation of a specific route. + */ +public struct RoutePresentationContext { + /** + Route that's being presented. + */ + public var route: RouteType + + /** + Whether transition is animated. + */ + public var isAnimated: Bool + + /** + Metadata associated with the route. + */ + public var metadata: Any? +} + /** Struct holding information used by delegate to perform sub-navigation of the route in subject. */ diff --git a/ios/RoutingTests/RouterBlockDelegate.swift b/ios/RoutingTests/RouterBlockDelegate.swift index b1c1f5890837..977454b7e42b 100644 --- a/ios/RoutingTests/RouterBlockDelegate.swift +++ b/ios/RoutingTests/RouterBlockDelegate.swift @@ -10,7 +10,7 @@ import Foundation import Routing class RouterBlockDelegate: ApplicationRouterDelegate { - var handleRoute: ((RouteType, Bool, (Coordinator) -> Void) -> Void)? + var handleRoute: ((RoutePresentationContext, Bool, (Coordinator) -> Void) -> Void)? var handleDismiss: ((RouteDismissalContext, () -> Void) -> Void)? var shouldPresent: ((RouteType) -> Bool)? var shouldDismiss: ((RouteDismissalContext) -> Bool)? @@ -18,11 +18,11 @@ class RouterBlockDelegate: ApplicationRouterDelegat func applicationRouter( _ router: ApplicationRouter, - route: RouteType, + presentWithContext context: RoutePresentationContext, animated: Bool, completion: @escaping (Coordinator) -> Void ) { - handleRoute?(route, animated, completion) ?? completion(Coordinator()) + handleRoute?(context, animated, completion) ?? completion(Coordinator()) } func applicationRouter(