From ad06e812bcc0fca22e89eed36a1b5d09770a3375 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 23 Feb 2023 16:01:48 +0200 Subject: [PATCH] =?UTF-8?q?Fix=20navigationStackCoordinator=20getting=20to?= =?UTF-8?q?rn=20down=20when=20being=20reset=20on=20=E2=80=A6=20(#613)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Fix navigationStackCoordinator getting torn down when being reset on the navigationSplitCoordinator, added unit tests for it * Guard against using the same coordinator more than once --- .../Navigation/NavigationCoordinators.swift | 28 +++++++++++++++++++ .../UserSessionFlowCoordinator.swift | 7 +++-- .../NavigationSplitCoordinatorTests.swift | 6 +++- changelog.d/pr-613.bugfix | 1 + 4 files changed, 39 insertions(+), 3 deletions(-) create mode 100644 changelog.d/pr-613.bugfix diff --git a/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift b/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift index 020f0472a..52cec3f64 100644 --- a/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift +++ b/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift @@ -138,6 +138,10 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS return } + if sidebarModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + sidebarModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) } @@ -151,6 +155,10 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS return } + if detailModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + detailModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) } @@ -164,6 +172,10 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS return } + if sheetModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + sheetModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) } @@ -177,6 +189,10 @@ class NavigationSplitCoordinator: CoordinatorProtocol, ObservableObject, CustomS return } + if fullScreenCoverModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + fullScreenCoverModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) } @@ -509,6 +525,10 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS return } + if rootModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + popToRoot(animated: false) rootModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) @@ -565,6 +585,10 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS return } + if sheetModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + sheetModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) } @@ -584,6 +608,10 @@ class NavigationStackCoordinator: ObservableObject, CoordinatorProtocol, CustomS return } + if fullScreenCoverModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + fullScreenCoverModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) } diff --git a/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift b/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift index 4288cf3a4..4e80ce371 100644 --- a/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift +++ b/ElementX/Sources/Services/UserSession/UserSessionFlowCoordinator.swift @@ -168,8 +168,7 @@ class UserSessionFlowCoordinator: CoordinatorProtocol { emojiProvider: emojiProvider) let coordinator = RoomScreenCoordinator(parameters: parameters) - detailNavigationStackCoordinator.setRootCoordinator(coordinator) - navigationSplitCoordinator.setDetailCoordinator(detailNavigationStackCoordinator) { [weak self, roomIdentifier] in + detailNavigationStackCoordinator.setRootCoordinator(coordinator) { [weak self, roomIdentifier] in guard let self else { return } // Move the state machine to no room selected if the room currently being dimissed @@ -180,6 +179,10 @@ class UserSessionFlowCoordinator: CoordinatorProtocol { self.detailNavigationStackCoordinator.setRootCoordinator(nil) } } + + if navigationSplitCoordinator.detailCoordinator == nil { + navigationSplitCoordinator.setDetailCoordinator(detailNavigationStackCoordinator) + } } } diff --git a/UnitTests/Sources/NavigationSplitCoordinatorTests.swift b/UnitTests/Sources/NavigationSplitCoordinatorTests.swift index 11a2432b0..7fc49d720 100644 --- a/UnitTests/Sources/NavigationSplitCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationSplitCoordinatorTests.swift @@ -259,9 +259,13 @@ class NavigationSplitCoordinatorTests: XCTestCase { // MARK: - Private private func assertCoordinatorsEqual(_ lhs: CoordinatorProtocol?, _ rhs: CoordinatorProtocol?) { + if lhs == nil, rhs == nil { + return + } + guard let lhs = lhs as? SomeTestCoordinator, let rhs = rhs as? SomeTestCoordinator else { - XCTFail("Coordinators are not the same") + XCTFail("Coordinators are not the same: \(String(describing: lhs)) != \(String(describing: rhs))") return } diff --git a/changelog.d/pr-613.bugfix b/changelog.d/pr-613.bugfix new file mode 100644 index 000000000..a051f9d47 --- /dev/null +++ b/changelog.d/pr-613.bugfix @@ -0,0 +1 @@ +Fix broken split layout room navigation \ No newline at end of file