Skip to content

Commit

Permalink
Refactor alerts to be presented on their respective coordinator
Browse files Browse the repository at this point in the history
  • Loading branch information
Jon Petersson committed Sep 12, 2023
1 parent f928d2d commit 3a71f8c
Show file tree
Hide file tree
Showing 30 changed files with 229 additions and 152 deletions.
2 changes: 2 additions & 0 deletions ios/.swiftlint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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
}
15 changes: 8 additions & 7 deletions ios/MullvadVPN/Classes/AppRoutes.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import UIKit
Enum type describing groups of routes. Each group is a modal layer with horizontal navigation
inside with exception where primary navigation is a part of root controller on iPhone.
*/
enum AppRouteGroup: String, AppRouteGroupProtocol {
enum AppRouteGroup: AppRouteGroupProtocol {
/**
Primary horizontal navigation group.
*/
Expand All @@ -40,9 +40,9 @@ enum AppRouteGroup: String, AppRouteGroupProtocol {
case changelog

/**
Alert group.
Alert group. Alert id should match the id of the alert being contained.
*/
case alert
case alert(_ alertId: String)

var isModal: Bool {
switch self {
Expand Down Expand Up @@ -92,9 +92,10 @@ enum AppRoute: AppRouteProtocol {
case changelog

/**
Alert route.
Alert route. Alert id must be a unique string in order to produce a unique route
that distinguishes between different kinds of alerts.
*/
case alert(AlertPresentation)
case alert(_ alertId: String)

/**
Routes that are part of primary horizontal navigation group.
Expand Down Expand Up @@ -130,8 +131,8 @@ enum AppRoute: AppRouteProtocol {
return .account
case .settings:
return .settings
case .alert:
return .alert
case let .alert(id):
return .alert(id)
}
}
}
23 changes: 14 additions & 9 deletions ios/MullvadVPN/Coordinators/AccountCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -29,10 +29,6 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting {
navigationController
}

var presentationContext: UIViewController {
navigationController
}

var didFinish: ((AccountCoordinator, AccountDismissReason) -> Void)?

init(
Expand All @@ -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
Expand Down Expand Up @@ -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() {
Expand All @@ -164,6 +167,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting {
)

let presentation = AlertPresentation(
id: "account-device-info-alert",
message: message,
buttons: [AlertAction(
title: NSLocalizedString(
Expand All @@ -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)
}
}
38 changes: 19 additions & 19 deletions ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

/**
Expand Down Expand Up @@ -120,11 +119,11 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo

func applicationRouter(
_ router: ApplicationRouter<RouteType>,
route: AppRoute,
presentWithContext context: RoutePresentationContext<RouteType>,
animated: Bool,
completion: @escaping (Coordinator) -> Void
) {
switch route {
switch context.route {
case .account:
presentAccount(animated: animated, completion: completion)

Expand Down Expand Up @@ -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)
}
}

Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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)
Expand All @@ -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()

Expand Down Expand Up @@ -642,19 +637,24 @@ final class ApplicationCoordinator: Coordinator, Presenting, RootContainerViewCo
}

private func presentAlert(
presentation: AlertPresentation,
animated: Bool,
context: RoutePresentationContext<RouteType>,
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)
}
}
Expand Down Expand Up @@ -989,13 +989,13 @@ fileprivate extension AppPreferencesDataSource {
mutating func markChangeLogSeen() {
self.lastSeenChangeLogVersion = Bundle.main.shortVersion
}

// swiftlint:disable:next file_length
}

private protocol Poppable: Presentable {
func popFromNavigationStack(
animated: Bool,
completion: () -> Void
)

// swiftlint:disable:next file_length
}
6 changes: 4 additions & 2 deletions ios/MullvadVPN/Coordinators/InAppPurchaseCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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: [
Expand All @@ -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)
}
}
}
Expand Down
11 changes: 8 additions & 3 deletions ios/MullvadVPN/Coordinators/LoginCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -22,6 +22,10 @@ final class LoginCoordinator: Coordinator, DeviceManagementViewControllerDelegat
var didFinish: ((LoginCoordinator) -> Void)?
var didCreateAccount: (() -> Void)?

var presentationContext: UIViewController {
navigationController
}

let navigationController: RootContainerViewController

init(
Expand Down Expand Up @@ -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:
Expand Down
8 changes: 6 additions & 2 deletions ios/MullvadVPN/Coordinators/OutOfTimeCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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?

Expand All @@ -42,7 +46,7 @@ class OutOfTimeCoordinator: Coordinator, OutOfTimeViewControllerDelegate {

let controller = OutOfTimeViewController(
interactor: interactor,
errorPresenter: PaymentAlertPresenter(coordinator: self)
errorPresenter: PaymentAlertPresenter(alertContext: self)
)

controller.delegate = self
Expand Down
8 changes: 2 additions & 6 deletions ios/MullvadVPN/Coordinators/SettingsCoordinator.swift
Original file line number Diff line number Diff line change
Expand Up @@ -32,10 +32,6 @@ final class SettingsCoordinator: Coordinator, Presentable, Presenting, SettingsV
navigationController
}

var presentationContext: UIViewController {
navigationController
}

var willNavigate: ((
_ coordinator: SettingsCoordinator,
_ from: SettingsNavigationRoute?,
Expand Down Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 3a71f8c

Please sign in to comment.