Allow RoomFlowCoordinator to start a child flow. (#2644)

ElementX/Resources/Localizations/en.lproj/Localizable.strings

@@ -538,6 +538,7 @@
 "screen_room_details_notification_mode_default" = "Default";
 "screen_room_details_notification_title" = "Notifications";
 "screen_room_details_share_room_title" = "Share room";
+"screen_room_details_title" = "Room info";
 "screen_room_details_updating_room" = "Updating room…";
 "screen_room_directory_search_loading_error" = "Failed loading";
 "screen_room_directory_search_title" = "Room directory";
@@ -549,6 +550,7 @@
 "screen_room_member_details_block_alert_action" = "Block";
 "screen_room_member_details_block_alert_description" = "Blocked users won't be able to send you messages and all their messages will be hidden. You can unblock them anytime.";
 "screen_room_member_details_block_user" = "Block user";
+"screen_room_member_details_title" = "Profile";
 "screen_room_member_details_unblock_alert_action" = "Unblock";
 "screen_room_member_details_unblock_alert_description" = "You'll be able to see all messages from them again.";
 "screen_room_member_details_unblock_user" = "Unblock user";

ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift

@@ -42,6 +42,7 @@ enum RoomFlowCoordinatorAction: Equatable {
 class RoomFlowCoordinator: FlowCoordinatorProtocol {
     private let roomProxy: RoomProxyProtocol
     private let userSession: UserSessionProtocol
+    private let isChildFlow: Bool
     private let roomTimelineControllerFactory: RoomTimelineControllerFactoryProtocol
     private let navigationStackCoordinator: NavigationStackCoordinator
     private let emojiProvider: EmojiProviderProtocol
@@ -52,6 +53,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
     
     // periphery:ignore - used to avoid deallocation
     private var rolesAndPermissionsFlowCoordinator: RoomRolesAndPermissionsFlowCoordinator?
+    // periphery:ignore - used to avoid deallocation
+    private var childRoomFlowCoordinator: RoomFlowCoordinator?
     
     private let stateMachine: StateMachine<State, Event> = .init(state: .initial)
     
@@ -66,6 +69,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
     
     init(roomProxy: RoomProxyProtocol,
          userSession: UserSessionProtocol,
+         isChildFlow: Bool,
          roomTimelineControllerFactory: RoomTimelineControllerFactoryProtocol,
          navigationStackCoordinator: NavigationStackCoordinator,
          emojiProvider: EmojiProviderProtocol,
@@ -75,6 +79,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
          orientationManager: OrientationManagerProtocol) async {
         self.roomProxy = roomProxy
         self.userSession = userSession
+        self.isChildFlow = isChildFlow
         self.roomTimelineControllerFactory = roomTimelineControllerFactory
         self.navigationStackCoordinator = navigationStackCoordinator
         self.emojiProvider = emojiProvider
@@ -229,6 +234,13 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
             case (.rolesAndPermissions, .dismissRolesAndPermissionsScreen):
                 return .roomDetails(isRoot: false)
             
+            // Child flow
+            
+            case (_, .presentChildRoom(let roomID)):
+                return .presentingChild(childRoomID: roomID, previousState: fromState)
+            case (.presentingChild(_, let previousState), .dismissChildRoom):
+                return previousState
+            
             default:
                 return nil
             }
@@ -346,6 +358,12 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
             case (.rolesAndPermissions, .dismissRolesAndPermissionsScreen, .roomDetails):
                 rolesAndPermissionsFlowCoordinator = nil
             
+            // Child flow
+            case (_, .presentChildRoom(let roomID), .presentingChild):
+                Task { await self.presentChildFlow(for: roomID) }
+            case (.presentingChild, .dismissChildRoom, _):
+                childRoomFlowCoordinator = nil
+            
             default:
                 fatalError("Unknown transition: \(context)")
             }
@@ -370,11 +388,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
     
     /// Updates the navigation stack so it displays the timeline for the given room
     /// - Parameters:
-    ///   - roomID: the identifier of the room that is to be presented
     ///   - animated: whether it should animate the transition
-    ///   - destinationRoomProxy: an optional already build roomProxy for the target room. It is currently used when
-    ///   forwarding messages so that we can take advantage of the local echo
-    ///   and have the message already there when presenting the room
     private func presentRoom(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
@@ -440,17 +454,27 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
             }
             .store(in: &cancellables)
         
-        navigationStackCoordinator.setRootCoordinator(coordinator, animated: animated) { [weak self] in
+        if !isChildFlow {
-            // Move the state machine to no room selected if the room currently being dismissed
+            navigationStackCoordinator.setRootCoordinator(coordinator, animated: animated) { [weak self] in
-            // is the same as the one selected in the state machine.
+                self?.stateMachine.tryEvent(.dismissRoom)
-            // This generally happens when popping the room screen while in a compact layout
+            }
-            self?.stateMachine.tryEvent(.dismissRoom)
+        } else {
+            navigationStackCoordinator.push(coordinator, animated: animated) { [weak self] in
+                self?.stateMachine.tryEvent(.dismissRoom)
+            }
         }
     }
     
     private func dismissFlow(animated: Bool) {
-        navigationStackCoordinator.popToRoot(animated: false)
+        childRoomFlowCoordinator?.handleAppRoute(.roomList, animated: animated)
-        navigationStackCoordinator.setRootCoordinator(nil, animated: false)
+        
+        if isChildFlow {
+            // We don't support dismissing a child flow by itself, only the entire chain.
+            MXLog.info("Leaving navigation clean-up to the parent flow.")
+        } else {
+            navigationStackCoordinator.popToRoot(animated: false)
+            navigationStackCoordinator.setRootCoordinator(nil, animated: false)
+        }
         
         roomProxy.unsubscribeFromUpdates()
         timelineController = nil
@@ -857,12 +881,12 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
                     let currentDirectRoom = await userSession.clientProxy.directRoomForUserID(userID)
                     switch currentDirectRoom {
                     case .success(.some(let roomID)):
-                        actionsSubject.send(.presentRoom(roomID: roomID))
+                        stateMachine.tryEvent(.presentChildRoom(roomID: roomID))
                     case .success(nil):
                         switch await userSession.clientProxy.createDirectRoom(with: userID, expectedRoomName: displayName) {
                         case .success(let roomID):
                             analytics.trackCreatedRoom(isDM: true)
-                            actionsSubject.send(.presentRoom(roomID: roomID))
+                            stateMachine.tryEvent(.presentChildRoom(roomID: roomID))
                         case .failure:
                             userIndicatorController.alertInfo = .init(id: UUID())
                         }
@@ -935,9 +959,9 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
             return
         }
         
-        // This will become a child flow and we can pass the proxy down afterwards.
+        // We don't need to worry about passing in the room proxy as timelines are
-        
+        // cached. The local echo will be visible when fetching the room by its ID.
-        actionsSubject.send(.presentRoom(roomID: roomID))
+        stateMachine.tryEvent(.presentChildRoom(roomID: roomID))
     }
     
     private func presentNotificationSettingsScreen() {
@@ -1066,6 +1090,44 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
         rolesAndPermissionsFlowCoordinator = coordinator
         coordinator.start()
     }
+    
+    // MARK: - Child Flow
+    
+    private func presentChildFlow(for roomID: String) async {
+        guard let roomProxy = await userSession.clientProxy.roomForIdentifier(roomID) else {
+            MXLog.error("Child flow requested for missing room.")
+            userIndicatorController.submitIndicator(UserIndicator(title: L10n.errorUnknown))
+            stateMachine.tryEvent(.dismissChildRoom)
+            return
+        }
+        
+        let coordinator = await RoomFlowCoordinator(roomProxy: roomProxy,
+                                                    userSession: userSession,
+                                                    isChildFlow: true,
+                                                    roomTimelineControllerFactory: roomTimelineControllerFactory,
+                                                    navigationStackCoordinator: navigationStackCoordinator,
+                                                    emojiProvider: emojiProvider,
+                                                    appSettings: appSettings,
+                                                    analytics: analytics,
+                                                    userIndicatorController: userIndicatorController,
+                                                    orientationManager: orientationManager)
+        coordinator.actions.sink { [weak self] action in
+            guard let self else { return }
+            
+            switch action {
+            case .presentRoom(let roomID):
+                actionsSubject.send(.presentRoom(roomID: roomID))
+            case .presentCallScreen(let roomProxy):
+                actionsSubject.send(.presentCallScreen(roomProxy: roomProxy))
+            case .finished:
+                stateMachine.tryEvent(.dismissChildRoom)
+            }
+        }
+        .store(in: &cancellables)
+        
+        childRoomFlowCoordinator = coordinator
+        coordinator.handleAppRoute(.room(roomID: roomID), animated: true)
+    }
 }
 
 private extension RoomFlowCoordinator {
@@ -1081,7 +1143,7 @@ private extension RoomFlowCoordinator {
         }
     }
 
-    enum State: StateType {
+    indirect enum State: StateType {
         case initial
         case room
         case roomDetails(isRoot: Bool)
@@ -1102,6 +1164,8 @@ private extension RoomFlowCoordinator {
         case pollsHistoryForm
         case rolesAndPermissions
         
+        /// A child flow is in progress.
+        case presentingChild(childRoomID: String, previousState: State)
         /// The flow is complete and is handing control of the stack back to its parent.
         case complete
     }
@@ -1161,6 +1225,10 @@ private extension RoomFlowCoordinator {
         
         case presentRolesAndPermissionsScreen
         case dismissRolesAndPermissionsScreen
+        
+        // Child room flow events
+        case presentChildRoom(roomID: String)
+        case dismissChildRoom
     }
 }
 

ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift

@@ -421,6 +421,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
         
         let coordinator = await RoomFlowCoordinator(roomProxy: roomProxy,
                                                     userSession: userSession,
+                                                    isChildFlow: false,
                                                     roomTimelineControllerFactory: roomTimelineControllerFactory,
                                                     navigationStackCoordinator: detailNavigationStackCoordinator,
                                                     emojiProvider: EmojiProvider(),

ElementX/Sources/Generated/Strings.swift

@@ -1321,6 +1321,8 @@ internal enum L10n {
   internal static var screenRoomDetailsSecurityTitle: String { return L10n.tr("Localizable", "screen_room_details_security_title") }
   /// Share room
   internal static var screenRoomDetailsShareRoomTitle: String { return L10n.tr("Localizable", "screen_room_details_share_room_title") }
+  /// Room info
+  internal static var screenRoomDetailsTitle: String { return L10n.tr("Localizable", "screen_room_details_title") }
   /// Topic
   internal static var screenRoomDetailsTopicTitle: String { return L10n.tr("Localizable", "screen_room_details_topic_title") }
   /// Updating room…
@@ -1347,6 +1349,8 @@ internal enum L10n {
   internal static var screenRoomMemberDetailsBlockAlertDescription: String { return L10n.tr("Localizable", "screen_room_member_details_block_alert_description") }
   /// Block user
   internal static var screenRoomMemberDetailsBlockUser: String { return L10n.tr("Localizable", "screen_room_member_details_block_user") }
+  /// Profile
+  internal static var screenRoomMemberDetailsTitle: String { return L10n.tr("Localizable", "screen_room_member_details_title") }
   /// Unblock
   internal static var screenRoomMemberDetailsUnblockAlertAction: String { return L10n.tr("Localizable", "screen_room_member_details_unblock_alert_action") }
   /// You'll be able to see all messages from them again.

ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift

@@ -61,6 +61,7 @@ struct RoomDetailsScreen: View {
                 }
             }
         }
+        .navigationTitle(L10n.screenRoomDetailsTitle)
         .track(screen: .RoomDetails)
         .interactiveQuickLook(item: $context.mediaPreviewItem, shouldHideControls: true)
     }

ElementX/Sources/Screens/RoomMemberDetailsScreen/View/RoomMemberDetailsScreen.swift

@@ -23,6 +23,7 @@ struct RoomMemberDetailsScreen: View {
     var body: some View {
         content
             .compoundList()
+            .navigationTitle(L10n.screenRoomMemberDetailsTitle)
             .alert(item: $context.ignoreUserAlert, actions: blockUserAlertActions, message: blockUserAlertMessage)
             .alert(item: $context.alertInfo)
             .track(screen: .User)

ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift

@@ -47,6 +47,7 @@ struct RoomScreen: View {
                     .background(Color.compound.bgCanvasDefault.ignoresSafeArea())
                     .environmentObject(context)
             }
+            .navigationTitle(L10n.commonRoom) // Hidden but used for back button text.
             .navigationBarTitleDisplayMode(.inline)
             .navigationBarHidden(isNavigationBarHidden)
             .toolbar { toolbar }

UnitTests/Sources/RoomFlowCoordinatorTests.swift

@@ -25,31 +25,9 @@ class RoomFlowCoordinatorTests: XCTestCase {
     var navigationStackCoordinator: NavigationStackCoordinator!
     var cancellables = Set<AnyCancellable>()
     
-    override func setUp() async throws {
-        cancellables.removeAll()
-        let clientProxy = ClientProxyMock(.init(userID: "hi@bob", roomSummaryProvider: RoomSummaryProviderMock(.init(state: .loaded(.mockRooms)))))
-        let mediaProvider = MockMediaProvider()
-        let voiceMessageMediaManager = VoiceMessageMediaManagerMock()
-        let userSession = MockUserSession(clientProxy: clientProxy,
-                                          mediaProvider: mediaProvider,
-                                          voiceMessageMediaManager: voiceMessageMediaManager)
-        
-        let navigationSplitCoordinator = NavigationSplitCoordinator(placeholderCoordinator: PlaceholderScreenCoordinator())
-        navigationStackCoordinator = NavigationStackCoordinator()
-        navigationSplitCoordinator.setDetailCoordinator(navigationStackCoordinator)
-        
-        roomFlowCoordinator = await RoomFlowCoordinator(roomProxy: RoomProxyMock(with: .init(id: "1")),
-                                                        userSession: userSession,
-                                                        roomTimelineControllerFactory: MockRoomTimelineControllerFactory(),
-                                                        navigationStackCoordinator: navigationStackCoordinator,
-                                                        emojiProvider: EmojiProvider(),
-                                                        appSettings: ServiceLocator.shared.settings,
-                                                        analytics: ServiceLocator.shared.analytics,
-                                                        userIndicatorController: ServiceLocator.shared.userIndicatorController,
-                                                        orientationManager: OrientationManagerMock())
-    }
-    
     func testRoomPresentation() async throws {
+        await setupViewModel()
+        
         try await process(route: .room(roomID: "1"))
         XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator)
         
@@ -58,6 +36,8 @@ class RoomFlowCoordinatorTests: XCTestCase {
     }
     
     func testRoomDetailsPresentation() async throws {
+        await setupViewModel()
+        
         try await process(route: .roomDetails(roomID: "1"))
         XCTAssert(navigationStackCoordinator.rootCoordinator is RoomDetailsScreenCoordinator)
         
@@ -66,6 +46,8 @@ class RoomFlowCoordinatorTests: XCTestCase {
     }
     
     func testNoOp() async throws {
+        await setupViewModel()
+        
         try await process(route: .roomDetails(roomID: "1"))
         XCTAssert(navigationStackCoordinator.rootCoordinator is RoomDetailsScreenCoordinator)
         let detailsCoordinator = navigationStackCoordinator.rootCoordinator
@@ -78,6 +60,8 @@ class RoomFlowCoordinatorTests: XCTestCase {
     }
     
     func testPushDetails() async throws {
+        await setupViewModel()
+        
         try await process(route: .room(roomID: "1"))
         XCTAssert(navigationStackCoordinator.rootCoordinator is RoomScreenCoordinator)
         XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 0)
@@ -88,6 +72,24 @@ class RoomFlowCoordinatorTests: XCTestCase {
         XCTAssert(navigationStackCoordinator.stackCoordinators.first is RoomDetailsScreenCoordinator)
     }
     
+    func testChildFlowTearDown() async throws {
+        await setupViewModel(asChildFlow: true)
+        navigationStackCoordinator.setRootCoordinator(BlankFormCoordinator())
+        
+        try await process(route: .room(roomID: "1"))
+        try await process(route: .roomDetails(roomID: "1"))
+        XCTAssertTrue(navigationStackCoordinator.rootCoordinator is BlankFormCoordinator, "A child room flow should push onto the stack, leaving the root alone.")
+        XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 2)
+        XCTAssertTrue(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator)
+        XCTAssertTrue(navigationStackCoordinator.stackCoordinators.last is RoomDetailsScreenCoordinator)
+        
+        try await process(route: .roomList, expectedAction: .finished)
+        XCTAssertTrue(navigationStackCoordinator.rootCoordinator is BlankFormCoordinator)
+        XCTAssertEqual(navigationStackCoordinator.stackCoordinators.count, 2, "A child room flow should leave its parent to clean up the stack.")
+        XCTAssertTrue(navigationStackCoordinator.stackCoordinators.first is RoomScreenCoordinator, "A child room flow should leave its parent to clean up the stack.")
+        XCTAssertTrue(navigationStackCoordinator.stackCoordinators.last is RoomDetailsScreenCoordinator, "A child room flow should leave its parent to clean up the stack.")
+    }
+    
     // MARK: - Private
     
     private func process(route: AppRoute) async throws {
@@ -118,4 +120,29 @@ class RoomFlowCoordinatorTests: XCTestCase {
             try await fulfillment.fulfill()
         }
     }
+    
+    private func setupViewModel(asChildFlow: Bool = false) async {
+        cancellables.removeAll()
+        let clientProxy = ClientProxyMock(.init(userID: "hi@bob", roomSummaryProvider: RoomSummaryProviderMock(.init(state: .loaded(.mockRooms)))))
+        let mediaProvider = MockMediaProvider()
+        let voiceMessageMediaManager = VoiceMessageMediaManagerMock()
+        let userSession = MockUserSession(clientProxy: clientProxy,
+                                          mediaProvider: mediaProvider,
+                                          voiceMessageMediaManager: voiceMessageMediaManager)
+        
+        let navigationSplitCoordinator = NavigationSplitCoordinator(placeholderCoordinator: PlaceholderScreenCoordinator())
+        navigationStackCoordinator = NavigationStackCoordinator()
+        navigationSplitCoordinator.setDetailCoordinator(navigationStackCoordinator)
+        
+        roomFlowCoordinator = await RoomFlowCoordinator(roomProxy: RoomProxyMock(with: .init(id: "1")),
+                                                        userSession: userSession,
+                                                        isChildFlow: asChildFlow,
+                                                        roomTimelineControllerFactory: MockRoomTimelineControllerFactory(),
+                                                        navigationStackCoordinator: navigationStackCoordinator,
+                                                        emojiProvider: EmojiProvider(),
+                                                        appSettings: ServiceLocator.shared.settings,
+                                                        analytics: ServiceLocator.shared.analytics,
+                                                        userIndicatorController: ServiceLocator.shared.userIndicatorController,
+                                                        orientationManager: OrientationManagerMock())
+    }
 }

changelog.d/2587.change

@@ -0,0 +1 @@
+Allow a room to push another room onto the navigation stack instead of replacing itself.