diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index d3f91636b..910754018 100644 --- a/ElementX/Resources/Localizations/en.lproj/Localizable.strings +++ b/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"; diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 793df9299..a48a4b63a 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/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 = .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 - // Move the state machine to no room selected if the room currently being dismissed - // is the same as the one selected in the state machine. - // This generally happens when popping the room screen while in a compact layout - self?.stateMachine.tryEvent(.dismissRoom) + if !isChildFlow { + navigationStackCoordinator.setRootCoordinator(coordinator, animated: animated) { [weak self] in + 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) - navigationStackCoordinator.setRootCoordinator(nil, animated: false) + childRoomFlowCoordinator?.handleAppRoute(.roomList, animated: animated) + + 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. - - actionsSubject.send(.presentRoom(roomID: roomID)) + // 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. + 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 } } diff --git a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift index 39ea605a9..247ed1a76 100644 --- a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift +++ b/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(), diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift index e0c0fd933..0fbd9a536 100644 --- a/ElementX/Sources/Generated/Strings.swift +++ b/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. diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift b/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift index a8ad7af1b..ac6c696f9 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift +++ b/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) } diff --git a/ElementX/Sources/Screens/RoomMemberDetailsScreen/View/RoomMemberDetailsScreen.swift b/ElementX/Sources/Screens/RoomMemberDetailsScreen/View/RoomMemberDetailsScreen.swift index 09989513d..10159ec7a 100644 --- a/ElementX/Sources/Screens/RoomMemberDetailsScreen/View/RoomMemberDetailsScreen.swift +++ b/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) diff --git a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift index 3e323c585..bb926ec07 100644 --- a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift +++ b/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 } diff --git a/UnitTests/Sources/RoomFlowCoordinatorTests.swift b/UnitTests/Sources/RoomFlowCoordinatorTests.swift index a3f27770e..88503e498 100644 --- a/UnitTests/Sources/RoomFlowCoordinatorTests.swift +++ b/UnitTests/Sources/RoomFlowCoordinatorTests.swift @@ -25,31 +25,9 @@ class RoomFlowCoordinatorTests: XCTestCase { var navigationStackCoordinator: NavigationStackCoordinator! var cancellables = Set() - 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()) + } } diff --git a/changelog.d/2587.change b/changelog.d/2587.change new file mode 100644 index 000000000..23a337c58 --- /dev/null +++ b/changelog.d/2587.change @@ -0,0 +1 @@ +Allow a room to push another room onto the navigation stack instead of replacing itself. \ No newline at end of file