Skip to content

Commit 3f6fc03

Browse files
authored
Fix some app route navigation bugs. (#4415)
- Opening a notification would update the stack with an animated pop. - Opening a permalink from the timeline wouldn't push the room as a child.
1 parent f1f6cac commit 3f6fc03

File tree

4 files changed

+43
-5
lines changed

4 files changed

+43
-5
lines changed

ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -157,20 +157,32 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
157157
.roomDetails, .roomMemberDetails, .userProfile,
158158
.event, .eventOnRoomAlias, .childEvent, .childEventOnRoomAlias,
159159
.call, .genericCallLink, .share, .transferOwnership:
160-
clearRoute(animated: animated) // Make sure the presented route is visible.
160+
clearPresentedSheets(animated: animated) // Make sure the presented route is visible.
161161
chatsFlowCoordinator.handleAppRoute(appRoute, animated: animated)
162-
navigationTabCoordinator.selectedTab = .chats
162+
if navigationTabCoordinator.selectedTab != .chats {
163+
navigationTabCoordinator.selectedTab = .chats
164+
}
163165
}
164166
}
165167

166168
func clearRoute(animated: Bool) {
169+
clearPresentedSheets(animated: animated)
170+
chatsFlowCoordinator.clearRoute(animated: animated)
171+
}
172+
173+
// Clearing routes is more complicated than it first seems. When passing routes
174+
// to the chats flow we can't clear all routes as e.g. childRoom/childEvent etc
175+
// expect to push into the existing stack. But we do need to hide any sheets that
176+
// might cover up the presented route. BUT! We probably shouldn't dismiss onboarding
177+
// or verification flows until they're complete… This needs more thought before we
178+
// codify it all into the state machine.
179+
private func clearPresentedSheets(animated: Bool) {
167180
switch stateMachine.state {
168181
case .initial, .tabBar:
169182
break
170183
case .settingsScreen:
171184
navigationTabCoordinator.setSheetCoordinator(nil, animated: animated)
172185
}
173-
chatsFlowCoordinator.clearRoute(animated: animated)
174186
}
175187

176188
func isDisplayingRoomScreen(withRoomID roomID: String) -> Bool {
@@ -258,7 +270,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
258270

259271
// MARK: - Onboarding
260272

261-
func attemptStartingOnboarding() {
273+
private func attemptStartingOnboarding() {
262274
MXLog.info("Attempting to start onboarding")
263275

264276
if onboardingFlowCoordinator.shouldStart {

ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,6 @@
55
// Please see LICENSE files in the repository root for full details.
66
//
77

8-
import Algorithms
98
import Combine
109
import Foundation
1110
import MatrixRustSDK

ElementX/Sources/Services/Room/RoomProxyProtocol.swift

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
// Please see LICENSE files in the repository root for full details.
66
//
77

8+
import Algorithms
89
import Combine
910
import Foundation
1011
import MatrixRustSDK

UnitTests/Sources/UserSessionFlowCoordinatorTests.swift

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,32 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
6868
XCTAssertNotNil(detailCoordinator)
6969
}
7070

71+
func testRoomPresentationClearsSettings() async throws {
72+
try await process(route: .settings, expectedUserSessionState: .settingsScreen)
73+
XCTAssertTrue((tabCoordinator?.sheetCoordinator as? NavigationStackCoordinator)?.rootCoordinator is SettingsScreenCoordinator)
74+
XCTAssertNil(detailCoordinator)
75+
76+
try await process(route: .room(roomID: "1", via: []), expectedChatsState: .roomList(roomListSelectedRoomID: "1"))
77+
XCTAssertNil((tabCoordinator?.sheetCoordinator))
78+
XCTAssertTrue(detailNavigationStack?.rootCoordinator is RoomScreenCoordinator)
79+
XCTAssertNotNil(detailCoordinator)
80+
}
81+
82+
func testChildRoomPresentation() async throws {
83+
try await process(route: .room(roomID: "1", via: []), expectedChatsState: .roomList(roomListSelectedRoomID: "1"))
84+
let detailNavigationStack = try XCTUnwrap(detailNavigationStack, "There must be a navigation stack.")
85+
XCTAssertTrue(detailNavigationStack.rootCoordinator is RoomScreenCoordinator)
86+
XCTAssertNotNil(detailCoordinator)
87+
88+
let deferred = deferFulfillment(detailNavigationStack.observe(\.stackCoordinators.count)) { $0 == 1 }
89+
try await process(route: .childRoom(roomID: "2", via: []))
90+
try await deferred.fulfill()
91+
XCTAssertTrue(detailNavigationStack.rootCoordinator is RoomScreenCoordinator)
92+
XCTAssertNotNil(detailCoordinator)
93+
XCTAssertEqual(detailNavigationStack.stackCoordinators.count, 1)
94+
XCTAssertTrue(detailNavigationStack.stackCoordinators.first is RoomScreenCoordinator)
95+
}
96+
7197
func testShareMediaRouteWithoutRoom() async throws {
7298
try await process(route: .settings, expectedUserSessionState: .settingsScreen)
7399
XCTAssertTrue((tabCoordinator?.sheetCoordinator as? NavigationStackCoordinator)?.rootCoordinator is SettingsScreenCoordinator)

0 commit comments

Comments
 (0)