From 26eda3ccfde61da4ac1b3407cf58a0019b1714ef Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Wed, 3 Sep 2025 17:01:07 +0100 Subject: [PATCH] Move call presentation from the chats flow into the user session flow. (#4459) --- .../Navigation/NavigationCoordinators.swift | 75 ------------ .../NavigationRootCoordinator.swift | 1 + .../Navigation/NavigationTabCoordinator.swift | 77 ++++++++++++ .../ChatsFlowCoordinator.swift | 112 ++---------------- .../RoomFlowCoordinator.swift | 16 +-- .../UserSessionFlowCoordinator.swift | 106 ++++++++++++++++- .../RoomMemberDetailsScreenCoordinator.swift | 6 +- .../RoomMemberDetailsScreenModels.swift | 2 +- .../RoomMemberDetailsScreenViewModel.swift | 26 +++- .../UserProfileScreenCoordinator.swift | 6 +- .../UserProfileScreenModels.swift | 2 +- .../UserProfileScreenViewModel.swift | 26 +++- .../NavigationRootCoordinatorTests.swift | 2 + .../NavigationSplitCoordinatorTests.swift | 51 +------- .../NavigationStackCoordinatorTests.swift | 2 + .../NavigationTabCoordinatorTests.swift | 49 ++++++++ 16 files changed, 299 insertions(+), 260 deletions(-) diff --git a/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift b/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift index b399d1c1e..71b97d90d 100644 --- a/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift +++ b/ElementX/Sources/Application/Navigation/NavigationCoordinators.swift @@ -90,28 +90,6 @@ import SwiftUI fullScreenCoverModule?.coordinator } - fileprivate var overlayModule: NavigationModule? { - didSet { - if let oldValue { - logPresentationChange("Remove overlay", oldValue) - oldValue.tearDown() - } - - if let overlayModule { - logPresentationChange("Set overlay", overlayModule) - overlayModule.coordinator?.start() - } - } - } - - /// The currently displayed overlay coordinator - var overlayCoordinator: (any CoordinatorProtocol)? { - overlayModule?.coordinator - } - - enum OverlayPresentationMode { case fullScreen, minimized } - fileprivate var overlayPresentationMode: OverlayPresentationMode = .minimized - fileprivate var compactLayoutRootModule: NavigationModule? { if let sidebarNavigationStackCoordinator = sidebarModule?.coordinator as? NavigationStackCoordinator { if let sidebarRootModule = sidebarNavigationStackCoordinator.rootModule { @@ -273,47 +251,6 @@ import SwiftUI fullScreenCoverModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) } } - - /// Present an overlay on top of the split view - /// - Parameters: - /// - coordinator: the coordinator to display - /// - presentationMode: how the coordinator should be presented - /// - animated: whether the transition should be animated - /// - dismissalCallback: called when the overlay has been dismissed, programatically or otherwise - func setOverlayCoordinator(_ coordinator: (any CoordinatorProtocol)?, - presentationMode: OverlayPresentationMode = .fullScreen, - animated: Bool = true, - dismissalCallback: (() -> Void)? = nil) { - guard let coordinator else { - overlayModule = nil - return - } - - if overlayModule?.coordinator === coordinator { - fatalError("Cannot use the same coordinator more than once") - } - - var transaction = Transaction() - transaction.disablesAnimations = !animated - - withTransaction(transaction) { - overlayPresentationMode = presentationMode - overlayModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) - } - } - - /// Updates the presentation of the overlay coordinator. - /// - Parameters: - /// - mode: The type of presentation to use. - /// - animated: whether the transition should be animated - func setOverlayPresentationMode(_ mode: OverlayPresentationMode, animated: Bool = true) { - var transaction = Transaction() - transaction.disablesAnimations = !animated - - withTransaction(transaction) { - overlayPresentationMode = mode - } - } // MARK: - CoordinatorProtocol @@ -415,18 +352,6 @@ private struct NavigationSplitCoordinatorView: View { module.coordinator?.toPresentable() .id(module.id) } - .accessibilityHidden(navigationSplitCoordinator.overlayModule?.coordinator != nil && navigationSplitCoordinator.overlayPresentationMode == .fullScreen) - .overlay { - Group { - if let coordinator = navigationSplitCoordinator.overlayModule?.coordinator { - coordinator.toPresentable() - .opacity(navigationSplitCoordinator.overlayPresentationMode == .minimized ? 0 : 1) - .transition(.opacity) - } - } - .animation(.elementDefault, value: navigationSplitCoordinator.overlayPresentationMode) - .animation(.elementDefault, value: navigationSplitCoordinator.overlayModule) - } .ignoresSafeArea() // Necessary when embedded in a TabView on iPadOS otherwise there's a gap at the top (as of 18.5). } diff --git a/ElementX/Sources/Application/Navigation/NavigationRootCoordinator.swift b/ElementX/Sources/Application/Navigation/NavigationRootCoordinator.swift index 2b9ddaab3..45db8ce0a 100644 --- a/ElementX/Sources/Application/Navigation/NavigationRootCoordinator.swift +++ b/ElementX/Sources/Application/Navigation/NavigationRootCoordinator.swift @@ -162,6 +162,7 @@ private struct NavigationRootCoordinatorView: View { .sheet(item: $rootCoordinator.sheetModule) { module in module.coordinator?.toPresentable() } + .accessibilityHidden(rootCoordinator.overlayModule?.coordinator != nil) .overlay { Group { if let coordinator = rootCoordinator.overlayModule?.coordinator { diff --git a/ElementX/Sources/Application/Navigation/NavigationTabCoordinator.swift b/ElementX/Sources/Application/Navigation/NavigationTabCoordinator.swift index 19daf9bed..399fbe228 100644 --- a/ElementX/Sources/Application/Navigation/NavigationTabCoordinator.swift +++ b/ElementX/Sources/Application/Navigation/NavigationTabCoordinator.swift @@ -190,6 +190,71 @@ import SwiftUI } } + // MARK: - Overlay + + fileprivate var overlayModule: NavigationModule? { + didSet { + if let oldValue { + logPresentationChange("Remove overlay", oldValue) + oldValue.tearDown() + } + + if let overlayModule { + logPresentationChange("Set overlay", overlayModule) + overlayModule.coordinator?.start() + } + } + } + + /// The currently displayed overlay coordinator + var overlayCoordinator: (any CoordinatorProtocol)? { + overlayModule?.coordinator + } + + enum OverlayPresentationMode { case fullScreen, minimized } + fileprivate var overlayPresentationMode: OverlayPresentationMode = .minimized + + /// Present an overlay on top of the tab view + /// - Parameters: + /// - coordinator: the coordinator to display + /// - presentationMode: how the coordinator should be presented + /// - animated: whether the transition should be animated + /// - dismissalCallback: called when the overlay has been dismissed, programatically or otherwise + func setOverlayCoordinator(_ coordinator: (any CoordinatorProtocol)?, + presentationMode: OverlayPresentationMode = .fullScreen, + animated: Bool = true, + dismissalCallback: (() -> Void)? = nil) { + guard let coordinator else { + overlayModule = nil + return + } + + if overlayModule?.coordinator === coordinator { + fatalError("Cannot use the same coordinator more than once") + } + + var transaction = Transaction() + transaction.disablesAnimations = !animated + + withTransaction(transaction) { + overlayPresentationMode = presentationMode + overlayModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback) + } + } + + /// Updates the presentation of the overlay coordinator. + /// - Parameters: + /// - mode: The type of presentation to use. + /// - animated: whether the transition should be animated + func setOverlayPresentationMode(_ mode: OverlayPresentationMode, animated: Bool = true) { + var transaction = Transaction() + transaction.disablesAnimations = !animated + + withTransaction(transaction) { + overlayPresentationMode = mode + } + } + // MARK: - CoordinatorProtocol /// No idea if this is particuarly needed for the TabView but we do this for the NavigationStackCoordinator and NavigationSplitCoordinator so it @@ -251,6 +316,18 @@ private struct NavigationTabCoordinatorView: View { module.coordinator?.toPresentable() .id(module.id) } + .accessibilityHidden(navigationTabCoordinator.overlayModule?.coordinator != nil && navigationTabCoordinator.overlayPresentationMode == .fullScreen) + .overlay { + Group { + if let coordinator = navigationTabCoordinator.overlayModule?.coordinator { + coordinator.toPresentable() + .opacity(navigationTabCoordinator.overlayPresentationMode == .minimized ? 0 : 1) + .transition(.opacity) + } + } + .animation(.elementDefault, value: navigationTabCoordinator.overlayPresentationMode) + .animation(.elementDefault, value: navigationTabCoordinator.overlayModule) + } } private func configureAppearance(_ tabBarController: UITabBarController) { diff --git a/ElementX/Sources/FlowCoordinators/ChatsFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/ChatsFlowCoordinator.swift index a85ab173c..5ca38e15e 100644 --- a/ElementX/Sources/FlowCoordinators/ChatsFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/ChatsFlowCoordinator.swift @@ -6,7 +6,6 @@ // import AnalyticsEvents -import AVKit import Combine import MatrixRustSDK import SwiftUI @@ -15,6 +14,8 @@ enum ChatsFlowCoordinatorAction { case showSettings case showChatBackupSettings case sessionVerification(SessionVerificationScreenFlow) + case showCallScreen(roomProxy: JoinedRoomProxyProtocol) + case hideCallScreenOverlay case logout } @@ -143,10 +144,6 @@ class ChatsFlowCoordinator: FlowCoordinatorProtocol { case .userProfile(let userID): stateMachine.processEvent(.showUserProfileScreen(userID: userID), userInfo: .init(animated: animated)) - case .call(let roomID): - Task { await presentCallScreen(roomID: roomID) } - case .genericCallLink(let url): - presentCallScreen(genericCallLink: url) case .share(let payload): if let roomID = payload.roomID { stateMachine.processEvent(.selectRoom(roomID: roomID, @@ -162,7 +159,7 @@ class ChatsFlowCoordinator: FlowCoordinatorProtocol { } else { stateMachine.processEvent(.selectRoom(roomID: roomID, via: [], entryPoint: .transferOwnership)) } - case .accountProvisioningLink, .settings, .chatBackupSettings: + case .accountProvisioningLink, .settings, .chatBackupSettings, .call, .genericCallLink: break // These routes cannot be handled. } } @@ -200,7 +197,7 @@ class ChatsFlowCoordinator: FlowCoordinatorProtocol { } else { startRoomFlow(roomID: roomID, via: via, entryPoint: entryPoint, animated: animated) } - hideCallScreenOverlay() // Turn any active call into a PiP so that navigation from a notification is visible to the user. + actionsSubject.send(.hideCallScreenOverlay) // Turn any active call into a PiP so that navigation from a notification is visible to the user. case(.roomList, .deselectRoom, .roomList): dismissRoomFlow(animated: animated) @@ -302,18 +299,6 @@ class ChatsFlowCoordinator: FlowCoordinatorProtocol { } } .store(in: &cancellables) - - flowParameters.elementCallService.actions - .receive(on: DispatchQueue.main) - .sink { [weak self] action in - switch action { - case .endCall: - self?.dismissCallScreenIfNeeded() - default: - break - } - } - .store(in: &cancellables) } private func processDecryptionError(_ info: UnableToDecryptInfo) { @@ -465,7 +450,7 @@ class ChatsFlowCoordinator: FlowCoordinatorProtocol { switch action { case .presentCallScreen(let roomProxy): - presentCallScreen(roomProxy: roomProxy) + actionsSubject.send(.showCallScreen(roomProxy: roomProxy)) case .verifyUser(let userID): actionsSubject.send(.sessionVerification(.userInitiator(userID: userID))) case .finished: @@ -543,89 +528,6 @@ class ChatsFlowCoordinator: FlowCoordinatorProtocol { self?.stateMachine.processEvent(.dismissedStartChatScreen) } } - - // MARK: Calls - - private func presentCallScreen(genericCallLink url: URL) { - presentCallScreen(configuration: .init(genericCallLink: url)) - } - - private func presentCallScreen(roomID: String) async { - guard case let .joined(roomProxy) = await userSession.clientProxy.roomForIdentifier(roomID) else { - return - } - - presentCallScreen(roomProxy: roomProxy) - } - - private func presentCallScreen(roomProxy: JoinedRoomProxyProtocol) { - let colorScheme: ColorScheme = flowParameters.windowManager.mainWindow.traitCollection.userInterfaceStyle == .light ? .light : .dark - presentCallScreen(configuration: .init(roomProxy: roomProxy, - clientProxy: userSession.clientProxy, - clientID: InfoPlistReader.main.bundleIdentifier, - elementCallBaseURL: flowParameters.appSettings.elementCallBaseURL, - elementCallBaseURLOverride: flowParameters.appSettings.elementCallBaseURLOverride, - colorScheme: colorScheme)) - } - - private var callScreenPictureInPictureController: AVPictureInPictureController? - private func presentCallScreen(configuration: ElementCallConfiguration) { - guard flowParameters.ongoingCallRoomIDPublisher.value != configuration.callRoomID else { - MXLog.info("Returning to existing call.") - callScreenPictureInPictureController?.stopPictureInPicture() - return - } - - let callScreenCoordinator = CallScreenCoordinator(parameters: .init(elementCallService: flowParameters.elementCallService, - configuration: configuration, - allowPictureInPicture: true, - appSettings: flowParameters.appSettings, - appHooks: flowParameters.appHooks, - analytics: flowParameters.analytics)) - - callScreenCoordinator.actions - .sink { [weak self] action in - guard let self else { return } - switch action { - case .pictureInPictureIsAvailable(let controller): - callScreenPictureInPictureController = controller - case .pictureInPictureStarted: - MXLog.info("Hiding call for PiP presentation.") - navigationSplitCoordinator.setOverlayPresentationMode(.minimized) - case .pictureInPictureStopped: - MXLog.info("Restoring call after PiP presentation.") - navigationSplitCoordinator.setOverlayPresentationMode(.fullScreen) - case .dismiss: - callScreenPictureInPictureController = nil - navigationSplitCoordinator.setOverlayCoordinator(nil) - } - } - .store(in: &cancellables) - - navigationSplitCoordinator.setOverlayCoordinator(callScreenCoordinator, animated: true) - - flowParameters.analytics.track(screen: .RoomCall) - } - - private func hideCallScreenOverlay() { - guard let callScreenPictureInPictureController else { - MXLog.warning("Picture in picture isn't available, dismissing the call screen.") - dismissCallScreenIfNeeded() - return - } - - MXLog.info("Starting picture in picture to hide the call screen overlay.") - callScreenPictureInPictureController.startPictureInPicture() - navigationSplitCoordinator.setOverlayPresentationMode(.minimized) - } - - private func dismissCallScreenIfNeeded() { - guard navigationSplitCoordinator.overlayCoordinator is CallScreenCoordinator else { - return - } - - navigationSplitCoordinator.setOverlayCoordinator(nil) - } // MARK: Secure backup @@ -768,8 +670,8 @@ class ChatsFlowCoordinator: FlowCoordinatorProtocol { case .openDirectChat(let roomID): navigationSplitCoordinator.setSheetCoordinator(nil) stateMachine.processEvent(.selectRoom(roomID: roomID, via: [], entryPoint: .room)) - case .startCall(let roomID): - Task { await self.presentCallScreen(roomID: roomID) } + case .startCall(let roomProxy): + actionsSubject.send(.showCallScreen(roomProxy: roomProxy)) case .dismiss: navigationSplitCoordinator.setSheetCoordinator(nil) } diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index ac24d289f..b3fb064ea 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -215,14 +215,6 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { navigationStackCoordinator.setSheetCoordinator(stackCoordinator, animated: true) } - private func presentCallScreen(roomID: String) async { - guard case let .joined(roomProxy) = await userSession.clientProxy.roomForIdentifier(roomID) else { - return - } - - actionsSubject.send(.presentCallScreen(roomProxy: roomProxy)) - } - private func handleRoomRoute(roomID: String, via: [String], presentationAction: PresentationAction? = nil, animated: Bool) async { guard roomID == self.roomID else { fatalError("Navigation route doesn't belong to this room flow.") } @@ -1128,8 +1120,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { stateMachine.tryEvent(.presentUserProfile(userID: userID)) case .openDirectChat(let roomID): stateMachine.tryEvent(.startChildFlow(roomID: roomID, via: [], entryPoint: .room)) - case .startCall(let roomID): - Task { await self.presentCallScreen(roomID: roomID) } + case .startCall(let roomProxy): + actionsSubject.send(.presentCallScreen(roomProxy: roomProxy)) case .verifyUser(let userID): actionsSubject.send(.verifyUser(userID: userID)) } @@ -1154,8 +1146,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { switch action { case .openDirectChat(let roomID): stateMachine.tryEvent(.startChildFlow(roomID: roomID, via: [], entryPoint: .room)) - case .startCall(let roomID): - Task { await self.presentCallScreen(roomID: roomID) } + case .startCall(let roomProxy): + actionsSubject.send(.presentCallScreen(roomProxy: roomProxy)) case .dismiss: break // Not supported when pushed. } diff --git a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift index 220e7e0c7..32087e997 100644 --- a/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift @@ -5,6 +5,7 @@ // Please see LICENSE files in the repository root for full details. // +import AVKit import Combine import Compound import MatrixRustSDK @@ -127,10 +128,14 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol { stateMachine.tryEvent(.showSettingsScreen) } settingsFlowCoordinator?.handleAppRoute(appRoute, animated: animated) + case .call(let roomID): + Task { await presentCallScreen(roomID: roomID) } + case .genericCallLink(let url): + presentCallScreen(genericCallLink: url) case .roomList, .room, .roomAlias, .childRoom, .childRoomAlias, .roomDetails, .roomMemberDetails, .userProfile, .event, .eventOnRoomAlias, .childEvent, .childEventOnRoomAlias, - .call, .genericCallLink, .share, .transferOwnership: + .share, .transferOwnership: clearPresentedSheets(animated: animated) // Make sure the presented route is visible. chatsFlowCoordinator.handleAppRoute(appRoute, animated: animated) if navigationTabCoordinator.selectedTab != .chats { @@ -198,6 +203,10 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol { handleAppRoute(.chatBackupSettings, animated: true) case .sessionVerification(let flow): presentSessionVerificationScreen(flow: flow) + case .showCallScreen(let roomProxy): + presentCallScreen(roomProxy: roomProxy) + case .hideCallScreenOverlay: + hideCallScreenOverlay() case .logout: Task { await self.runLogoutFlow() } } @@ -241,6 +250,18 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol { } .store(in: &cancellables) + flowParameters.elementCallService.actions + .receive(on: DispatchQueue.main) + .sink { [weak self] action in + switch action { + case .endCall: + self?.dismissCallScreenIfNeeded() + default: + break + } + } + .store(in: &cancellables) + flowParameters.appSettings.$spacesEnabled .combineLatest(userSession.clientProxy.spaceService.joinedSpacesPublisher) .map { $0 && !$1.isEmpty ? nil : .hidden } @@ -346,6 +367,89 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol { navigationTabCoordinator.setSheetCoordinator(navigationStackCoordinator) } + // MARK: - Calls + + private func presentCallScreen(genericCallLink url: URL) { + presentCallScreen(configuration: .init(genericCallLink: url)) + } + + private func presentCallScreen(roomID: String) async { + guard case let .joined(roomProxy) = await userSession.clientProxy.roomForIdentifier(roomID) else { + return + } + + presentCallScreen(roomProxy: roomProxy) + } + + private func presentCallScreen(roomProxy: JoinedRoomProxyProtocol) { + let colorScheme: ColorScheme = flowParameters.windowManager.mainWindow.traitCollection.userInterfaceStyle == .light ? .light : .dark + presentCallScreen(configuration: .init(roomProxy: roomProxy, + clientProxy: userSession.clientProxy, + clientID: InfoPlistReader.main.bundleIdentifier, + elementCallBaseURL: flowParameters.appSettings.elementCallBaseURL, + elementCallBaseURLOverride: flowParameters.appSettings.elementCallBaseURLOverride, + colorScheme: colorScheme)) + } + + private var callScreenPictureInPictureController: AVPictureInPictureController? + private func presentCallScreen(configuration: ElementCallConfiguration) { + guard flowParameters.ongoingCallRoomIDPublisher.value != configuration.callRoomID else { + MXLog.info("Returning to existing call.") + callScreenPictureInPictureController?.stopPictureInPicture() + return + } + + let callScreenCoordinator = CallScreenCoordinator(parameters: .init(elementCallService: flowParameters.elementCallService, + configuration: configuration, + allowPictureInPicture: true, + appSettings: flowParameters.appSettings, + appHooks: flowParameters.appHooks, + analytics: flowParameters.analytics)) + + callScreenCoordinator.actions + .sink { [weak self] action in + guard let self else { return } + switch action { + case .pictureInPictureIsAvailable(let controller): + callScreenPictureInPictureController = controller + case .pictureInPictureStarted: + MXLog.info("Hiding call for PiP presentation.") + navigationTabCoordinator.setOverlayPresentationMode(.minimized) + case .pictureInPictureStopped: + MXLog.info("Restoring call after PiP presentation.") + navigationTabCoordinator.setOverlayPresentationMode(.fullScreen) + case .dismiss: + callScreenPictureInPictureController = nil + navigationTabCoordinator.setOverlayCoordinator(nil) + } + } + .store(in: &cancellables) + + navigationTabCoordinator.setOverlayCoordinator(callScreenCoordinator, animated: true) + + flowParameters.analytics.track(screen: .RoomCall) + } + + private func hideCallScreenOverlay() { + guard let callScreenPictureInPictureController else { + MXLog.warning("Picture in picture isn't available, dismissing the call screen.") + dismissCallScreenIfNeeded() + return + } + + MXLog.info("Starting picture in picture to hide the call screen overlay.") + callScreenPictureInPictureController.startPictureInPicture() + navigationTabCoordinator.setOverlayPresentationMode(.minimized) + } + + private func dismissCallScreenIfNeeded() { + guard navigationTabCoordinator.overlayCoordinator is CallScreenCoordinator else { + return + } + + navigationTabCoordinator.setOverlayCoordinator(nil) + } + // MARK: - Logout private func runLogoutFlow() async { diff --git a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift index ff79ad2e0..844b40a3e 100644 --- a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift +++ b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift @@ -19,7 +19,7 @@ struct RoomMemberDetailsScreenCoordinatorParameters { enum RoomMemberDetailsScreenCoordinatorAction { case openUserProfile case openDirectChat(roomID: String) - case startCall(roomID: String) + case startCall(roomProxy: JoinedRoomProxyProtocol) case verifyUser(userID: String) } @@ -50,8 +50,8 @@ final class RoomMemberDetailsScreenCoordinator: CoordinatorProtocol { actionsSubject.send(.openUserProfile) case .openDirectChat(let roomID): actionsSubject.send(.openDirectChat(roomID: roomID)) - case .startCall(let roomID): - actionsSubject.send(.startCall(roomID: roomID)) + case .startCall(let roomProxy): + actionsSubject.send(.startCall(roomProxy: roomProxy)) case .verifyUser(let userID): actionsSubject.send(.verifyUser(userID: userID)) } diff --git a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenModels.swift b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenModels.swift index 3e49d7914..74581f187 100644 --- a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenModels.swift +++ b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenModels.swift @@ -10,7 +10,7 @@ import Foundation enum RoomMemberDetailsScreenViewModelAction { case openUserProfile case openDirectChat(roomID: String) - case startCall(roomID: String) + case startCall(roomProxy: JoinedRoomProxyProtocol) case verifyUser(userID: String) } diff --git a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift index 1e0e77b39..0784d4d59 100644 --- a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift @@ -81,7 +81,7 @@ class RoomMemberDetailsScreenViewModel: RoomMemberDetailsScreenViewModelType, Ro case .createDirectChat: Task { await createDirectChat() } case .startCall(let roomID): - actionsSubject.send(.startCall(roomID: roomID)) + Task { await startCall(roomID: roomID) } case .verifyUser: actionsSubject.send(.verifyUser(userID: state.userID)) case .withdrawVerification: @@ -224,12 +224,21 @@ class RoomMemberDetailsScreenViewModel: RoomMemberDetailsScreenViewModelType, Ro } } - // MARK: Loading indicator + private func startCall(roomID: String) async { + guard case let .joined(roomProxy) = await userSession.clientProxy.roomForIdentifier(roomID) else { + showErrorIndicator() + return + } + actionsSubject.send(.startCall(roomProxy: roomProxy)) + } - private static let loadingIndicatorIdentifier = "\(RoomMemberDetailsScreenViewModel.self)-Loading" + // MARK: User Indicators + + private var loadingIndicatorIdentifier: String { "\(Self.self)-Loading" } + private var statusIndicatorIdentifier: String { "\(Self.self)-Status" } private func showMemberLoadingIndicator() { - userIndicatorController.submitIndicator(UserIndicator(id: Self.loadingIndicatorIdentifier, + userIndicatorController.submitIndicator(UserIndicator(id: loadingIndicatorIdentifier, type: .modal(progress: .indeterminate, interactiveDismissDisabled: false, allowsInteraction: true), title: L10n.commonLoading, persistent: true), @@ -237,6 +246,13 @@ class RoomMemberDetailsScreenViewModel: RoomMemberDetailsScreenViewModelType, Ro } private func hideMemberLoadingIndicator() { - userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorIdentifier) + userIndicatorController.retractIndicatorWithId(loadingIndicatorIdentifier) + } + + private func showErrorIndicator() { + userIndicatorController.submitIndicator(UserIndicator(id: statusIndicatorIdentifier, + type: .toast, + title: L10n.errorUnknown, + iconName: "xmark")) } } diff --git a/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenCoordinator.swift b/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenCoordinator.swift index 699db7bf2..2ff4743f8 100644 --- a/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenCoordinator.swift +++ b/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenCoordinator.swift @@ -18,7 +18,7 @@ struct UserProfileScreenCoordinatorParameters { enum UserProfileScreenCoordinatorAction { case openDirectChat(roomID: String) - case startCall(roomID: String) + case startCall(roomProxy: JoinedRoomProxyProtocol) case dismiss } @@ -47,8 +47,8 @@ final class UserProfileScreenCoordinator: CoordinatorProtocol { switch action { case .openDirectChat(let roomID): actionsSubject.send(.openDirectChat(roomID: roomID)) - case .startCall(let roomID): - actionsSubject.send(.startCall(roomID: roomID)) + case .startCall(let roomProxy): + actionsSubject.send(.startCall(roomProxy: roomProxy)) case .dismiss: actionsSubject.send(.dismiss) } diff --git a/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenModels.swift b/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenModels.swift index 03eea9491..3b8d88ab4 100644 --- a/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenModels.swift +++ b/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenModels.swift @@ -9,7 +9,7 @@ import Foundation enum UserProfileScreenViewModelAction { case openDirectChat(roomID: String) - case startCall(roomID: String) + case startCall(roomProxy: JoinedRoomProxyProtocol) case dismiss } diff --git a/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenViewModel.swift b/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenViewModel.swift index 941066dd8..533557bb7 100644 --- a/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenViewModel.swift +++ b/ElementX/Sources/Screens/UserProfileScreen/UserProfileScreenViewModel.swift @@ -62,7 +62,7 @@ class UserProfileScreenViewModel: UserProfileScreenViewModelType, UserProfileScr case .createDirectChat: Task { await createDirectChat() } case .startCall(let roomID): - actionsSubject.send(.startCall(roomID: roomID)) + Task { await startCall(roomID: roomID) } case .dismiss: actionsSubject.send(.dismiss) } @@ -143,12 +143,21 @@ class UserProfileScreenViewModel: UserProfileScreenViewModelType, UserProfileScr } } - // MARK: Loading indicator + private func startCall(roomID: String) async { + guard case let .joined(roomProxy) = await userSession.clientProxy.roomForIdentifier(roomID) else { + showErrorIndicator() + return + } + actionsSubject.send(.startCall(roomProxy: roomProxy)) + } - private static let loadingIndicatorIdentifier = "\(UserProfileScreenViewModel.self)-Loading" + // MARK: User Indicators + + private var loadingIndicatorIdentifier: String { "\(Self.self)-Loading" } + private var statusIndicatorIdentifier: String { "\(Self.self)-Status" } private func showLoadingIndicator(allowsInteraction: Bool) { - userIndicatorController.submitIndicator(UserIndicator(id: Self.loadingIndicatorIdentifier, + userIndicatorController.submitIndicator(UserIndicator(id: loadingIndicatorIdentifier, type: .modal(progress: .indeterminate, interactiveDismissDisabled: false, allowsInteraction: allowsInteraction), title: L10n.commonLoading, persistent: true), @@ -156,6 +165,13 @@ class UserProfileScreenViewModel: UserProfileScreenViewModelType, UserProfileScr } private func hideLoadingIndicator() { - userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorIdentifier) + userIndicatorController.retractIndicatorWithId(loadingIndicatorIdentifier) + } + + private func showErrorIndicator() { + userIndicatorController.submitIndicator(UserIndicator(id: statusIndicatorIdentifier, + type: .toast, + title: L10n.errorUnknown, + iconName: "xmark")) } } diff --git a/UnitTests/Sources/NavigationRootCoordinatorTests.swift b/UnitTests/Sources/NavigationRootCoordinatorTests.swift index 2e1c2f0d3..e6e7de794 100644 --- a/UnitTests/Sources/NavigationRootCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationRootCoordinatorTests.swift @@ -47,6 +47,8 @@ class NavigationRootCoordinatorTests: XCTestCase { XCTAssertNil(navigationRootCoordinator.overlayCoordinator) } + // MARK: - Dismissal Callbacks + func testReplacementDismissalCallbacks() { XCTAssertNil(navigationRootCoordinator.rootCoordinator) diff --git a/UnitTests/Sources/NavigationSplitCoordinatorTests.swift b/UnitTests/Sources/NavigationSplitCoordinatorTests.swift index 4e5d9c332..64deb4b2e 100644 --- a/UnitTests/Sources/NavigationSplitCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationSplitCoordinatorTests.swift @@ -110,31 +110,7 @@ class NavigationSplitCoordinatorTests: XCTestCase { XCTAssertNil(navigationSplitCoordinator.fullScreenCoverCoordinator) } - func testOverlay() { - let sidebarCoordinator = SomeTestCoordinator() - navigationSplitCoordinator.setSidebarCoordinator(sidebarCoordinator) - let detailCoordinator = SomeTestCoordinator() - navigationSplitCoordinator.setDetailCoordinator(detailCoordinator) - - let overlayCoordinator = SomeTestCoordinator() - navigationSplitCoordinator.setOverlayCoordinator(overlayCoordinator) - - assertCoordinatorsEqual(sidebarCoordinator, navigationSplitCoordinator.sidebarCoordinator) - assertCoordinatorsEqual(detailCoordinator, navigationSplitCoordinator.detailCoordinator) - assertCoordinatorsEqual(overlayCoordinator, navigationSplitCoordinator.overlayCoordinator) - - // The coordinator should still be retained when changing the presentation mode. - navigationSplitCoordinator.setOverlayPresentationMode(.minimized) - assertCoordinatorsEqual(overlayCoordinator, navigationSplitCoordinator.overlayCoordinator) - navigationSplitCoordinator.setOverlayPresentationMode(.fullScreen) - assertCoordinatorsEqual(overlayCoordinator, navigationSplitCoordinator.overlayCoordinator) - - navigationSplitCoordinator.setOverlayCoordinator(nil) - - assertCoordinatorsEqual(sidebarCoordinator, navigationSplitCoordinator.sidebarCoordinator) - assertCoordinatorsEqual(detailCoordinator, navigationSplitCoordinator.detailCoordinator) - XCTAssertNil(navigationSplitCoordinator.overlayCoordinator) - } + // MARK: - Dismissal Callbacks func testSidebarReplacementCallbacks() { let sidebarCoordinator = SomeTestCoordinator() @@ -184,30 +160,7 @@ class NavigationSplitCoordinatorTests: XCTestCase { waitForExpectations(timeout: 1.0) } - func testOverlayDismissalCallback() { - let overlayCoordinator = SomeTestCoordinator() - - let expectation = expectation(description: "Wait for callback") - navigationSplitCoordinator.setOverlayCoordinator(overlayCoordinator) { - expectation.fulfill() - } - - navigationSplitCoordinator.setOverlayCoordinator(nil) - waitForExpectations(timeout: 1.0) - } - - func testOverlayDismissalCallbackWhenChangingMode() { - let overlayCoordinator = SomeTestCoordinator() - - let expectation = expectation(description: "Wait for callback") - expectation.isInverted = true - navigationSplitCoordinator.setOverlayCoordinator(overlayCoordinator) { - expectation.fulfill() - } - - navigationSplitCoordinator.setOverlayPresentationMode(.minimized) - waitForExpectations(timeout: 1.0) - } + // MARK: - Advanced func testEmbeddedStackPresentsSheetThroughSplit() { let sidebarNavigationStackCoordinator = NavigationStackCoordinator(navigationSplitCoordinator: navigationSplitCoordinator) diff --git a/UnitTests/Sources/NavigationStackCoordinatorTests.swift b/UnitTests/Sources/NavigationStackCoordinatorTests.swift index b65c32e75..f1127a885 100644 --- a/UnitTests/Sources/NavigationStackCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationStackCoordinatorTests.swift @@ -140,6 +140,8 @@ class NavigationStackCoordinatorTests: XCTestCase { assertCoordinatorsEqual(sheetCoordinator, navigationStackCoordinator.sheetCoordinator) } + // MARK: - Dismissal Callbacks + func testPopDismissalCallbacks() { let pushedCoordinator = SomeTestCoordinator() diff --git a/UnitTests/Sources/NavigationTabCoordinatorTests.swift b/UnitTests/Sources/NavigationTabCoordinatorTests.swift index 6ebb78fbe..dbfb61ed9 100644 --- a/UnitTests/Sources/NavigationTabCoordinatorTests.swift +++ b/UnitTests/Sources/NavigationTabCoordinatorTests.swift @@ -83,6 +83,30 @@ class NavigationTabCoordinatorTests: XCTestCase { XCTAssertNil(navigationTabCoordinator.fullScreenCoverCoordinator) } + func testOverlay() { + let tabCoordinator = SomeTestCoordinator() + navigationTabCoordinator.setTabs([.init(coordinator: tabCoordinator, details: .init(tag: .tab, title: "Tab", icon: \.help, selectedIcon: \.helpSolid))]) + + let overlayCoordinator = SomeTestCoordinator() + navigationTabCoordinator.setOverlayCoordinator(overlayCoordinator) + + assertCoordinatorsEqual(navigationTabCoordinator.tabCoordinators, [tabCoordinator]) + assertCoordinatorsEqual(overlayCoordinator, navigationTabCoordinator.overlayCoordinator) + + // The coordinator should still be retained when changing the presentation mode. + navigationTabCoordinator.setOverlayPresentationMode(.minimized) + assertCoordinatorsEqual(overlayCoordinator, navigationTabCoordinator.overlayCoordinator) + navigationTabCoordinator.setOverlayPresentationMode(.fullScreen) + assertCoordinatorsEqual(overlayCoordinator, navigationTabCoordinator.overlayCoordinator) + + navigationTabCoordinator.setOverlayCoordinator(nil) + + assertCoordinatorsEqual(navigationTabCoordinator.tabCoordinators, [tabCoordinator]) + XCTAssertNil(navigationTabCoordinator.overlayCoordinator) + } + + // MARK: - Dismissal Callbacks + func testTabDismissalCallbacks() { let chatsCoordinator = SomeTestCoordinator() let spacesCoordinator = SomeTestCoordinator() @@ -122,6 +146,31 @@ class NavigationTabCoordinatorTests: XCTestCase { waitForExpectations(timeout: 1.0) } + func testOverlayDismissalCallback() { + let overlayCoordinator = SomeTestCoordinator() + + let expectation = expectation(description: "Wait for callback") + navigationTabCoordinator.setOverlayCoordinator(overlayCoordinator) { + expectation.fulfill() + } + + navigationTabCoordinator.setOverlayCoordinator(nil) + waitForExpectations(timeout: 1.0) + } + + func testOverlayDismissalCallbackWhenChangingMode() { + let overlayCoordinator = SomeTestCoordinator() + + let expectation = expectation(description: "Wait for callback") + expectation.isInverted = true + navigationTabCoordinator.setOverlayCoordinator(overlayCoordinator) { + expectation.fulfill() + } + + navigationTabCoordinator.setOverlayPresentationMode(.minimized) + waitForExpectations(timeout: 1.0) + } + // MARK: - Private private func assertCoordinatorsEqual(_ lhs: CoordinatorProtocol?, _ rhs: CoordinatorProtocol?) {