Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove GenericCallLinkCoordinator, merging it into CallScreen. #3181

Merged
merged 4 commits into from
Aug 19, 2024
Merged

Remove GenericCallLinkCoordinator, merging it into CallScreen. #3181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f59af45
Remove incorrect message send call and fix typo.
pixlwave Aug 16, 2024
c5fcca6
Add overlay coordinator presentation to the NavigationRootCoordinator.
pixlwave Aug 19, 2024
ec11df8
Remove GenericCallLinkCoordinator, merging it into CallScreen.
pixlwave Aug 19, 2024
56d7982
PR comments and view tweaks.
pixlwave Aug 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions ElementX.xcodeproj/project.pbxproj
View file
Open in desktop

Large diffs are not rendered by default.

27 changes: 26 additions & 1 deletion ElementX/Sources/Application/AppCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg
if let userSessionFlowCoordinator {
userSessionFlowCoordinator.handleAppRoute(route, animated: true)
} else {
navigationRootCoordinator.setSheetCoordinator(GenericCallLinkCoordinator(parameters: .init(url: url)))
presentCallScreen(genericCallLink: url)
}
case .userProfile(let userID):
if isExternalURL {
Expand Down Expand Up @@ -648,6 +648,31 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg

elementCallService.setClientProxy(userSession.clientProxy)
}

private func presentCallScreen(genericCallLink url: URL) {
let configuration = ElementCallConfiguration(genericCallLink: url)

let callScreenCoordinator = CallScreenCoordinator(parameters: .init(elementCallService: elementCallService,
configuration: configuration,
elementCallPictureInPictureEnabled: false,
appHooks: appHooks))

callScreenCoordinator.actions
.sink { [weak self] action in
guard let self else { return }
switch action {
case .pictureInPictureStarted, .pictureInPictureStopped:
// Don't allow PiP when signed out - the user could login at which point we'd
// need to hand over the call from here to the user session flow coordinator.
MXLog.error("Picture in Picture not supported before login.")
case .dismiss:
navigationRootCoordinator.setOverlayCoordinator(nil)
}
}
.store(in: &cancellables)

navigationRootCoordinator.setOverlayCoordinator(callScreenCoordinator, animated: false)
}

private func configureNotificationManager() {
notificationManager.setUserSession(userSession)
Expand Down
1 change: 1 addition & 0 deletions ElementX/Sources/Application/Navigation/NavigationCoordinators.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -457,6 +457,7 @@ private struct NavigationSplitCoordinatorView: View {
}
}
.animation(.elementDefault, value: navigationSplitCoordinator.overlayPresentationMode)
.animation(.elementDefault, value: navigationSplitCoordinator.overlayModule)
}
// Handle `horizontalSizeClass` changes breaking the navigation bar
// https://github.com/element-hq/element-x-ios/issues/617
Expand Down
53 changes: 53 additions & 0 deletions ElementX/Sources/Application/Navigation/NavigationRootCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,25 @@ class NavigationRootCoordinator: ObservableObject, CoordinatorProtocol, CustomSt
sheetModule?.coordinator
}

@Published 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
}

/// Sets or replaces the presented coordinator
/// - Parameter coordinator: the coordinator to display
func setRootCoordinator(_ coordinator: (any CoordinatorProtocol)?, animated: Bool = true, dismissalCallback: (() -> Void)? = nil) {
Expand Down Expand Up @@ -90,6 +109,31 @@ class NavigationRootCoordinator: ObservableObject, CoordinatorProtocol, CustomSt
sheetModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback)
}
}

/// Present an overlay on top of the split view
/// - Parameters:
/// - coordinator: the coordinator to display
/// - animated: whether the transition should be animated
/// - dismissalCallback: called when the overlay has been dismissed, programatically or otherwise
func setOverlayCoordinator(_ coordinator: (any CoordinatorProtocol)?,
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) {
overlayModule = NavigationModule(coordinator, dismissalCallback: dismissalCallback)
}
}

// MARK: - CoordinatorProtocol

Expand Down Expand Up @@ -127,5 +171,14 @@ private struct NavigationRootCoordinatorView: View {
.sheet(item: $rootCoordinator.sheetModule) { module in
module.coordinator?.toPresentable()
}
.overlay {
Group {
if let coordinator = rootCoordinator.overlayModule?.coordinator {
coordinator.toPresentable()
.transition(.opacity)
}
}
.animation(.elementDefault, value: rootCoordinator.overlayModule)
}
Comment on lines +174 to +182
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about the navigation triggered by notifications? I imagine this won't dismiss the overlay but just navigate behind it right?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm, so it doesn't matter in this instance and it is only used when signed out (and we're not supporting PiP there even if there was somewhere to navigate to). But you're right, I need to think about this in the UserSessionFlowCoordinator 🤔

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will follow this up in another PR.

}
}
48 changes: 27 additions & 21 deletions ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -268,11 +268,9 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
case .userProfile(let userID):
stateMachine.processEvent(.showUserProfileScreen(userID: userID), userInfo: .init(animated: animated))
case .call(let roomID):
Task {
await presentCallScreen(roomID: roomID)
}
Task { await presentCallScreen(roomID: roomID) }
case .genericCallLink(let url):
navigationSplitCoordinator.setSheetCoordinator(GenericCallLinkCoordinator(parameters: .init(url: url)), animated: animated)
presentCallScreen(genericCallLink: url)
case .settings, .chatBackupSettings:
settingsFlowCoordinator.handleAppRoute(appRoute, animated: animated)
case .oidcCallback:
Expand Down Expand Up @@ -558,23 +556,39 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {

// MARK: Calls

private var callScreenPictureInPictureController: AVPictureInPictureController?
private func presentCallScreen(genericCallLink url: URL) {
presentCallScreen(configuration: .init(genericCallLink: url))
}

private func presentCallScreen(roomID: String) async {
guard let roomProxy = await userSession.clientProxy.roomForIdentifier(roomID) else {
return
}

presentCallScreen(roomProxy: roomProxy)
}

private func presentCallScreen(roomProxy: RoomProxyProtocol) {
guard elementCallService.ongoingCallRoomID != roomProxy.id else {
let colorScheme: ColorScheme = appMediator.windowManager.mainWindow.traitCollection.userInterfaceStyle == .light ? .light : .dark
presentCallScreen(configuration: .init(roomProxy: roomProxy,
clientProxy: userSession.clientProxy,
clientID: InfoPlistReader.main.bundleIdentifier,
elementCallBaseURL: appSettings.elementCallBaseURL,
elementCallBaseURLOverride: appSettings.elementCallBaseURLOverride,
colorScheme: colorScheme))
}

private var callScreenPictureInPictureController: AVPictureInPictureController?
private func presentCallScreen(configuration: ElementCallConfiguration) {
guard elementCallService.ongoingCallRoomID != configuration.callID else {
MXLog.info("Returning to existing call.")
callScreenPictureInPictureController?.stopPictureInPicture()
return
}

let colorScheme: ColorScheme = appMediator.windowManager.mainWindow.traitCollection.userInterfaceStyle == .light ? .light : .dark
let callScreenCoordinator = CallScreenCoordinator(parameters: .init(elementCallService: elementCallService,
clientProxy: userSession.clientProxy,
roomProxy: roomProxy,
clientID: InfoPlistReader.main.bundleIdentifier,
elementCallBaseURL: appSettings.elementCallBaseURL,
elementCallBaseURLOverride: appSettings.elementCallBaseURLOverride,
configuration: configuration,
elementCallPictureInPictureEnabled: appSettings.elementCallPictureInPictureEnabled,
colorScheme: colorScheme,
appHooks: appHooks))

callScreenCoordinator.actions
Expand All @@ -600,14 +614,6 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
analytics.track(screen: .RoomCall)
}

private func presentCallScreen(roomID: String) async {
guard let roomProxy = await userSession.clientProxy.roomForIdentifier(roomID) else {
return
}

presentCallScreen(roomProxy: roomProxy)
}

private func dismissCallScreenIfNeeded() {
guard navigationSplitCoordinator.sheetCoordinator is CallScreenCoordinator else {
return
Expand Down
92 changes: 46 additions & 46 deletions ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5142,74 +5142,74 @@ class ElementCallWidgetDriverMock: ElementCallWidgetDriverProtocol {
return startBaseURLClientIDColorSchemeReturnValue
}
}
//MARK: - sendMessage
//MARK: - handleMessage

var sendMessageUnderlyingCallsCount = 0
var sendMessageCallsCount: Int {
var handleMessageUnderlyingCallsCount = 0
var handleMessageCallsCount: Int {
get {
if Thread.isMainThread {
return sendMessageUnderlyingCallsCount
return handleMessageUnderlyingCallsCount
} else {
var returnValue: Int? = nil
DispatchQueue.main.sync {
returnValue = sendMessageUnderlyingCallsCount
returnValue = handleMessageUnderlyingCallsCount
}

return returnValue!
}
}
set {
if Thread.isMainThread {
sendMessageUnderlyingCallsCount = newValue
handleMessageUnderlyingCallsCount = newValue
} else {
DispatchQueue.main.sync {
sendMessageUnderlyingCallsCount = newValue
handleMessageUnderlyingCallsCount = newValue
}
}
}
}
var sendMessageCalled: Bool {
return sendMessageCallsCount > 0
var handleMessageCalled: Bool {
return handleMessageCallsCount > 0
}
var sendMessageReceivedMessage: String?
var sendMessageReceivedInvocations: [String] = []
var handleMessageReceivedMessage: String?
var handleMessageReceivedInvocations: [String] = []

var sendMessageUnderlyingReturnValue: Result<Bool, ElementCallWidgetDriverError>!
var sendMessageReturnValue: Result<Bool, ElementCallWidgetDriverError>! {
var handleMessageUnderlyingReturnValue: Result<Bool, ElementCallWidgetDriverError>!
var handleMessageReturnValue: Result<Bool, ElementCallWidgetDriverError>! {
get {
if Thread.isMainThread {
return sendMessageUnderlyingReturnValue
return handleMessageUnderlyingReturnValue
} else {
var returnValue: Result<Bool, ElementCallWidgetDriverError>? = nil
DispatchQueue.main.sync {
returnValue = sendMessageUnderlyingReturnValue
returnValue = handleMessageUnderlyingReturnValue
}

return returnValue!
}
}
set {
if Thread.isMainThread {
sendMessageUnderlyingReturnValue = newValue
handleMessageUnderlyingReturnValue = newValue
} else {
DispatchQueue.main.sync {
sendMessageUnderlyingReturnValue = newValue
handleMessageUnderlyingReturnValue = newValue
}
}
}
}
var sendMessageClosure: ((String) async -> Result<Bool, ElementCallWidgetDriverError>)?
var handleMessageClosure: ((String) async -> Result<Bool, ElementCallWidgetDriverError>)?

func sendMessage(_ message: String) async -> Result<Bool, ElementCallWidgetDriverError> {
sendMessageCallsCount += 1
sendMessageReceivedMessage = message
func handleMessage(_ message: String) async -> Result<Bool, ElementCallWidgetDriverError> {
handleMessageCallsCount += 1
handleMessageReceivedMessage = message
DispatchQueue.main.async {
self.sendMessageReceivedInvocations.append(message)
self.handleMessageReceivedInvocations.append(message)
}
if let sendMessageClosure = sendMessageClosure {
return await sendMessageClosure(message)
if let handleMessageClosure = handleMessageClosure {
return await handleMessageClosure(message)
} else {
return sendMessageReturnValue
return handleMessageReturnValue
}
}
}
Expand Down Expand Up @@ -11129,68 +11129,68 @@ class RoomProxyMock: RoomProxyProtocol {
return elementCallWidgetDriverDeviceIDReturnValue
}
}
//MARK: - sendCallNotificationIfNeeeded
//MARK: - sendCallNotificationIfNeeded

var sendCallNotificationIfNeeededUnderlyingCallsCount = 0
var sendCallNotificationIfNeeededCallsCount: Int {
var sendCallNotificationIfNeededUnderlyingCallsCount = 0
var sendCallNotificationIfNeededCallsCount: Int {
get {
if Thread.isMainThread {
return sendCallNotificationIfNeeededUnderlyingCallsCount
return sendCallNotificationIfNeededUnderlyingCallsCount
} else {
var returnValue: Int? = nil
DispatchQueue.main.sync {
returnValue = sendCallNotificationIfNeeededUnderlyingCallsCount
returnValue = sendCallNotificationIfNeededUnderlyingCallsCount
}

return returnValue!
}
}
set {
if Thread.isMainThread {
sendCallNotificationIfNeeededUnderlyingCallsCount = newValue
sendCallNotificationIfNeededUnderlyingCallsCount = newValue
} else {
DispatchQueue.main.sync {
sendCallNotificationIfNeeededUnderlyingCallsCount = newValue
sendCallNotificationIfNeededUnderlyingCallsCount = newValue
}
}
}
}
var sendCallNotificationIfNeeededCalled: Bool {
return sendCallNotificationIfNeeededCallsCount > 0
var sendCallNotificationIfNeededCalled: Bool {
return sendCallNotificationIfNeededCallsCount > 0
}

var sendCallNotificationIfNeeededUnderlyingReturnValue: Result<Void, RoomProxyError>!
var sendCallNotificationIfNeeededReturnValue: Result<Void, RoomProxyError>! {
var sendCallNotificationIfNeededUnderlyingReturnValue: Result<Void, RoomProxyError>!
var sendCallNotificationIfNeededReturnValue: Result<Void, RoomProxyError>! {
get {
if Thread.isMainThread {
return sendCallNotificationIfNeeededUnderlyingReturnValue
return sendCallNotificationIfNeededUnderlyingReturnValue
} else {
var returnValue: Result<Void, RoomProxyError>? = nil
DispatchQueue.main.sync {
returnValue = sendCallNotificationIfNeeededUnderlyingReturnValue
returnValue = sendCallNotificationIfNeededUnderlyingReturnValue
}

return returnValue!
}
}
set {
if Thread.isMainThread {
sendCallNotificationIfNeeededUnderlyingReturnValue = newValue
sendCallNotificationIfNeededUnderlyingReturnValue = newValue
} else {
DispatchQueue.main.sync {
sendCallNotificationIfNeeededUnderlyingReturnValue = newValue
sendCallNotificationIfNeededUnderlyingReturnValue = newValue
}
}
}
}
var sendCallNotificationIfNeeededClosure: (() async -> Result<Void, RoomProxyError>)?
var sendCallNotificationIfNeededClosure: (() async -> Result<Void, RoomProxyError>)?

func sendCallNotificationIfNeeeded() async -> Result<Void, RoomProxyError> {
sendCallNotificationIfNeeededCallsCount += 1
if let sendCallNotificationIfNeeededClosure = sendCallNotificationIfNeeededClosure {
return await sendCallNotificationIfNeeededClosure()
func sendCallNotificationIfNeeded() async -> Result<Void, RoomProxyError> {
sendCallNotificationIfNeededCallsCount += 1
if let sendCallNotificationIfNeededClosure = sendCallNotificationIfNeededClosure {
return await sendCallNotificationIfNeededClosure()
} else {
return sendCallNotificationIfNeeededReturnValue
return sendCallNotificationIfNeededReturnValue
}
}
//MARK: - matrixToPermalink
Expand Down
2 changes: 1 addition & 1 deletion ElementX/Sources/Mocks/RoomProxyMock.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ extension RoomProxyMock {
widgetDriver.startBaseURLClientIDColorSchemeReturnValue = .success(url)

elementCallWidgetDriverDeviceIDReturnValue = widgetDriver
sendCallNotificationIfNeeededReturnValue = .success(())
sendCallNotificationIfNeededReturnValue = .success(())

matrixToPermalinkReturnValue = .success(.homeDirectory)
matrixToEventPermalinkReturnValue = .success(.homeDirectory)
Expand Down
Loading
Loading