From 960ba8468382a39d9a2033c651714da4d7f6e8ac Mon Sep 17 00:00:00 2001 From: Shadowfacts Date: Thu, 22 Aug 2024 14:32:01 -0400 Subject: [PATCH] New way of sequencing navigation operations Better fix for #484 --- Tusker/AppDelegate.swift | 14 +++-- ...ountSwitchingContainerViewController.swift | 4 +- Tusker/Screens/Main/Duckable+Root.swift | 4 +- .../Main/MainSplitViewController.swift | 13 ++--- .../Main/MainTabBarViewController.swift | 25 +++++---- .../Main/TuskerRootViewController.swift | 54 ++++++++++++++++++- Tusker/Shortcuts/AppShortcutItems.swift | 4 +- .../UserActivityHandlingContext.swift | 4 +- 8 files changed, 90 insertions(+), 32 deletions(-) diff --git a/Tusker/AppDelegate.swift b/Tusker/AppDelegate.swift index aa2826fdc7..5778348b81 100644 --- a/Tusker/AppDelegate.swift +++ b/Tusker/AppDelegate.swift @@ -292,12 +292,16 @@ extension AppDelegate: UNUserNotificationCenterDelegate { let rootViewController = delegate.rootViewController { let mastodonController = MastodonController.getForAccount(account) - // if the scene is already active, then we animate the account switching if necessary - delegate.activateAccount(account, animated: scene.activationState == .foregroundActive) + // if the scene is already active, then we animate things + let animated = scene.activationState == .foregroundActive - rootViewController.select(route: .notifications, animated: false) - let vc = NotificationLoadingViewController(notificationID: notificationID, mastodonController: mastodonController) - rootViewController.getNavigationController().pushViewController(vc, animated: false) + delegate.activateAccount(account, animated: animated) + + rootViewController.runNavigation(animated: animated) { navigation in + navigation.select(route: .notifications) + let vc = NotificationLoadingViewController(notificationID: notificationID, mastodonController: mastodonController) + navigation.push(viewController: vc) + } } else { let activity = UserActivityManager.showNotificationActivity(id: notificationID, accountID: accountID) if #available(iOS 17.0, *) { diff --git a/Tusker/Screens/Main/AccountSwitchingContainerViewController.swift b/Tusker/Screens/Main/AccountSwitchingContainerViewController.swift index 32d7880801..afdc000208 100644 --- a/Tusker/Screens/Main/AccountSwitchingContainerViewController.swift +++ b/Tusker/Screens/Main/AccountSwitchingContainerViewController.swift @@ -159,9 +159,9 @@ extension AccountSwitchingContainerViewController: TuskerRootViewController { root.compose(editing: draft, animated: animated, isDucked: isDucked, completion: completion) } - func select(route: TuskerRoute, animated: Bool) { + func select(route: TuskerRoute, animated: Bool, completion: (() -> Void)?) { loadViewIfNeeded() - root.select(route: route, animated: animated) + root.select(route: route, animated: animated, completion: completion) } func getNavigationDelegate() -> TuskerNavigationDelegate? { diff --git a/Tusker/Screens/Main/Duckable+Root.swift b/Tusker/Screens/Main/Duckable+Root.swift index 69d363695e..5ab80ca5f2 100644 --- a/Tusker/Screens/Main/Duckable+Root.swift +++ b/Tusker/Screens/Main/Duckable+Root.swift @@ -35,8 +35,8 @@ extension DuckableContainerViewController: AccountSwitchableViewController { (child as! TuskerRootViewController).getNavigationController() } - func select(route: TuskerRoute, animated: Bool) { - (child as? TuskerRootViewController)?.select(route: route, animated: animated) + func select(route: TuskerRoute, animated: Bool, completion: (() -> Void)?) { + (child as? TuskerRootViewController)?.select(route: route, animated: animated, completion: completion) } func performSearch(query: String) { diff --git a/Tusker/Screens/Main/MainSplitViewController.swift b/Tusker/Screens/Main/MainSplitViewController.swift index c78a2c6b8a..687be122b8 100644 --- a/Tusker/Screens/Main/MainSplitViewController.swift +++ b/Tusker/Screens/Main/MainSplitViewController.swift @@ -333,14 +333,14 @@ extension MainSplitViewController: UISplitViewControllerDelegate { // Transfer the navigation stack, dropping the search VC, to keep anything the user has opened transferNavigationStack(from: .tab(.explore), to: exploreNav, dropFirst: true, append: true) - tabBarViewController.select(tab: .explore, dismissPresented: false) + tabBarViewController.select(tab: .explore, dismissPresented: false, animated: false) case let .tab(tab): // sidebar items that map 1 <-> 1 can be transferred directly - tabBarViewController.select(tab: tab, dismissPresented: false) + tabBarViewController.select(tab: tab, dismissPresented: false, animated: false) case .bookmarks, .favorites, .list(_), .savedHashtag(_), .savedInstance(_): - tabBarViewController.select(tab: .explore, dismissPresented: false) + tabBarViewController.select(tab: .explore, dismissPresented: false, animated: false) // Make sure the Explore VC doesn't show its search bar when it appears, in case the user was previously // in compact mode and performing a search. let exploreNav = tabBarViewController.viewController(for: .explore) as! UINavigationController @@ -546,14 +546,14 @@ extension MainSplitViewController: StateRestorableViewController { } extension MainSplitViewController: TuskerRootViewController { - func select(route: TuskerRoute, animated: Bool) { + func select(route: TuskerRoute, animated: Bool, completion: (() -> Void)?) { guard traitCollection.horizontalSizeClass != .compact else { - tabBarViewController?.select(route: route, animated: animated) + tabBarViewController?.select(route: route, animated: animated, completion: completion) return } guard presentedViewController == nil else { dismiss(animated: animated) { - self.select(route: route, animated: animated) + self.select(route: route, animated: animated, completion: completion) } return } @@ -579,6 +579,7 @@ extension MainSplitViewController: TuskerRootViewController { let oldItem = sidebar.selectedItem sidebar.select(item: item, animated: false) select(newItem: item, oldItem: oldItem) + completion?() } func getNavigationDelegate() -> TuskerNavigationDelegate? { diff --git a/Tusker/Screens/Main/MainTabBarViewController.swift b/Tusker/Screens/Main/MainTabBarViewController.swift index e3473b2830..6436050d1c 100644 --- a/Tusker/Screens/Main/MainTabBarViewController.swift +++ b/Tusker/Screens/Main/MainTabBarViewController.swift @@ -54,19 +54,22 @@ class MainTabBarViewController: BaseMainTabBarViewController { view.backgroundColor = .appBackground } - func select(tab: Tab, dismissPresented: Bool) { + func select(tab: Tab, dismissPresented: Bool, animated: Bool, completion: (() -> Void)? = nil) { if tab == .compose { - compose(editing: nil) + compose(editing: nil, completion: completion) } else { // when switching tabs, dismiss the currently presented VC // otherwise the selected tab changes behind the presented VC if presentedViewController != nil && dismissPresented { - dismiss(animated: true) { + dismiss(animated: animated) { + stateRestorationLogger.info("MainTabBarViewController: selecting \(String(describing: tab), privacy: .public)") self.selectedIndex = tab.rawValue + completion?() } } else { stateRestorationLogger.info("MainTabBarViewController: selecting \(String(describing: tab), privacy: .public)") selectedIndex = tab.rawValue + completion?() } } } @@ -148,21 +151,21 @@ extension MainTabBarViewController: UITabBarControllerDelegate { } extension MainTabBarViewController: TuskerRootViewController { - func select(route: TuskerRoute, animated: Bool) { + func select(route: TuskerRoute, animated: Bool, completion: (() -> Void)?) { switch route { case .timelines: - select(tab: .timelines, dismissPresented: true) + select(tab: .timelines, dismissPresented: true, animated: animated, completion: completion) case .notifications: - select(tab: .notifications, dismissPresented: true) + select(tab: .notifications, dismissPresented: true, animated: animated, completion: completion) case .myProfile: - select(tab: .myProfile, dismissPresented: true) + select(tab: .myProfile, dismissPresented: true, animated: animated, completion: completion) case .explore: - select(tab: .explore, dismissPresented: true) + select(tab: .explore, dismissPresented: true, animated: animated, completion: completion) case .bookmarks: - select(tab: .explore, dismissPresented: true) + select(tab: .explore, dismissPresented: true, animated: animated, completion: completion) getNavigationController().pushViewController(BookmarksViewController(mastodonController: mastodonController), animated: animated) case .list(id: let id): - select(tab: .explore, dismissPresented: true) + select(tab: .explore, dismissPresented: true, animated: animated, completion: completion) if let list = mastodonController.getCachedList(id: id) { let nav = getNavigationController() _ = nav.popToRootViewController(animated: animated) @@ -185,7 +188,7 @@ extension MainTabBarViewController: TuskerRootViewController { return } - select(tab: .explore, dismissPresented: true) + select(tab: .explore, dismissPresented: true, animated: false) exploreNavController.popToRootViewController(animated: false) // setting searchController.isActive directly doesn't work until the view has loaded/appeared for the first time diff --git a/Tusker/Screens/Main/TuskerRootViewController.swift b/Tusker/Screens/Main/TuskerRootViewController.swift index aa392d954c..e58a1d58b5 100644 --- a/Tusker/Screens/Main/TuskerRootViewController.swift +++ b/Tusker/Screens/Main/TuskerRootViewController.swift @@ -12,8 +12,7 @@ import ComposeUI @MainActor protocol TuskerRootViewController: UIViewController, StateRestorableViewController, StatusBarTappableViewController { func compose(editing draft: Draft?, animated: Bool, isDucked: Bool, completion: (() -> Void)?) - func select(route: TuskerRoute, animated: Bool) - func getTabController(tab: MainTabBarViewController.Tab) -> UIViewController? + func select(route: TuskerRoute, animated: Bool, completion: (() -> Void)?) func getNavigationDelegate() -> TuskerNavigationDelegate? func getNavigationController() -> NavigationControllerProtocol func performSearch(query: String) @@ -21,6 +20,14 @@ protocol TuskerRootViewController: UIViewController, StateRestorableViewControll func presentPreferences(completion: (() -> Void)?) -> PreferencesNavigationController? } +extension TuskerRootViewController { + func runNavigation(animated: Bool, _ builder: (_ navigation: TuskerNavigationSequence) -> Void) { + let sequence = TuskerNavigationSequence(root: self, animated: animated) + builder(sequence) + sequence.run() + } +} + enum TuskerRoute { case timelines case notifications @@ -30,6 +37,49 @@ enum TuskerRoute { case list(id: String) } +/// A class that manages running a sequence of navigation operations on a ``TuskerRootViewController``. +/// +/// Use this type, rather than calling multiple methods on the root VC in a row, because it manages waiting until each previous step finishes. +@MainActor +final class TuskerNavigationSequence { + let root: any TuskerRootViewController + let animated: Bool + private var operations = [() -> Void]() + + init(root: any TuskerRootViewController, animated: Bool) { + self.root = root + self.animated = animated + } + + func select(route: TuskerRoute) { + operations.append { + self.root.select(route: route, animated: self.animated, completion: self.run) + } + } + + func push(viewController: UIViewController) { + operations.append { + let nav = self.root.getNavigationController() + nav.pushViewController(viewController, animated: self.animated) + self.run() + } + } + + func popToRoot() { + operations.append { + let nav = self.root.getNavigationController() + nav.popToRootViewController(animated: self.animated) + self.run() + } + } + + func run() { + if !operations.isEmpty { + operations.removeFirst()() + } + } +} + @MainActor protocol NavigationControllerProtocol: UIViewController { var viewControllers: [UIViewController] { get set } diff --git a/Tusker/Shortcuts/AppShortcutItems.swift b/Tusker/Shortcuts/AppShortcutItems.swift index baa5905aa8..af6b6883aa 100644 --- a/Tusker/Shortcuts/AppShortcutItems.swift +++ b/Tusker/Shortcuts/AppShortcutItems.swift @@ -44,9 +44,9 @@ enum AppShortcutItem: String, CaseIterable { } switch self { case .showHomeTimeline: - root.select(route: .timelines, animated: false) + root.select(route: .timelines, animated: false, completion: nil) case .showNotifications: - root.select(route: .notifications, animated: false) + root.select(route: .notifications, animated: false, completion: nil) case .composePost: root.compose(editing: nil, animated: false, isDucked: false, completion: nil) } diff --git a/Tusker/Shortcuts/UserActivityHandlingContext.swift b/Tusker/Shortcuts/UserActivityHandlingContext.swift index 5ee9f185aa..e4ae550d70 100644 --- a/Tusker/Shortcuts/UserActivityHandlingContext.swift +++ b/Tusker/Shortcuts/UserActivityHandlingContext.swift @@ -36,7 +36,7 @@ struct ActiveAccountUserActivityHandlingContext: UserActivityHandlingContext { } func select(route: TuskerRoute) { - root.select(route: route, animated: true) + root.select(route: route, animated: true, completion: nil) } func present(_ vc: UIViewController) { @@ -72,7 +72,7 @@ class StateRestorationUserActivityHandlingContext: UserActivityHandlingContext { var isHandoff: Bool { false } func select(route: TuskerRoute) { - root.select(route: route, animated: false) + root.select(route: route, animated: false, completion: nil) state = .selectedRoute }