From d949b174485403f3ea07b4badf7b78db7b0ea783 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Tue, 23 Apr 2024 16:25:05 +0300 Subject: [PATCH] dd support for deeplinking/navigating into the same room multiple times - fix bugs around the view <-> view models going out of sync - unwind the stack if the room is already presented --- .../RoomFlowCoordinator.swift | 30 ++++++++++++++++--- .../UserSessionFlowCoordinator.swift | 10 +++++-- .../Sources/RoomFlowCoordinatorTests.swift | 5 +++- changelog.d/2653.bugfix | 1 + 4 files changed, 39 insertions(+), 7 deletions(-) create mode 100644 changelog.d/2653.bugfix diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 49815ebb4..7c9deb53f 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -19,6 +19,7 @@ import SwiftState import SwiftUI import UserNotifications +// swiftlint:disable file_length enum RoomFlowCoordinatorAction: Equatable { case presentRoom(roomID: String) case presentCallScreen(roomProxy: RoomProxyProtocol) @@ -38,7 +39,7 @@ enum RoomFlowCoordinatorAction: Equatable { } } -// swiftlint:disable file_length +// swiftlint:disable:next type_body_length class RoomFlowCoordinator: FlowCoordinatorProtocol { private let roomID: String private let userSession: UserSessionProtocol @@ -104,6 +105,10 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { } func handleAppRoute(_ appRoute: AppRoute, animated: Bool) { + guard stateMachine.state != .complete else { + fatalError("This flow coordinator is `finished` ☠️") + } + switch appRoute { case .room(let roomID): Task { @@ -324,7 +329,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { dismissFlow(animated: animated) case (_, .presentRoom, .room): - Task { await self.presentRoom(animated: animated) } + Task { await self.presentRoom(fromState: context.fromState, animated: animated) } case (_, .dismissFlow, .complete): dismissFlow(animated: animated) @@ -463,11 +468,25 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { /// Updates the navigation stack so it displays the timeline for the given room /// - Parameters: /// - animated: whether it should animate the transition - private func presentRoom(animated: Bool) async { + private func presentRoom(fromState: State, animated: Bool) async { // If any sheets are presented dismiss them, rely on their dismissal callbacks to transition the state machine // through the correct states before presenting the room navigationStackCoordinator.setSheetCoordinator(nil) + // Handle selecting the same room again + if !isChildFlow { + // First unwind the navigation stack + navigationStackCoordinator.popToRoot(animated: animated) + + // And then decide if the room actually needs to be presented again + switch fromState { + case .initial, .roomDetails(isRoot: true), .joinRoomScreen: + break + default: + return // The room is already on the stack, no need to present it again + } + } + Task { // Flag the room as read on entering, the timeline will take care of the read receipts await roomProxy.flagAsUnread(false) @@ -649,7 +668,10 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { if isRoot { navigationStackCoordinator.setRootCoordinator(coordinator, animated: animated) { [weak self] in - self?.stateMachine.tryEvent(.dismissFlow) + guard let self else { return } + if stateMachine.state != .room { // The root has been replaced by a room + stateMachine.tryEvent(.dismissFlow) + } } } else { navigationStackCoordinator.push(coordinator, animated: animated) { [weak self] in diff --git a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift index bdc58e9f3..259fbc906 100644 --- a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift @@ -267,8 +267,14 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol { presentHomeScreen() attemptStartingOnboarding() - case(.roomList, .selectRoom(let roomID, let showingRoomDetails), .roomList): - Task { await self.startRoomFlow(roomID: roomID, showingRoomDetails: showingRoomDetails, animated: animated) } + case(.roomList(let selectedRoomID), .selectRoom(let roomID, let showingRoomDetails), .roomList): + if selectedRoomID == roomID { + if let roomFlowCoordinator { + roomFlowCoordinator.handleAppRoute(.room(roomID: roomID), animated: animated) + } + } else { + Task { await self.startRoomFlow(roomID: roomID, showingRoomDetails: showingRoomDetails, animated: animated) } + } case(.roomList, .deselectRoom, .roomList): dismissRoomFlow(animated: animated) diff --git a/UnitTests/Sources/RoomFlowCoordinatorTests.swift b/UnitTests/Sources/RoomFlowCoordinatorTests.swift index 82b49828a..8c5a07eb6 100644 --- a/UnitTests/Sources/RoomFlowCoordinatorTests.swift +++ b/UnitTests/Sources/RoomFlowCoordinatorTests.swift @@ -162,6 +162,8 @@ class RoomFlowCoordinatorTests: XCTestCase { try await clearRoute(expectedActions: [.finished]) XCTAssertNil(navigationStackCoordinator.rootCoordinator) + await setupRoomFlowCoordinator(roomType: .invited(roomID: "InvitedRoomID")) + try await process(route: .room(roomID: "InvitedRoomID")) XCTAssert(navigationStackCoordinator.rootCoordinator is JoinRoomScreenCoordinator) XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 0) @@ -188,7 +190,8 @@ class RoomFlowCoordinatorTests: XCTestCase { try await clearRoute(expectedActions: [.finished]) XCTAssertNil(navigationStackCoordinator.stackCoordinators.last, "A child room flow should remove the join room scren on dismissal") - navigationStackCoordinator.popToRoot() + await setupRoomFlowCoordinator(asChildFlow: true, roomType: .invited(roomID: "InvitedRoomID")) + navigationStackCoordinator.setRootCoordinator(BlankFormCoordinator()) try await process(route: .room(roomID: "InvitedRoomID")) XCTAssertTrue(navigationStackCoordinator.rootCoordinator is BlankFormCoordinator, "A child room flow should push onto the stack, leaving the root alone.") diff --git a/changelog.d/2653.bugfix b/changelog.d/2653.bugfix new file mode 100644 index 000000000..7e7f6156a --- /dev/null +++ b/changelog.d/2653.bugfix @@ -0,0 +1 @@ +Fix deeplinking/navigating into the same room multiple times \ No newline at end of file