From e2ce740995aaf7175852475daba1fd73c0a54a0d Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Wed, 23 Aug 2023 15:38:12 +0200 Subject: [PATCH 1/2] Coordinate alert presentation by integrating it into routing system --- .../Coordinators/AccountCoordinator.swift | 1 - .../Coordinators/ApplicationCoordinator.swift | 7 +++ .../Operations/PresentAlertOperation.swift | 59 +++++++++++++++++++ 3 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 ios/MullvadVPN/Operations/PresentAlertOperation.swift diff --git a/ios/MullvadVPN/Coordinators/AccountCoordinator.swift b/ios/MullvadVPN/Coordinators/AccountCoordinator.swift index 0338d7427bf1..4202a0076c4e 100644 --- a/ios/MullvadVPN/Coordinators/AccountCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/AccountCoordinator.swift @@ -136,7 +136,6 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting { let presentation = AlertPresentation( id: "account-logout-alert", icon: .spinner, - message: nil, buttons: [] ) diff --git a/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift b/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift index 7f6cb1d47435..e6ede7a06c11 100644 --- a/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift @@ -1008,3 +1008,10 @@ fileprivate extension AppPreferencesDataSource { // swiftlint:disable:next file_length } + +private protocol Poppable: Presentable { + func popFromNavigationStack( + animated: Bool, + completion: () -> Void + ) +} diff --git a/ios/MullvadVPN/Operations/PresentAlertOperation.swift b/ios/MullvadVPN/Operations/PresentAlertOperation.swift new file mode 100644 index 000000000000..cd6f5b02c2f4 --- /dev/null +++ b/ios/MullvadVPN/Operations/PresentAlertOperation.swift @@ -0,0 +1,59 @@ +// +// PresentAlertOperation.swift +// MullvadVPN +// +// Created by pronebird on 06/09/2021. +// Copyright © 2021 Mullvad VPN AB. All rights reserved. +// + +import Operations +import UIKit + +final class PresentAlertOperation: AsyncOperation { + private let alertController: AlertViewController + private let presentingController: UIViewController + private let presentCompletion: (() -> Void)? + + init( + alertController: AlertViewController, + presentingController: UIViewController, + presentCompletion: (() -> Void)? = nil + ) { + self.alertController = alertController + self.presentingController = presentingController + self.presentCompletion = presentCompletion + + super.init(dispatchQueue: .main) + } + + override func operationDidCancel() { + // Guard against trying to dismiss the alert when operation hasn't started yet. + guard isExecuting else { return } + + // Guard against dismissing controller during transition. + if !alertController.isBeingPresented, !alertController.isBeingDismissed { + dismissAndFinish() + } + } + + override func main() { + alertController.didDismiss = { [weak self] in + self?.finish() + } + + presentingController.present(alertController, animated: true) { + self.presentCompletion?() + + // Alert operation was cancelled during transition? + if self.isCancelled { + self.dismissAndFinish() + } + } + } + + private func dismissAndFinish() { + alertController.dismiss(animated: false) { + self.finish() + } + } +} From b3b58ec8c49482d0bb6535e52544cfd8327fb766 Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Tue, 29 Aug 2023 11:16:56 +0200 Subject: [PATCH 2/2] Update screenshots test to work with updated UI --- ios/MullvadVPN.xcodeproj/project.pbxproj | 2 +- .../Coordinators/AccountCoordinator.swift | 1 + .../Coordinators/AlertCoordinator.swift | 7 ++- .../Coordinators/ApplicationCoordinator.swift | 7 --- .../Coordinators/ChangeLogCoordinator.swift | 1 + .../Operations/PresentAlertOperation.swift | 59 ------------------- .../Alert/AlertPresentation.swift | 1 + .../Alert/AlertViewController.swift | 5 +- .../MullvadVPNScreenshots.swift | 1 + 9 files changed, 14 insertions(+), 70 deletions(-) delete mode 100644 ios/MullvadVPN/Operations/PresentAlertOperation.swift diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index 16fa53b8293d..03de4dd1ab9a 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -2584,9 +2584,9 @@ 7A83C3FC2A55B39500DFB83A /* TestPlans */ = { isa = PBXGroup; children = ( - 7A02D4EA2A9CEC7A00C19E31 /* MullvadVPNScreenshots.xctestplan */, 7A83C3FE2A55B72E00DFB83A /* MullvadVPNApp.xctestplan */, 7A83C4002A55B81A00DFB83A /* MullvadVPNCI.xctestplan */, + 7A02D4EA2A9CEC7A00C19E31 /* MullvadVPNScreenshots.xctestplan */, ); path = TestPlans; sourceTree = ""; diff --git a/ios/MullvadVPN/Coordinators/AccountCoordinator.swift b/ios/MullvadVPN/Coordinators/AccountCoordinator.swift index 4202a0076c4e..0338d7427bf1 100644 --- a/ios/MullvadVPN/Coordinators/AccountCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/AccountCoordinator.swift @@ -136,6 +136,7 @@ final class AccountCoordinator: Coordinator, Presentable, Presenting { let presentation = AlertPresentation( id: "account-logout-alert", icon: .spinner, + message: nil, buttons: [] ) diff --git a/ios/MullvadVPN/Coordinators/AlertCoordinator.swift b/ios/MullvadVPN/Coordinators/AlertCoordinator.swift index 3dbdc59f7aa5..c06691e22230 100644 --- a/ios/MullvadVPN/Coordinators/AlertCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/AlertCoordinator.swift @@ -39,7 +39,12 @@ final class AlertCoordinator: Coordinator, Presentable { } presentation.buttons.forEach { action in - alertController.addAction(title: action.title, style: action.style, handler: action.handler) + alertController.addAction( + title: action.title, + style: action.style, + accessibilityId: action.accessibilityID, + handler: action.handler + ) } } } diff --git a/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift b/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift index e6ede7a06c11..7f6cb1d47435 100644 --- a/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift @@ -1008,10 +1008,3 @@ fileprivate extension AppPreferencesDataSource { // swiftlint:disable:next file_length } - -private protocol Poppable: Presentable { - func popFromNavigationStack( - animated: Bool, - completion: () -> Void - ) -} diff --git a/ios/MullvadVPN/Coordinators/ChangeLogCoordinator.swift b/ios/MullvadVPN/Coordinators/ChangeLogCoordinator.swift index ae86829b1211..e55a31b13db7 100644 --- a/ios/MullvadVPN/Coordinators/ChangeLogCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/ChangeLogCoordinator.swift @@ -38,6 +38,7 @@ final class ChangeLogCoordinator: Coordinator, Presentable { comment: "" ), style: .default, + accessibilityId: "OkButton", handler: { [weak self] in guard let self else { return } didFinish?(self) diff --git a/ios/MullvadVPN/Operations/PresentAlertOperation.swift b/ios/MullvadVPN/Operations/PresentAlertOperation.swift deleted file mode 100644 index cd6f5b02c2f4..000000000000 --- a/ios/MullvadVPN/Operations/PresentAlertOperation.swift +++ /dev/null @@ -1,59 +0,0 @@ -// -// PresentAlertOperation.swift -// MullvadVPN -// -// Created by pronebird on 06/09/2021. -// Copyright © 2021 Mullvad VPN AB. All rights reserved. -// - -import Operations -import UIKit - -final class PresentAlertOperation: AsyncOperation { - private let alertController: AlertViewController - private let presentingController: UIViewController - private let presentCompletion: (() -> Void)? - - init( - alertController: AlertViewController, - presentingController: UIViewController, - presentCompletion: (() -> Void)? = nil - ) { - self.alertController = alertController - self.presentingController = presentingController - self.presentCompletion = presentCompletion - - super.init(dispatchQueue: .main) - } - - override func operationDidCancel() { - // Guard against trying to dismiss the alert when operation hasn't started yet. - guard isExecuting else { return } - - // Guard against dismissing controller during transition. - if !alertController.isBeingPresented, !alertController.isBeingDismissed { - dismissAndFinish() - } - } - - override func main() { - alertController.didDismiss = { [weak self] in - self?.finish() - } - - presentingController.present(alertController, animated: true) { - self.presentCompletion?() - - // Alert operation was cancelled during transition? - if self.isCancelled { - self.dismissAndFinish() - } - } - } - - private func dismissAndFinish() { - alertController.dismiss(animated: false) { - self.finish() - } - } -} diff --git a/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift b/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift index 9fcaac842906..c09d0e1e72e5 100644 --- a/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift +++ b/ios/MullvadVPN/View controllers/Alert/AlertPresentation.swift @@ -17,6 +17,7 @@ struct AlertMetadata { struct AlertAction { let title: String let style: AlertActionStyle + var accessibilityID: String? var handler: (() -> Void)? } diff --git a/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift b/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift index 0a3fced6716f..bac96175b2a8 100644 --- a/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift +++ b/ios/MullvadVPN/View controllers/Alert/AlertViewController.swift @@ -120,14 +120,15 @@ class AlertViewController: UIViewController { } } - func addAction(title: String, style: AlertActionStyle, handler: (() -> Void)? = nil) { + func addAction(title: String, style: AlertActionStyle, accessibilityId: String?, handler: (() -> Void)? = nil) { // The presence of a button should reset any custom button margin to default. containerView.directionalLayoutMargins.bottom = UIMetrics.CustomAlert.containerMargins.bottom let button = AppButton(style: style.buttonStyle) - button.addTarget(self, action: #selector(didTapButton), for: .touchUpInside) button.setTitle(title, for: .normal) + button.accessibilityIdentifier = accessibilityId + button.addTarget(self, action: #selector(didTapButton), for: .touchUpInside) containerView.addArrangedSubview(button) handler.flatMap { handlers[button] = $0 } diff --git a/ios/MullvadVPNScreenshots/MullvadVPNScreenshots.swift b/ios/MullvadVPNScreenshots/MullvadVPNScreenshots.swift index 4621728c32f9..0e050b872ddc 100644 --- a/ios/MullvadVPNScreenshots/MullvadVPNScreenshots.swift +++ b/ios/MullvadVPNScreenshots/MullvadVPNScreenshots.swift @@ -71,6 +71,7 @@ class MullvadVPNScreenshots: XCTestCase { if cityCell.exists { cityCell.tap() } else { + _ = countryCell.buttons["CollapseButton"].waitForExistence(timeout: 5) countryCell.buttons["CollapseButton"].tap() cityCell.tap() }