diff --git a/ElementX/Sources/Application/CoordinatorProtocol.swift b/ElementX/Sources/Application/CoordinatorProtocol.swift index 23cf2378a..88b4bd769 100644 --- a/ElementX/Sources/Application/CoordinatorProtocol.swift +++ b/ElementX/Sources/Application/CoordinatorProtocol.swift @@ -17,7 +17,7 @@ import SwiftUI @MainActor -protocol CoordinatorProtocol { +protocol CoordinatorProtocol: AnyObject { func start() func stop() func toPresentable() -> AnyView diff --git a/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift b/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift index bb0574d4f..020f0472a 100644 --- a/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift +++ b/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift @@ -213,14 +213,14 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS /// /// For added complexity, the NavigationSplitCoordinator has an internal compact layout NavigationStack for which we need to manually nil things again private func releaseAllCoordinatorReferences() { - sidebarModule?.coordinator = nil - detailModule?.coordinator = nil - sheetModule?.coordinator = nil - fullScreenCoverModule?.coordinator = nil + sidebarModule?.tearDown() + detailModule?.tearDown() + sheetModule?.tearDown() + fullScreenCoverModule?.tearDown() - compactLayoutRootModule?.coordinator = nil + compactLayoutRootModule?.tearDown() compactLayoutStackModules.forEach { module in - module.coordinator = nil + module.tearDown() } } @@ -594,6 +594,19 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS .presentationDetents(presentationDetents)) } + /// The NavigationStack has a tendency to hold on to path items for longer than needed. We work around that by manually nilling the coordinator + /// when a NavigationModule is dismissed. As the NavigationModule is just a wrapper multiple instances of it continuing living is of no consequence + /// https://stackoverflow.com/questions/73885353/found-a-strange-behaviour-of-state-when-combined-to-the-new-navigation-stack/ + func stop() { + rootModule?.tearDown() + sheetModule?.tearDown() + fullScreenCoverModule?.tearDown() + + stackModules.forEach { module in + module.tearDown() + } + } + // MARK: - CustomStringConvertible var description: String { diff --git a/ElementX/Sources/Application/Navigation/NavigationModule.swift b/ElementX/Sources/Application/Navigation/NavigationModule.swift index 061fc331d..a8d3f4bf7 100644 --- a/ElementX/Sources/Application/Navigation/NavigationModule.swift +++ b/ElementX/Sources/Application/Navigation/NavigationModule.swift @@ -27,7 +27,7 @@ class NavigationModule: Identifiable, Hashable { /// As the NavigationModule is just a wrapper multiple instances of it continuing living is of no consequence /// https://stackoverflow.com/questions/73885353/found-a-strange-behaviour-of-state-when-combined-to-the-new-navigation-stack/ var coordinator: (any CoordinatorProtocol)? - let dismissalCallback: (() -> Void)? + var dismissalCallback: (() -> Void)? init(_ coordinator: any CoordinatorProtocol, dismissalCallback: (() -> Void)? = nil) { self.coordinator = coordinator @@ -38,6 +38,7 @@ class NavigationModule: Identifiable, Hashable { coordinator?.stop() dismissalCallback?() coordinator = nil + dismissalCallback = nil } nonisolated static func == (lhs: NavigationModule, rhs: NavigationModule) -> Bool { diff --git a/ElementX/Sources/Other/UserNotifications/MockUserNotificationController.swift b/ElementX/Sources/Other/UserNotifications/MockUserNotificationController.swift index faff544a1..2b757aec7 100644 --- a/ElementX/Sources/Other/UserNotifications/MockUserNotificationController.swift +++ b/ElementX/Sources/Other/UserNotifications/MockUserNotificationController.swift @@ -16,7 +16,7 @@ import Foundation -struct MockUserNotificationController: UserNotificationControllerProtocol { +class MockUserNotificationController: UserNotificationControllerProtocol { func submitNotification(_ notification: UserNotification) { } func retractNotificationWithId(_ id: String) { } diff --git a/ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift b/ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift index c497b5fd7..e2b5bca7a 100644 --- a/ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift +++ b/ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift @@ -23,7 +23,7 @@ enum BugReportCoordinatorResult { struct BugReportCoordinatorParameters { let bugReportService: BugReportServiceProtocol - let userNotificationController: UserNotificationControllerProtocol + weak var userNotificationController: UserNotificationControllerProtocol? let screenshot: UIImage? let isModallyPresented: Bool } @@ -76,17 +76,17 @@ final class BugReportCoordinator: CoordinatorProtocol { static let loadingIndicatorIdentifier = "BugReportLoading" private func startLoading(label: String = ElementL10n.loading) { - parameters.userNotificationController.submitNotification(UserNotification(id: Self.loadingIndicatorIdentifier, - type: .modal, - title: label, - persistent: true)) + parameters.userNotificationController?.submitNotification(UserNotification(id: Self.loadingIndicatorIdentifier, + type: .modal, + title: label, + persistent: true)) } private func stopLoading() { - parameters.userNotificationController.retractNotificationWithId(Self.loadingIndicatorIdentifier) + parameters.userNotificationController?.retractNotificationWithId(Self.loadingIndicatorIdentifier) } private func showError(label: String) { - parameters.userNotificationController.submitNotification(UserNotification(title: label)) + parameters.userNotificationController?.submitNotification(UserNotification(title: label)) } } diff --git a/ElementX/Sources/Screens/Other/ActivityCoordinator.swift b/ElementX/Sources/Screens/Other/ActivityCoordinator.swift index 0c01e2de4..8ca35bfe5 100644 --- a/ElementX/Sources/Screens/Other/ActivityCoordinator.swift +++ b/ElementX/Sources/Screens/Other/ActivityCoordinator.swift @@ -16,9 +16,13 @@ import SwiftUI -struct ActivityCoordinator: CoordinatorProtocol { +class ActivityCoordinator: CoordinatorProtocol { let items: [Any] + init(items: [Any]) { + self.items = items + } + func toPresentable() -> AnyView { AnyView(UIActivityViewControllerWrapper(activityItems: items) .presentationDetents([.medium]) diff --git a/ElementX/Sources/Screens/Other/InviteFriendsCoordinator.swift b/ElementX/Sources/Screens/Other/InviteFriendsCoordinator.swift index 0f3d11898..50b8f3fb2 100644 --- a/ElementX/Sources/Screens/Other/InviteFriendsCoordinator.swift +++ b/ElementX/Sources/Screens/Other/InviteFriendsCoordinator.swift @@ -16,9 +16,13 @@ import SwiftUI -struct InviteFriendsCoordinator: CoordinatorProtocol { +class InviteFriendsCoordinator: CoordinatorProtocol { let userId: String + init(userId: String) { + self.userId = userId + } + func toPresentable() -> AnyView { guard let permalink = try? PermalinkBuilder.permalinkTo(userIdentifier: userId).absoluteString else { return AnyView(EmptyView()) diff --git a/ElementX/Sources/Screens/Other/SplashScreenCoordinator.swift b/ElementX/Sources/Screens/Other/SplashScreenCoordinator.swift index a2ea2b9c8..2e22d649a 100644 --- a/ElementX/Sources/Screens/Other/SplashScreenCoordinator.swift +++ b/ElementX/Sources/Screens/Other/SplashScreenCoordinator.swift @@ -16,7 +16,7 @@ import SwiftUI -struct SplashScreenCoordinator: CoordinatorProtocol { +class SplashScreenCoordinator: CoordinatorProtocol { func toPresentable() -> AnyView { AnyView( ZStack { diff --git a/ElementX/Sources/Screens/Settings/SettingsScreenCoordinator.swift b/ElementX/Sources/Screens/Settings/SettingsScreenCoordinator.swift index db1d2e384..27b783429 100644 --- a/ElementX/Sources/Screens/Settings/SettingsScreenCoordinator.swift +++ b/ElementX/Sources/Screens/Settings/SettingsScreenCoordinator.swift @@ -17,8 +17,8 @@ import SwiftUI struct SettingsScreenCoordinatorParameters { - let navigationStackCoordinator: NavigationStackCoordinator - let userNotificationController: UserNotificationControllerProtocol + weak var navigationStackCoordinator: NavigationStackCoordinator? + weak var userNotificationController: UserNotificationControllerProtocol? let userSession: UserSessionProtocol let bugReportService: BugReportServiceProtocol } @@ -88,10 +88,10 @@ final class SettingsScreenCoordinator: CoordinatorProtocol { break } - self?.parameters.navigationStackCoordinator.pop() + self?.parameters.navigationStackCoordinator?.pop() } - parameters.navigationStackCoordinator.push(coordinator) + parameters.navigationStackCoordinator?.push(coordinator) } private func verifySession() { @@ -103,15 +103,15 @@ final class SettingsScreenCoordinator: CoordinatorProtocol { let coordinator = SessionVerificationCoordinator(parameters: verificationParameters) coordinator.callback = { [weak self] in - self?.parameters.navigationStackCoordinator.setSheetCoordinator(nil) + self?.parameters.navigationStackCoordinator?.setSheetCoordinator(nil) } - parameters.navigationStackCoordinator.setSheetCoordinator(coordinator) { [weak self] in - self?.parameters.navigationStackCoordinator.setSheetCoordinator(nil) + parameters.navigationStackCoordinator?.setSheetCoordinator(coordinator) { [weak self] in + self?.parameters.navigationStackCoordinator?.setSheetCoordinator(nil) } } private func showSuccess(label: String) { - parameters.userNotificationController.submitNotification(UserNotification(title: label)) + parameters.userNotificationController?.submitNotification(UserNotification(title: label)) } } diff --git a/ElementX/Sources/UITests/UITestsAppCoordinator.swift b/ElementX/Sources/UITests/UITestsAppCoordinator.swift index 896ac21d1..5904be477 100644 --- a/ElementX/Sources/UITests/UITestsAppCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsAppCoordinator.swift @@ -119,7 +119,7 @@ class MockScreen: Identifiable { case .settings: let navigationStackCoordinator = NavigationStackCoordinator() let coordinator = SettingsScreenCoordinator(parameters: .init(navigationStackCoordinator: navigationStackCoordinator, - userNotificationController: MockUserNotificationController(), + userNotificationController: nil, userSession: MockUserSession(clientProxy: MockClientProxy(userID: "@mock:client.com"), mediaProvider: MockMediaProvider()), bugReportService: MockBugReportService())) @@ -128,7 +128,7 @@ class MockScreen: Identifiable { case .bugReport: let navigationStackCoordinator = NavigationStackCoordinator() let coordinator = BugReportCoordinator(parameters: .init(bugReportService: MockBugReportService(), - userNotificationController: MockUserNotificationController(), + userNotificationController: nil, screenshot: nil, isModallyPresented: true)) navigationStackCoordinator.setRootCoordinator(coordinator) @@ -136,7 +136,7 @@ class MockScreen: Identifiable { case .bugReportWithScreenshot: let navigationStackCoordinator = NavigationStackCoordinator() let coordinator = BugReportCoordinator(parameters: .init(bugReportService: MockBugReportService(), - userNotificationController: MockUserNotificationController(), + userNotificationController: nil, screenshot: Asset.Images.appLogo.image, isModallyPresented: false)) navigationStackCoordinator.setRootCoordinator(coordinator) diff --git a/ElementX/Sources/UITests/UITestsRootCoordinator.swift b/ElementX/Sources/UITests/UITestsRootCoordinator.swift index ae2711335..dba2d319e 100644 --- a/ElementX/Sources/UITests/UITestsRootCoordinator.swift +++ b/ElementX/Sources/UITests/UITestsRootCoordinator.swift @@ -16,18 +16,23 @@ import SwiftUI -struct UITestsRootCoordinator: CoordinatorProtocol { +class UITestsRootCoordinator: CoordinatorProtocol { let mockScreens: [MockScreen] var selectionCallback: ((UITestScreenIdentifier) -> Void)? + init(mockScreens: [MockScreen], selectionCallback: ((UITestScreenIdentifier) -> Void)? = nil) { + self.mockScreens = mockScreens + self.selectionCallback = selectionCallback + } + func toPresentable() -> AnyView { AnyView(body) } private var body: some View { List(mockScreens) { coordinator in - Button(coordinator.id.description) { - selectionCallback?(coordinator.id) + Button(coordinator.id.description) { [weak self] in + self?.selectionCallback?(coordinator.id) } .accessibilityIdentifier(coordinator.id.rawValue) } diff --git a/UnitTests/Sources/NavigationRootCoordinatorTests.swift b/UnitTests/Sources/NavigationRootCoordinatorTests.swift index 7998ee0ef..83a8c42a0 100644 --- a/UnitTests/Sources/NavigationRootCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationRootCoordinatorTests.swift @@ -67,6 +67,6 @@ class NavigationRootCoordinatorTests: XCTestCase { } } -private struct SomeTestCoordinator: CoordinatorProtocol { +private class SomeTestCoordinator: CoordinatorProtocol { let id = UUID() } diff --git a/UnitTests/Sources/NavigationSplitCoordinatorTests.swift b/UnitTests/Sources/NavigationSplitCoordinatorTests.swift index 379a2352d..11a2432b0 100644 --- a/UnitTests/Sources/NavigationSplitCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationSplitCoordinatorTests.swift @@ -269,6 +269,6 @@ class NavigationSplitCoordinatorTests: XCTestCase { } } -private struct SomeTestCoordinator: CoordinatorProtocol { +private class SomeTestCoordinator: CoordinatorProtocol { let id = UUID() } diff --git a/UnitTests/Sources/NavigationStackCoordinatorTests.swift b/UnitTests/Sources/NavigationStackCoordinatorTests.swift index aea651ba9..21996d500 100644 --- a/UnitTests/Sources/NavigationStackCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationStackCoordinatorTests.swift @@ -212,6 +212,6 @@ class NavigationStackCoordinatorTests: XCTestCase { } } -private struct SomeTestCoordinator: CoordinatorProtocol { +private class SomeTestCoordinator: CoordinatorProtocol { let id = UUID() }