From 326c5b823d822f21ce25af29484960828f8f68be Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 20:10:59 +0200 Subject: [PATCH 1/4] Observe wizard creation via notifications Flow is too scattered to safely maintain delegation. --- Passepartout-iOS/AppDelegate.swift | 8 ------ .../ImportedHostsViewController.swift | 3 --- .../Organizer/OrganizerViewController.swift | 25 ++++++++++++------- .../Organizer/WizardHostViewController.swift | 9 ++++--- .../WizardProviderViewController.swift | 9 ++++--- Passepartout/Sources/Model/Wizard.swift | 10 +++++--- 6 files changed, 32 insertions(+), 32 deletions(-) diff --git a/Passepartout-iOS/AppDelegate.swift b/Passepartout-iOS/AppDelegate.swift index a7723c9d..9ded42e9 100644 --- a/Passepartout-iOS/AppDelegate.swift +++ b/Passepartout-iOS/AppDelegate.swift @@ -126,14 +126,6 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele wizard.parsedFile = parsedFile wizard.removesConfigurationOnCancel = true - // best effort to delegate to main vc - let split = root as? UISplitViewController - let master = split?.viewControllers.first as? UINavigationController - master?.viewControllers.forEach { - if let organizerVC = $0 as? OrganizerViewController { - wizard.delegate = organizerVC - } - } nav.modalPresentationStyle = .formSheet root.present(nav, animated: true, completion: nil) } diff --git a/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift b/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift index 1f9b2db5..2188141b 100644 --- a/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/ImportedHostsViewController.swift @@ -34,8 +34,6 @@ class ImportedHostsViewController: UITableViewController { private var parsedFile: ParsedFile? - weak var wizardDelegate: WizardDelegate? - override func viewDidLoad() { super.viewDidLoad() @@ -104,7 +102,6 @@ class ImportedHostsViewController: UITableViewController { return } wizard.parsedFile = parsedFile - wizard.delegate = wizardDelegate // retain back button wizard.navigationItem.leftBarButtonItem = nil diff --git a/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift b/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift index e0cd4f12..2dcfcd11 100644 --- a/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift @@ -69,6 +69,10 @@ class OrganizerViewController: UITableViewController, TableModelHost { } // MARK: UIViewController + + deinit { + NotificationCenter.default.removeObserver(self) + } override func awakeFromNib() { super.awakeFromNib() @@ -90,6 +94,8 @@ class OrganizerViewController: UITableViewController, TableModelHost { } service.delegate = self + + NotificationCenter.default.addObserver(self, selector: #selector(wizardDidCreate(notification:)), name: .WizardDidCreate, object: nil) } override func viewDidAppear(_ animated: Bool) { @@ -134,13 +140,8 @@ class OrganizerViewController: UITableViewController, TableModelHost { assert(selectedProfile != nil, "No selected profile") vc.profile = selectedProfile - } else if let vc = destination as? Wizard { - if let providerVC = vc as? WizardProviderViewController { - providerVC.availableNames = availableProviderNames ?? [] - } - vc.delegate = self - } else if let vc = destination as? ImportedHostsViewController { - vc.wizardDelegate = self + } else if let providerVC = destination as? WizardProviderViewController { + providerVC.availableNames = availableProviderNames ?? [] } } @@ -436,8 +437,14 @@ extension OrganizerViewController: ConnectionServiceDelegate { } } -extension OrganizerViewController: WizardDelegate { - func wizard(didCreate profile: ConnectionProfile, withCredentials credentials: Credentials) { +extension OrganizerViewController { + @objc private func wizardDidCreate(notification: Notification) { + guard let profile = notification.userInfo?[WizardCreationKey.profile] as? ConnectionProfile, + let credentials = notification.userInfo?[WizardCreationKey.credentials] as? Credentials else { + + fatalError("WizardDidCreate notification must post profile and credentials") + } + service.addOrReplaceProfile(profile, credentials: credentials) TransientStore.shared.serialize() // add diff --git a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift index f6919c0f..e08da738 100644 --- a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift @@ -29,7 +29,7 @@ import SwiftyBeaver private let log = SwiftyBeaver.self -class WizardHostViewController: UITableViewController, TableModelHost, Wizard { +class WizardHostViewController: UITableViewController, TableModelHost { @IBOutlet private weak var itemNext: UIBarButtonItem! private let existingHosts: [String] = { @@ -46,8 +46,6 @@ class WizardHostViewController: UITableViewController, TableModelHost, Wizard { private var createdProfile: HostConnectionProfile? - weak var delegate: WizardDelegate? - // MARK: TableModelHost lazy var model: TableModel = { @@ -150,7 +148,10 @@ class WizardHostViewController: UITableViewController, TableModelHost, Wizard { } dismiss(animated: true) { - self.delegate?.wizard(didCreate: profile, withCredentials: credentials) + NotificationCenter.default.post(name: .WizardDidCreate, object: nil, userInfo: [ + WizardCreationKey.profile: profile, + WizardCreationKey.credentials: credentials + ]) } } diff --git a/Passepartout-iOS/Scenes/Organizer/WizardProviderViewController.swift b/Passepartout-iOS/Scenes/Organizer/WizardProviderViewController.swift index 05e5c4d3..715eada3 100644 --- a/Passepartout-iOS/Scenes/Organizer/WizardProviderViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/WizardProviderViewController.swift @@ -25,13 +25,11 @@ import UIKit -class WizardProviderViewController: UITableViewController, Wizard { +class WizardProviderViewController: UITableViewController { var availableNames: [Infrastructure.Name] = [] private var createdProfile: ProviderConnectionProfile? - weak var delegate: WizardDelegate? - override func viewDidLoad() { super.viewDidLoad() @@ -55,7 +53,10 @@ class WizardProviderViewController: UITableViewController, Wizard { fatalError("No profile created?") } dismiss(animated: true) { - self.delegate?.wizard(didCreate: profile, withCredentials: credentials) + NotificationCenter.default.post(name: .WizardDidCreate, object: nil, userInfo: [ + WizardCreationKey.profile: profile, + WizardCreationKey.credentials: credentials + ]) } } diff --git a/Passepartout/Sources/Model/Wizard.swift b/Passepartout/Sources/Model/Wizard.swift index 6f04923b..cad6dceb 100644 --- a/Passepartout/Sources/Model/Wizard.swift +++ b/Passepartout/Sources/Model/Wizard.swift @@ -25,10 +25,12 @@ import Foundation -protocol Wizard: class { - var delegate: WizardDelegate? { get set } +extension Notification.Name { + static let WizardDidCreate = Notification.Name("WizardDidCreate") } -protocol WizardDelegate: class { - func wizard(didCreate profile: ConnectionProfile, withCredentials credentials: Credentials) +enum WizardCreationKey: String { + case profile + + case credentials } From c084c71db3e30afdcfb187546c008d123da7a852 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 20:37:30 +0200 Subject: [PATCH 2/4] Present host wizard in presented vc or root --- Passepartout-iOS/AppDelegate.swift | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/Passepartout-iOS/AppDelegate.swift b/Passepartout-iOS/AppDelegate.swift index 9ded42e9..bab9e9a3 100644 --- a/Passepartout-iOS/AppDelegate.swift +++ b/Passepartout-iOS/AppDelegate.swift @@ -89,45 +89,47 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele guard let root = window?.rootViewController else { fatalError("No window.rootViewController?") } + + let topmost = root.presentedViewController ?? root let fm = FileManager.default - guard let parsedFile = ParsedFile.from(url, withErrorAlertIn: root) else { + guard let parsedFile = ParsedFile.from(url, withErrorAlertIn: topmost) else { try? fm.removeItem(at: url) return true } if let warning = parsedFile.warning { - ParsedFile.alertImportWarning(url: url, in: root, withWarning: warning) { + ParsedFile.alertImportWarning(url: url, in: topmost, withWarning: warning) { if $0 { - self.handleParsedFile(parsedFile, in: root) + self.handleParsedFile(parsedFile, in: topmost) } else { try? fm.removeItem(at: url) } } return true } - handleParsedFile(parsedFile, in: root) + handleParsedFile(parsedFile, in: topmost) return true } - private func handleParsedFile(_ parsedFile: ParsedFile, in root: UIViewController) { + private func handleParsedFile(_ parsedFile: ParsedFile, in target: UIViewController) { // already presented: update parsed configuration - if let nav = root.presentedViewController as? UINavigationController, let wizard = nav.topViewController as? WizardHostViewController { + if let nav = target as? UINavigationController, let wizard = nav.topViewController as? WizardHostViewController { wizard.parsedFile = parsedFile wizard.removesConfigurationOnCancel = true return } // present now - let nav = StoryboardScene.Organizer.wizardHostIdentifier.instantiate() - guard let wizard = nav.topViewController as? WizardHostViewController else { + let wizardNav = StoryboardScene.Organizer.wizardHostIdentifier.instantiate() + guard let wizard = wizardNav.topViewController as? WizardHostViewController else { fatalError("Expected WizardHostViewController from storyboard") } wizard.parsedFile = parsedFile wizard.removesConfigurationOnCancel = true - nav.modalPresentationStyle = .formSheet - root.present(nav, animated: true, completion: nil) + wizardNav.modalPresentationStyle = .formSheet + target.present(wizardNav, animated: true, completion: nil) } } From 06ecd3367f72b09419f3193f42b672c90a130dba Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 20:38:08 +0200 Subject: [PATCH 3/4] Overwrite pending profile when reimporting - Overwrite title field - Remove old .ovpn useSuggestedTitle() is only called on load, it will never overwrite user input unless there's a new import. --- Passepartout-iOS/AppDelegate.swift | 3 +++ .../Scenes/Organizer/WizardHostViewController.swift | 7 +------ 2 files changed, 4 insertions(+), 6 deletions(-) diff --git a/Passepartout-iOS/AppDelegate.swift b/Passepartout-iOS/AppDelegate.swift index bab9e9a3..e2d76aab 100644 --- a/Passepartout-iOS/AppDelegate.swift +++ b/Passepartout-iOS/AppDelegate.swift @@ -115,6 +115,9 @@ class AppDelegate: UIResponder, UIApplicationDelegate, UISplitViewControllerDele // already presented: update parsed configuration if let nav = target as? UINavigationController, let wizard = nav.topViewController as? WizardHostViewController { + if let oldURL = wizard.parsedFile?.url { + try? FileManager.default.removeItem(at: oldURL) + } wizard.parsedFile = parsedFile wizard.removesConfigurationOnCancel = true return diff --git a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift index e08da738..7c165371 100644 --- a/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/WizardHostViewController.swift @@ -87,12 +87,7 @@ class WizardHostViewController: UITableViewController, TableModelHost { // MARK: Actions private func useSuggestedTitle() { - guard let field = cellTitle?.field else { - return - } - if field.text?.isEmpty ?? true { - field.text = parsedFile?.url.normalizedFilename - } + cellTitle?.field.text = parsedFile?.url.normalizedFilename } @IBAction private func next() { From 3d62728a95fd25aa21ebc7661a0b7a394f64f466 Mon Sep 17 00:00:00 2001 From: Davide De Rosa Date: Sat, 27 Oct 2018 21:13:21 +0200 Subject: [PATCH 4/4] Fix detail replacement in compact mode Do not push created profile onto presented profile or any other screen deeper inside (e.g. Parameters). --- .../Scenes/Organizer/OrganizerViewController.swift | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift b/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift index 2dcfcd11..7a8723b2 100644 --- a/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift +++ b/Passepartout-iOS/Scenes/Organizer/OrganizerViewController.swift @@ -451,6 +451,20 @@ extension OrganizerViewController { reloadModel() tableView.reloadData() + // XXX: hack around bad replace when detail presented in compact view + if let detailNav = navigationController?.viewControllers.last as? UINavigationController { + var existingServiceVC: ServiceViewController? + for vc in detailNav.viewControllers { + if let found = vc as? ServiceViewController { + existingServiceVC = found + break + } + } + let serviceVC = existingServiceVC ?? (StoryboardScene.Main.serviceIdentifier.instantiate().topViewController as! ServiceViewController) + serviceVC.profile = profile + detailNav.setViewControllers([serviceVC], animated: true) + return + } perform(segue: StoryboardSegue.Organizer.selectProfileSegueIdentifier, sender: profile) } }