From 0a58fbfeacdc62b53f84de3f85ea85f70562b466 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Thu, 10 Aug 2023 09:55:10 +0300 Subject: [PATCH] Fixes #1140 - Dismiss all modals when presenting a room --- .../RoomFlowCoordinator.swift | 4 +++ .../InteractiveQuickLook.swift | 28 +++++++++++++++---- .../RoomDetailsScreenCoordinator.swift | 4 +++ .../RoomDetailsScreenViewModel.swift | 5 ++++ .../RoomDetailsScreenViewModelProtocol.swift | 1 + .../RoomMemberDetailsScreenCoordinator.swift | 2 ++ .../RoomMemberDetailsScreenViewModel.swift | 5 ++++ ...MemberDetailsScreenViewModelProtocol.swift | 1 + .../RoomScreen/RoomScreenCoordinator.swift | 4 +++ .../RoomScreen/RoomScreenViewModel.swift | 5 ++++ .../RoomScreenViewModelProtocol.swift | 1 + changelog.d/1140.bugfix | 1 + 12 files changed, 56 insertions(+), 5 deletions(-) create mode 100644 changelog.d/1140.bugfix diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 849727ff4..6d16c10a5 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -259,6 +259,10 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { } private func asyncPresentRoom(_ roomID: String, animated: Bool, destinationRoomProxy: RoomProxyProtocol? = nil) async { + // If any sheets are presented dismiss them, rely on their dismissal callbacks to transition the state machine + // through the correct states before presenting the room + navigationStackCoordinator.setSheetCoordinator(nil) + if let roomProxy, roomProxy.id == roomID { navigationStackCoordinator.popToRoot() return diff --git a/ElementX/Sources/Screens/FilePreviewScreen/InteractiveQuickLook.swift b/ElementX/Sources/Screens/FilePreviewScreen/InteractiveQuickLook.swift index 860a4654f..e1418a11a 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/InteractiveQuickLook.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/InteractiveQuickLook.swift @@ -14,6 +14,7 @@ // limitations under the License. // +import Combine import QuickLook import SwiftUI @@ -28,10 +29,15 @@ extension View { private struct InteractiveQuickLookModifier: ViewModifier { @Binding var item: MediaPreviewItem? + @State private var dismissalPublisher = PassthroughSubject() + func body(content: Content) -> some View { content.background { if let item { - MediaPreviewViewController(previewItem: item) { self.item = nil } + MediaPreviewViewController(previewItem: item, dismissalPublisher: dismissalPublisher) { self.item = nil } + } else { + // Work around QLPreviewController dismissal issues, see below. + let _ = dismissalPublisher.send(()) } } } @@ -39,10 +45,11 @@ private struct InteractiveQuickLookModifier: ViewModifier { private struct MediaPreviewViewController: UIViewControllerRepresentable { let previewItem: MediaPreviewItem + let dismissalPublisher: PassthroughSubject let onDismiss: () -> Void func makeUIViewController(context: Context) -> PreviewHostingController { - PreviewHostingController(previewItem: previewItem, onDismiss: onDismiss) + PreviewHostingController(previewItem: previewItem, dismissalPublisher: dismissalPublisher, onDismiss: onDismiss) } func updateUIViewController(_ uiViewController: PreviewHostingController, context: Context) { } @@ -53,15 +60,26 @@ private struct MediaPreviewViewController: UIViewControllerRepresentable { /// animations and interactions which don't work if you represent it directly to SwiftUI 🤷‍♂️ class PreviewHostingController: UIViewController, QLPreviewControllerDataSource, QLPreviewControllerDelegate { let previewItem: MediaPreviewItem + let dismissalPublisher: PassthroughSubject let onDismiss: () -> Void + private var dismissalObserver: AnyCancellable? + var previewController: QLPreviewController? - init(previewItem: MediaPreviewItem, onDismiss: @escaping () -> Void) { + init(previewItem: MediaPreviewItem, dismissalPublisher: PassthroughSubject, onDismiss: @escaping () -> Void) { self.previewItem = previewItem + self.dismissalPublisher = dismissalPublisher self.onDismiss = onDismiss - + super.init(nibName: nil, bundle: nil) + + // The QLPreviewController will not automatically dismiss itself when the underlying view is removed + // (e.g. switching rooms from a notification) and it continues to hold on to the whole hierarcy. + // Manually tell it to dismiss itself here. + dismissalObserver = dismissalPublisher.sink { _ in + self.dismiss(animated: true) + } } @available(*, unavailable) @@ -125,6 +143,6 @@ struct PreviewView_Previews: PreviewProvider { title: "Important Document") static var previews: some View { - MediaPreviewViewController(previewItem: previewItem) { } + MediaPreviewViewController(previewItem: previewItem, dismissalPublisher: .init()) { } } } diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenCoordinator.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenCoordinator.swift index 62c3c7619..bd1928e75 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenCoordinator.swift +++ b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenCoordinator.swift @@ -79,6 +79,10 @@ final class RoomDetailsScreenCoordinator: CoordinatorProtocol { } } + func stop() { + viewModel.stop() + } + func toPresentable() -> AnyView { AnyView(RoomDetailsScreen(context: viewModel.context)) } diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift index c768b0606..3337361ee 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift @@ -69,6 +69,11 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr // MARK: - Public + func stop() { + // Work around QLPreviewController dismissal issues, see the InteractiveQuickLookModifier. + state.bindings.mediaPreviewItem = nil + } + override func process(viewAction: RoomDetailsScreenViewAction) { switch viewAction { case .processTapPeople: diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModelProtocol.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModelProtocol.swift index 8d4d4e2bc..3030758e2 100644 --- a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModelProtocol.swift +++ b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModelProtocol.swift @@ -20,4 +20,5 @@ import Foundation protocol RoomDetailsScreenViewModelProtocol { var callback: ((RoomDetailsScreenViewModelAction) -> Void)? { get set } var context: RoomDetailsScreenViewModelType.Context { get } + func stop() } diff --git a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift index 5cca2843e..1e8625f09 100644 --- a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift +++ b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenCoordinator.swift @@ -41,6 +41,8 @@ final class RoomMemberDetailsScreenCoordinator: CoordinatorProtocol { } func start() { } + + func stop() { viewModel.stop() } func toPresentable() -> AnyView { AnyView(RoomMemberDetailsScreen(context: viewModel.context)) diff --git a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift index d0ffbb309..877966d04 100644 --- a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModel.swift @@ -43,6 +43,11 @@ class RoomMemberDetailsScreenViewModel: RoomMemberDetailsScreenViewModelType, Ro // MARK: - Public + func stop() { + // Work around QLPreviewController dismissal issues, see the InteractiveQuickLookModifier. + state.bindings.mediaPreviewItem = nil + } + override func process(viewAction: RoomMemberDetailsScreenViewAction) { switch viewAction { case .showUnignoreAlert: diff --git a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModelProtocol.swift b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModelProtocol.swift index a06876f0b..96f42d983 100644 --- a/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModelProtocol.swift +++ b/ElementX/Sources/Screens/RoomMemberDetailsScreen/RoomMemberDetailsScreenViewModelProtocol.swift @@ -20,4 +20,5 @@ import Foundation protocol RoomMemberDetailsScreenViewModelProtocol { var callback: ((RoomMemberDetailsScreenViewModelAction) -> Void)? { get set } var context: RoomMemberDetailsScreenViewModelType.Context { get } + func stop() } diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift index 683aa5286..56ab8ae34 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenCoordinator.swift @@ -106,6 +106,10 @@ final class RoomScreenCoordinator: CoordinatorProtocol { .store(in: &cancellables) } + func stop() { + viewModel.stop() + } + func toPresentable() -> AnyView { AnyView(RoomScreen(context: viewModel.context, composerToolbar: ComposerToolbar(context: composerViewModel.context))) } diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift index 44c581b23..c15dcab7f 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift @@ -89,6 +89,11 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol actionsSubject.eraseToAnyPublisher() } + func stop() { + // Work around QLPreviewController dismissal issues, see the InteractiveQuickLookModifier. + state.bindings.mediaPreviewItem = nil + } + override func process(viewAction: RoomScreenViewAction) { switch viewAction { case .displayRoomDetails: diff --git a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift index 14fd53271..96d1ffd9f 100644 --- a/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift +++ b/ElementX/Sources/Screens/RoomScreen/RoomScreenViewModelProtocol.swift @@ -23,4 +23,5 @@ protocol RoomScreenViewModelProtocol { var actions: AnyPublisher { get } var context: RoomScreenViewModelType.Context { get } func process(composerAction: ComposerToolbarViewModelAction) + func stop() } diff --git a/changelog.d/1140.bugfix b/changelog.d/1140.bugfix new file mode 100644 index 000000000..23fecb121 --- /dev/null +++ b/changelog.d/1140.bugfix @@ -0,0 +1 @@ +Prevent inconsistent view hierarchies when opening rooms from push notifications \ No newline at end of file