From 660b9dfb7e57bfb1d476e0b2efda210e85565ef5 Mon Sep 17 00:00:00 2001 From: mojganii Date: Fri, 12 Apr 2024 11:51:09 +0200 Subject: [PATCH] Move tracking unsaved changes into coordinator --- ios/MullvadVPN.xcodeproj/project.pbxproj | 8 +- ...> InterceptableNavigationController.swift} | 6 +- .../CustomListViewController.swift | 101 +----------------- .../EditCustomListCoordinator.swift | 91 +++++++++++++++- .../ListCustomListCoordinator.swift | 6 ++ .../Coordinators/LocationCoordinator.swift | 2 +- 6 files changed, 107 insertions(+), 107 deletions(-) rename ios/MullvadVPN/Classes/{InterceptibleNavigationController.swift => InterceptableNavigationController.swift} (88%) diff --git a/ios/MullvadVPN.xcodeproj/project.pbxproj b/ios/MullvadVPN.xcodeproj/project.pbxproj index 15d379e53fb4..cc9749a4268f 100644 --- a/ios/MullvadVPN.xcodeproj/project.pbxproj +++ b/ios/MullvadVPN.xcodeproj/project.pbxproj @@ -540,7 +540,7 @@ 7A6F2FAB2AFD3097006D0856 /* CustomDNSCellFactory.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6F2FAA2AFD3097006D0856 /* CustomDNSCellFactory.swift */; }; 7A6F2FAD2AFD3DA7006D0856 /* CustomDNSViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6F2FAC2AFD3DA7006D0856 /* CustomDNSViewController.swift */; }; 7A6F2FAF2AFE36E7006D0856 /* VPNSettingsInfoButtonItem.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A6F2FAE2AFE36E7006D0856 /* VPNSettingsInfoButtonItem.swift */; }; - 7A7907332BC0280A00B61F81 /* InterceptibleNavigationController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7907322BC0280A00B61F81 /* InterceptibleNavigationController.swift */; }; + 7A7907332BC0280A00B61F81 /* InterceptableNavigationController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7907322BC0280A00B61F81 /* InterceptableNavigationController.swift */; }; 7A7AD28D29DC677800480EF1 /* FirstTimeLaunch.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */; }; 7A818F1F29F0305800C7F0F4 /* RootConfiguration.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A818F1E29F0305800C7F0F4 /* RootConfiguration.swift */; }; 7A83A0C62B29A750008B5CE7 /* APIAccessMethodsTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 7A83A0C52B29A750008B5CE7 /* APIAccessMethodsTests.swift */; }; @@ -1806,7 +1806,7 @@ 7A6F2FAA2AFD3097006D0856 /* CustomDNSCellFactory.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomDNSCellFactory.swift; sourceTree = ""; }; 7A6F2FAC2AFD3DA7006D0856 /* CustomDNSViewController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = CustomDNSViewController.swift; sourceTree = ""; }; 7A6F2FAE2AFE36E7006D0856 /* VPNSettingsInfoButtonItem.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VPNSettingsInfoButtonItem.swift; sourceTree = ""; }; - 7A7907322BC0280A00B61F81 /* InterceptibleNavigationController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InterceptibleNavigationController.swift; sourceTree = ""; }; + 7A7907322BC0280A00B61F81 /* InterceptableNavigationController.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = InterceptableNavigationController.swift; sourceTree = ""; }; 7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = FirstTimeLaunch.swift; sourceTree = ""; }; 7A818F1E29F0305800C7F0F4 /* RootConfiguration.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RootConfiguration.swift; sourceTree = ""; }; 7A83A0C52B29A750008B5CE7 /* APIAccessMethodsTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = APIAccessMethodsTests.swift; sourceTree = ""; }; @@ -2696,7 +2696,7 @@ 58138E60294871C600684F0C /* DeviceDataThrottling.swift */, 7A7AD28C29DC677800480EF1 /* FirstTimeLaunch.swift */, 582AE30F2440A6CA00E6733A /* InputTextFormatter.swift */, - 7A7907322BC0280A00B61F81 /* InterceptibleNavigationController.swift */, + 7A7907322BC0280A00B61F81 /* InterceptableNavigationController.swift */, 58DFF7D12B0256A300F864E0 /* MarkdownStylingOptions.swift */, 58CC40EE24A601900019D96E /* ObserverList.swift */, ); @@ -5466,7 +5466,7 @@ 7A42DEC92A05164100B209BE /* SettingsInputCell.swift in Sources */, 5803B4B22940A48700C23744 /* TunnelStore.swift in Sources */, 586A950F29012BEE007BAF2B /* AddressCacheTracker.swift in Sources */, - 7A7907332BC0280A00B61F81 /* InterceptibleNavigationController.swift in Sources */, + 7A7907332BC0280A00B61F81 /* InterceptableNavigationController.swift in Sources */, F02F41A02B9723AF00625A4F /* AddLocationsViewController.swift in Sources */, 587B753D2666468F00DEF7E9 /* NotificationController.swift in Sources */, ); diff --git a/ios/MullvadVPN/Classes/InterceptibleNavigationController.swift b/ios/MullvadVPN/Classes/InterceptableNavigationController.swift similarity index 88% rename from ios/MullvadVPN/Classes/InterceptibleNavigationController.swift rename to ios/MullvadVPN/Classes/InterceptableNavigationController.swift index 69b0c9f428cb..e85420f69070 100644 --- a/ios/MullvadVPN/Classes/InterceptibleNavigationController.swift +++ b/ios/MullvadVPN/Classes/InterceptableNavigationController.swift @@ -1,5 +1,5 @@ // -// InterceptibleNavigationController.swift +// InterceptableNavigationController.swift // MullvadVPN // // Created by Jon Petersson on 2024-04-05. @@ -8,12 +8,13 @@ import UIKit -class InterceptibleNavigationController: CustomNavigationController { +class InterceptableNavigationController: CustomNavigationController { var shouldPopViewController: ((UIViewController) -> Bool)? var shouldPopToViewController: ((UIViewController) -> Bool)? // Called when popping the topmost view controller in the stack, eg. by pressing a navigation // bar back button. + @discardableResult override func popViewController(animated: Bool) -> UIViewController? { guard let viewController = viewControllers.last else { return nil } @@ -26,6 +27,7 @@ class InterceptibleNavigationController: CustomNavigationController { // Called when popping to a specific view controller, eg. by long pressing a navigation bar // back button (revealing a navigation menu) and selecting a destination view controller. + @discardableResult override func popToViewController(_ viewController: UIViewController, animated: Bool) -> [UIViewController]? { if shouldPopToViewController?(viewController) ?? true { return super.popToViewController(viewController, animated: animated) diff --git a/ios/MullvadVPN/Coordinators/CustomLists/CustomListViewController.swift b/ios/MullvadVPN/Coordinators/CustomLists/CustomListViewController.swift index be6f39089f0e..a3a7518af8dc 100644 --- a/ios/MullvadVPN/Coordinators/CustomLists/CustomListViewController.swift +++ b/ios/MullvadVPN/Coordinators/CustomLists/CustomListViewController.swift @@ -31,7 +31,7 @@ class CustomListViewController: UIViewController { return interactor.fetchAll().first(where: { $0.id == subject.value.id }) } - private var hasUnsavedChanges: Bool { + var hasUnsavedChanges: Bool { persistedCustomList != subject.value.customList } @@ -83,12 +83,12 @@ class CustomListViewController: UIViewController { override func viewDidLoad() { super.viewDidLoad() + navigationItem.rightBarButtonItem = saveBarButton view.directionalLayoutMargins = UIMetrics.contentLayoutMargins view.backgroundColor = .secondaryColor isModalInPresentation = true addSubviews() - configureNavigationItem() configureDataSource() configureTableView() @@ -98,39 +98,6 @@ class CustomListViewController: UIViewController { }.store(in: &cancellables) } - private func configureNavigationItem() { - if let navigationController = navigationController as? InterceptibleNavigationController { - interceptNavigation(navigationController) - } - - navigationController?.interactivePopGestureRecognizer?.delegate = self - navigationItem.rightBarButtonItem = saveBarButton - } - - private func interceptNavigation(_ navigationController: InterceptibleNavigationController) { - navigationController.shouldPopViewController = { [weak self] viewController in - guard - let self, - viewController is Self, - hasUnsavedChanges - else { return true } - - self.onUnsavedChanges() - return false - } - - navigationController.shouldPopToViewController = { [weak self] viewController in - guard - let self, - viewController is ListCustomListViewController, - hasUnsavedChanges - else { return true } - - self.onUnsavedChanges() - return false - } - } - private func configureTableView() { tableView.delegate = dataSourceConfiguration tableView.backgroundColor = .secondaryColor @@ -232,68 +199,4 @@ class CustomListViewController: UIViewController { alertPresenter.showAlert(presentation: presentation, animated: true) } - - @objc private func onUnsavedChanges() { - let message = NSMutableAttributedString( - markdownString: NSLocalizedString( - "CUSTOM_LISTS_UNSAVED_CHANGES_PROMPT", - tableName: "CustomLists", - value: "You have unsaved changes.", - comment: "" - ), - options: MarkdownStylingOptions(font: .preferredFont(forTextStyle: .body)) - ) - - let presentation = AlertPresentation( - id: "api-custom-lists-unsaved-changes-alert", - icon: .alert, - attributedMessage: message, - buttons: [ - AlertAction( - title: NSLocalizedString( - "CUSTOM_LISTS_DISCARD_CHANGES_BUTTON", - tableName: "CustomLists", - value: "Discard changes", - comment: "" - ), - style: .destructive, - handler: { - // Reset subject/view model to no longer having unsaved changes. - if let persistedCustomList = self.persistedCustomList { - self.subject.value.update(with: persistedCustomList) - } - self.delegate?.customListDidSave(self.subject.value.customList) - } - ), - AlertAction( - title: NSLocalizedString( - "CUSTOM_LISTS_BACK_TO_EDITING_BUTTON", - tableName: "CustomLists", - value: "Back to editing", - comment: "" - ), - style: .default - ), - ] - ) - - alertPresenter.showAlert(presentation: presentation, animated: true) - } -} - -extension CustomListViewController: UIGestureRecognizerDelegate { - // For some reason, intercepting `popViewController(animated: Bool)` in `InterceptibleNavigationController` - // by SWIPING back leads to weird behaviour where subsequent navigation seem to happen systemwise but not - // UI-wise. This leads to the UI freezing up, and the only remedy is to restart the app. - // - // To get around this issue we can intercept the back swipe gesture and manually perform the transition - // instead, thereby bypassing the inner mechanisms that seem to go out of sync. - func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { - guard gestureRecognizer == navigationController?.interactivePopGestureRecognizer else { - return true - } - - navigationController?.popViewController(animated: true) - return false - } } diff --git a/ios/MullvadVPN/Coordinators/CustomLists/EditCustomListCoordinator.swift b/ios/MullvadVPN/Coordinators/CustomLists/EditCustomListCoordinator.swift index 8cc441738ae9..c3a8bd3992c0 100644 --- a/ios/MullvadVPN/Coordinators/CustomLists/EditCustomListCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/CustomLists/EditCustomListCoordinator.swift @@ -21,12 +21,16 @@ class EditCustomListCoordinator: Coordinator, Presentable, Presenting { let customList: CustomList let nodes: [LocationNode] let subject: CurrentValueSubject + private lazy var alertPresenter: AlertPresenter = { + AlertPresenter(context: self) + }() var presentedViewController: UIViewController { navigationController } var didFinish: ((EditCustomListCoordinator, FinishAction, CustomList) -> Void)? + var didCancel: ((EditCustomListCoordinator) -> Void)? init( navigationController: UINavigationController, @@ -50,7 +54,7 @@ class EditCustomListCoordinator: Coordinator, Presentable, Presenting { let controller = CustomListViewController( interactor: customListInteractor, subject: subject, - alertPresenter: AlertPresenter(context: self) + alertPresenter: alertPresenter ) controller.delegate = self @@ -61,7 +65,76 @@ class EditCustomListCoordinator: Coordinator, Presentable, Presenting { comment: "" ) + navigationController.interactivePopGestureRecognizer?.delegate = self navigationController.pushViewController(controller, animated: true) + + guard let interceptableNavigationController = navigationController as? InterceptableNavigationController else { + return + } + + interceptableNavigationController.shouldPopViewController = { [weak self] viewController in + guard + let self, + let customListViewController = viewController as? CustomListViewController, + customListViewController.hasUnsavedChanges + else { return true } + presentUnsavedChangesDialog() + return false + } + + interceptableNavigationController.shouldPopToViewController = { [weak self] viewController in + guard + let self, + let customListViewController = viewController as? CustomListViewController, + customListViewController.hasUnsavedChanges + else { return true } + + presentUnsavedChangesDialog() + return false + } + } + + private func presentUnsavedChangesDialog() { + let message = NSMutableAttributedString( + markdownString: NSLocalizedString( + "CUSTOM_LISTS_UNSAVED_CHANGES_PROMPT", + tableName: "CustomLists", + value: "You have unsaved changes.", + comment: "" + ), + options: MarkdownStylingOptions(font: .preferredFont(forTextStyle: .body)) + ) + + let presentation = AlertPresentation( + id: "api-custom-lists-unsaved-changes-alert", + icon: .alert, + attributedMessage: message, + buttons: [ + AlertAction( + title: NSLocalizedString( + "CUSTOM_LISTS_DISCARD_CHANGES_BUTTON", + tableName: "CustomLists", + value: "Discard changes", + comment: "" + ), + style: .destructive, + handler: { + self.didCancel?(self) + } + ), + AlertAction( + title: NSLocalizedString( + "CUSTOM_LISTS_BACK_TO_EDITING_BUTTON", + tableName: "CustomLists", + value: "Back to editing", + comment: "" + ), + style: .default + ), + ] + ) + + alertPresenter.showAlert(presentation: presentation, animated: true) } } @@ -90,3 +163,19 @@ extension EditCustomListCoordinator: CustomListViewControllerDelegate { addChild(coordinator) } } + +extension EditCustomListCoordinator: UIGestureRecognizerDelegate { + // For some reason, intercepting `popViewController(animated: Bool)` in `InterceptibleNavigationController` + // by SWIPING back leads to weird behaviour where subsequent navigation seem to happen systemwise but not + // UI-wise. This leads to the UI freezing up, and the only remedy is to restart the app. + // + // To get around this issue we can intercept the back swipe gesture and manually perform the transition + // instead, thereby bypassing the inner mechanisms that seem to go out of sync. + func gestureRecognizerShouldBegin(_ gestureRecognizer: UIGestureRecognizer) -> Bool { + guard gestureRecognizer == navigationController.interactivePopGestureRecognizer else { + return true + } + navigationController.popViewController(animated: true) + return false + } +} diff --git a/ios/MullvadVPN/Coordinators/CustomLists/ListCustomListCoordinator.swift b/ios/MullvadVPN/Coordinators/CustomLists/ListCustomListCoordinator.swift index 2b238dd1e5de..851af36dabfa 100644 --- a/ios/MullvadVPN/Coordinators/CustomLists/ListCustomListCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/CustomLists/ListCustomListCoordinator.swift @@ -65,6 +65,12 @@ class ListCustomListCoordinator: Coordinator, Presentable, Presenting { self.updateRelayConstraints(for: action, in: list) } + coordinator.didCancel = { [weak self] editCustomListCoordinator in + guard let self else { return } + popToList() + editCustomListCoordinator.removeFromParent() + } + coordinator.start() addChild(coordinator) } diff --git a/ios/MullvadVPN/Coordinators/LocationCoordinator.swift b/ios/MullvadVPN/Coordinators/LocationCoordinator.swift index 30fe23d8e3fa..b2b10f06119e 100644 --- a/ios/MullvadVPN/Coordinators/LocationCoordinator.swift +++ b/ios/MullvadVPN/Coordinators/LocationCoordinator.swift @@ -147,7 +147,7 @@ class LocationCoordinator: Coordinator, Presentable, Presenting { private func showEditCustomLists(nodes: [LocationNode]) { let coordinator = ListCustomListCoordinator( - navigationController: InterceptibleNavigationController(), + navigationController: InterceptableNavigationController(), interactor: CustomListInteractor(repository: customListRepository), tunnelManager: tunnelManager, nodes: nodes