Use the new preview screen when tapping media on the room and pinned …

…events screens. (#3736)

* Use the new TimelineMediaPreview modifier on the room and pinned timeline screens.

* Use the same presentation logic for all timeline media previews.

* Fix a bug with the detection of the timeline end.

* Send pagination requests from the media preview screen.

* Add SwiftLint to the Danger workflow (it is no longer installed on the runner).

* Put SwiftLint back on all of the GitHub runners too.

* Set the function_parameter_count lint rule to 10.

* Make sure to clean-up any previews when the coordinator is done.

* Handle the viewInRoomTimeline action more appropriately.

.github/workflows/danger.yml

@@ -15,7 +15,7 @@ jobs:
 
       - name: Setup environment
         run: 
-          brew install danger/tap/danger-swift
+          brew install danger/tap/danger-swift swiftlint
 
       - name: Danger
         run: 

.swiftlint.yml

@@ -41,6 +41,10 @@ function_body_length:
   warning: 100
   error: 100
 
+function_parameter_count:
+  warning: 10
+  error: 10
+
 cyclomatic_complexity:
   ignores_case_statements: true
 

ElementX/Sources/FlowCoordinators/MediaEventsTimelineFlowCoordinator.swift

@@ -64,7 +64,8 @@ class MediaEventsTimelineFlowCoordinator: FlowCoordinatorProtocol {
                                                           attributedStringBuilder: AttributedStringBuilder(mentionBuilder: MentionBuilder()),
                                                           stateEventStringBuilder: RoomStateEventStringBuilder(userID: userSession.clientProxy.userID))
 
-        guard case let .success(mediaTimelineController) = await timelineControllerFactory.buildMessageFilteredTimelineController(allowedMessageTypes: [.image, .video],
+        guard case let .success(mediaTimelineController) = await timelineControllerFactory.buildMessageFilteredTimelineController(focus: .live,
+                                                                                                                                  allowedMessageTypes: [.image, .video],
                                                                                                                                   presentation: .mediaFilesScreen,
                                                                                                                                   roomProxy: roomProxy,
                                                                                                                                   timelineItemFactory: timelineItemFactory,
@@ -73,7 +74,8 @@ class MediaEventsTimelineFlowCoordinator: FlowCoordinatorProtocol {
             return
         }
 
-        guard case let .success(filesTimelineController) = await timelineControllerFactory.buildMessageFilteredTimelineController(allowedMessageTypes: [.file, .audio],
+        guard case let .success(filesTimelineController) = await timelineControllerFactory.buildMessageFilteredTimelineController(focus: .live,
+                                                                                                                                  allowedMessageTypes: [.file, .audio],
                                                                                                                                   presentation: .mediaFilesScreen,
                                                                                                                                   roomProxy: roomProxy,
                                                                                                                                   timelineItemFactory: timelineItemFactory,

ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift

@@ -41,9 +41,16 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource {
         previewItems = itemViewStates.compactMap(TimelineMediaPreviewItem.Media.init)
         self.initialItem = initialItem
 
-        let initialItemArrayIndex = previewItems.firstIndex { $0.id == initialItem.id } ?? 0
-        initialItemIndex = initialItemArrayIndex + initialPadding
-        currentItem = .media(previewItems[initialItemArrayIndex])
+        if let initialItemArrayIndex = previewItems.firstIndex(where: { $0.id == initialItem.id.eventOrTransactionID }) {
+            initialItemIndex = initialItemArrayIndex + initialPadding
+            currentItem = .media(previewItems[initialItemArrayIndex])
+        } else {
+            // The timeline hasn't loaded the initial item yet, so replace the whatever was loaded with
+            // the item the user wants to preview.
+            initialItemIndex = initialPadding
+            previewItems = [.init(timelineItem: initialItem)]
+            currentItem = .media(previewItems[0])
+        }
 
         backwardPadding = initialPadding
         forwardPadding = initialPadding
@@ -83,10 +90,18 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource {
                 hasPaginated = true
             }
         } else {
-            // Do nothing! Not ideal but if we reload the data source the current item will
-            // also be, reloaded resetting any interaction the user has made with it. If we
-            // ignore the pagination, then the next time they swipe they'll land on a different
+            // When the timeline is loading items from the store and the initial item is the only
+            // preview in the array, we don't want to wipe it out, so if the existing items aren't
+            // found within the new items then let's ignore the update for now. This comes with a
+            // tradeoff that when a media gets redacted, no more previews will be added to the viewer.
+            //
+            // Note for the future if anyone wants to fix the redaction issue: Reloading the data source,
+            // will also reload the current item resetting any interaction the user has made with it.
+            // If you ignore the pagination, then the next time they swipe they'll land on a different
             // media but this is probably less jarring overall. I hate QLPreviewController!
+
+            MXLog.info("Ignoring update: unable to find existing preview items range.")
+            return
         }
 
         previewItems = newItems
@@ -109,9 +124,9 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource {
         let arrayIndex = index - backwardPadding
 
         if index < firstPreviewItemIndex {
-            return paginationState.backward == .timelineEndReached ? TimelineMediaPreviewItem.Loading.timelineStart : .paginating
+            return paginationState.backward == .timelineEndReached ? TimelineMediaPreviewItem.Loading.timelineStart : .paginatingBackwards
         } else if index > lastPreviewItemIndex {
-            return paginationState.forward == .timelineEndReached ? TimelineMediaPreviewItem.Loading.timelineEnd : .paginating
+            return paginationState.forward == .timelineEndReached ? TimelineMediaPreviewItem.Loading.timelineEnd : .paginatingForwards
         } else {
             return previewItems[arrayIndex]
         }
@@ -151,7 +166,15 @@ enum TimelineMediaPreviewItem: Equatable {
 
         // MARK: Identifiable
 
-        var id: TimelineItemIdentifier { timelineItem.id }
+        /// The timeline item's event or transaction ID.
+        ///
+        /// We're identifying items by this to ensure that all matching is made using only this part of the identifier. This is
+        /// because the unique ID will be different across timelines so when the initial item comes from a regular timeline and
+        /// we build a filtered timeline to fetch the other media items, it is impossible to match by the `TimelineItemIdentifier`.
+        var id: TimelineItemIdentifier.EventOrTransactionID {
+            guard let id = timelineItem.id.eventOrTransactionID else { fatalError("Virtual items cannot be previewed.") }
+            return id
+        }
 
         // MARK: QLPreviewItem
 
@@ -274,11 +297,12 @@ enum TimelineMediaPreviewItem: Equatable {
     }
 
     class Loading: NSObject, QLPreviewItem {
-        static let paginating = Loading(state: .paginating)
+        static let paginatingBackwards = Loading(state: .paginating(.backwards))
+        static let paginatingForwards = Loading(state: .paginating(.forwards))
         static let timelineStart = Loading(state: .timelineStart)
         static let timelineEnd = Loading(state: .timelineEnd)
 
-        enum State { case paginating, timelineStart, timelineEnd }
+        enum State { case paginating(PaginationDirection), timelineStart, timelineEnd }
         let state: State
 
         let previewItemURL: URL? = nil

ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift

@@ -14,7 +14,7 @@ enum TimelineMediaPreviewViewModelAction: Equatable {
 }
 
 enum TimelineMediaPreviewDriverAction {
-    case itemLoaded(TimelineItemIdentifier)
+    case itemLoaded(TimelineItemIdentifier.EventOrTransactionID)
     case showItemDetails(TimelineMediaPreviewItem.Media)
     case exportFile(TimelineMediaPreviewFileExportPicker.File)
     case authorizationRequired(appMediator: AppMediatorProtocol)

ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift

@@ -63,7 +63,11 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
 
         timelineViewModel.context.$viewState.map(\.timelineState.paginationState)
             .removeDuplicates()
-            .weakAssign(to: \.state.dataSource.paginationState, on: self)
+            .sink { [weak self] paginationState in
+                guard let self else { return }
+                state.dataSource.paginationState = paginationState
+                paginateIfNeeded()
+            }
             .store(in: &cancellables)
     }
 
@@ -77,7 +81,7 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
             switch action {
             case .viewInRoomTimeline:
                 state.previewControllerDriver.send(.dismissDetailsSheet)
-                actionsSubject.send(.viewInRoomTimeline(item.id))
+                actionsSubject.send(.viewInRoomTimeline(item.timelineItem.id))
             case .save:
                 Task { await saveCurrentItem() }
             case .redact:
@@ -111,6 +115,23 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
                     mediaItem.downloadError = error
                 }
             }
+        } else {
+            paginateIfNeeded()
+        }
+    }
+
+    private func paginateIfNeeded() {
+        switch state.currentItem {
+        case .loading(.paginatingBackwards):
+            if state.dataSource.paginationState.backward == .idle {
+                timelineViewModel.context.send(viewAction: .paginateBackwards)
+            }
+        case .loading(.paginatingForwards):
+            if state.dataSource.paginationState.forward == .idle {
+                timelineViewModel.context.send(viewAction: .paginateForwards)
+            }
+        default:
+            break
         }
     }
 
@@ -166,7 +187,7 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
     }
 
     private func redactItem(_ item: TimelineMediaPreviewItem.Media) {
-        timelineViewModel.context.send(viewAction: .handleTimelineItemMenuAction(itemID: item.id, action: .redact))
+        timelineViewModel.context.send(viewAction: .handleTimelineItemMenuAction(itemID: item.timelineItem.id, action: .redact))
         state.bindings.redactConfirmationItem = nil
         state.previewControllerDriver.send(.dismissDetailsSheet)
         actionsSubject.send(.dismiss)

ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewController.swift

@@ -182,7 +182,7 @@ class TimelineMediaPreviewController: QLPreviewController {
         }
     }
 
-    private func handleFileLoaded(itemID: TimelineItemIdentifier) {
+    private func handleFileLoaded(itemID: TimelineItemIdentifier.EventOrTransactionID) {
         guard (currentPreviewItem as? TimelineMediaPreviewItem.Media)?.id == itemID else { return }
         refreshCurrentPreviewItem()
     }
@@ -301,7 +301,8 @@ private struct DownloadIndicatorView: View {
     private var shouldShowDownloadIndicator: Bool {
         switch currentItem {
         case .media(let mediaItem): mediaItem.fileHandle == nil
-        case .loading(let loadingItem): loadingItem.state == .paginating
+        case .loading(.paginatingBackwards), .loading(.paginatingForwards): true
+        case .loading: false
         }
     }
 

ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewDetailsView.swift

@@ -222,7 +222,7 @@ struct TimelineMediaPreviewDetailsView_Previews: PreviewProvider, TestablePrevie
                                                         thumbnailInfo: .mockThumbnail,
                                                         contentType: contentType))
 
-        let timelineKind = TimelineKind.media(isPresentedOnRoomScreen ? .roomScreen : .mediaFilesScreen)
+        let timelineKind = TimelineKind.media(isPresentedOnRoomScreen ? .roomScreenLive : .mediaFilesScreen)
         let timelineController = MockTimelineController(timelineKind: timelineKind)
         timelineController.timelineItems = [item]
 

ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewRedactConfirmationView.swift

@@ -63,7 +63,7 @@ struct TimelineMediaPreviewRedactConfirmationView: View {
                     .scaledFrame(size: 40)
                     .background {
                         LoadableImage(mediaSource: mediaSource,
-                                      mediaType: .timelineItem(uniqueID: item.id.uniqueID),
+                                      mediaType: .generic,
                                       blurhash: item.blurhash,
                                       mediaProvider: context.mediaProvider) {
                             Color.compound.bgSubtleSecondary

ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenCoordinator.swift

@@ -79,6 +79,10 @@ final class MediaEventsTimelineScreenCoordinator: CoordinatorProtocol {
             .store(in: &cancellables)
     }
 
+    func stop() {
+        viewModel.stop()
+    }
+
     func toPresentable() -> AnyView {
         AnyView(MediaEventsTimelineScreen(context: viewModel.context))
     }

ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenViewModel.swift

@@ -64,6 +64,19 @@ class MediaEventsTimelineScreenViewModel: MediaEventsTimelineScreenViewModelType
         }
         .store(in: &cancellables)
 
+        mediaTimelineViewModel.actions.sink { [weak self] action in
+            switch action {
+            case .displayMediaPreview(let mediaPreviewViewModel):
+                self?.displayMediaPreview(mediaPreviewViewModel)
+            case .displayEmojiPicker, .displayReportContent, .displayCameraPicker, .displayMediaPicker,
+                 .displayDocumentPicker, .displayLocationPicker, .displayPollForm, .displayMediaUploadPreviewScreen,
+                 .tappedOnSenderDetails, .displayMessageForwarding, .displayLocation, .displayResolveSendFailure,
+                 .composer, .hasScrolled, .viewInRoomTimeline:
+                break
+            }
+        }
+        .store(in: &cancellables)
+
         filesTimelineViewModel.context.$viewState.sink { [weak self] timelineViewState in
             guard let self, state.bindings.screenMode == .files else {
                 return
@@ -73,6 +86,19 @@ class MediaEventsTimelineScreenViewModel: MediaEventsTimelineScreenViewModelType
         }
         .store(in: &cancellables)
 
+        filesTimelineViewModel.actions.sink { [weak self] action in
+            switch action {
+            case .displayMediaPreview(let mediaPreviewViewModel):
+                self?.displayMediaPreview(mediaPreviewViewModel)
+            case .displayEmojiPicker, .displayReportContent, .displayCameraPicker, .displayMediaPicker,
+                 .displayDocumentPicker, .displayLocationPicker, .displayPollForm, .displayMediaUploadPreviewScreen,
+                 .tappedOnSenderDetails, .displayMessageForwarding, .displayLocation, .displayResolveSendFailure,
+                 .composer, .hasScrolled, .viewInRoomTimeline:
+                break
+            }
+        }
+        .store(in: &cancellables)
+
         updateWithTimelineViewState(activeTimelineViewModel.context.viewState)
     }
 
@@ -90,10 +116,15 @@ class MediaEventsTimelineScreenViewModel: MediaEventsTimelineScreenViewModelType
         case .oldestItemDidDisappear:
             isOldestItemVisible = false
         case .tappedItem(let item):
-            handleItemTapped(item)
+            activeTimelineViewModel.context.send(viewAction: .mediaTapped(itemID: item.identifier))
         }
     }
 
+    func stop() {
+        // Work around QLPreviewController dismissal issues, see the InteractiveQuickLookModifier.
+        state.bindings.mediaPreviewViewModel = nil
+    }
+
     // MARK: - Private
 
     private func updateWithTimelineViewState(_ timelineViewState: TimelineViewState) {
@@ -146,26 +177,7 @@ class MediaEventsTimelineScreenViewModel: MediaEventsTimelineScreenViewModelType
         }
     }
 
-    private func handleItemTapped(_ item: RoomTimelineItemViewState) {
-        let item: EventBasedMessageTimelineItemProtocol? = switch item.type {
-        case .audio(let audioItem): audioItem
-        case .file(let fileItem): fileItem
-        case .image(let imageItem): imageItem
-        case .video(let videoItem): videoItem
-        default: nil
-        }
-
-        guard let item else {
-            MXLog.error("Unexpected item type tapped.")
-            return
-        }
-
-        let viewModel = TimelineMediaPreviewViewModel(initialItem: item,
-                                                      timelineViewModel: activeTimelineViewModel,
-                                                      mediaProvider: mediaProvider,
-                                                      photoLibraryManager: PhotoLibraryManager(),
-                                                      userIndicatorController: userIndicatorController,
-                                                      appMediator: appMediator)
+    private func displayMediaPreview(_ viewModel: TimelineMediaPreviewViewModel) {
         viewModel.actions.sink { [weak self] action in
             guard let self else { return }
             switch action {

ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenViewModelProtocol.swift

@@ -11,4 +11,6 @@ import Combine
 protocol MediaEventsTimelineScreenViewModelProtocol {
     var actionsPublisher: AnyPublisher<MediaEventsTimelineScreenViewModelAction, Never> { get }
     var context: MediaEventsTimelineScreenViewModelType.Context { get }
+
+    func stop()
 }

ElementX/Sources/Screens/PinnedEventsTimelineScreen/PinnedEventsTimelineScreenCoordinator.swift

@@ -62,6 +62,9 @@ final class PinnedEventsTimelineScreenCoordinator: CoordinatorProtocol {
 
             guard let self else { return }
             switch action {
+            case .viewInRoomTimeline(let itemID):
+                guard let eventID = itemID.eventID else { fatalError("A pinned event must have an event ID.") }
+                actionsSubject.send(.displayRoomScreenWithFocussedPin(eventID: eventID))
             case .dismiss:
                 self.actionsSubject.send(.dismiss)
             }
@@ -77,6 +80,8 @@ final class PinnedEventsTimelineScreenCoordinator: CoordinatorProtocol {
                 actionsSubject.send(.displayUser(userID: userID))
             case .displayMessageForwarding(let forwardingItem):
                 actionsSubject.send(.displayMessageForwarding(forwardingItem: forwardingItem))
+            case .displayMediaPreview(let mediaPreviewViewModel):
+                viewModel.displayMediaPreview(mediaPreviewViewModel)
             case .displayLocation(_, let geoURI, let description):
                 actionsSubject.send(.presentLocationViewer(geoURI: geoURI, description: description))
             case .viewInRoomTimeline(let eventID):
@@ -91,6 +96,10 @@ final class PinnedEventsTimelineScreenCoordinator: CoordinatorProtocol {
         }
         .store(in: &cancellables)
     }
+
+    func stop() {
+        viewModel.stop()
+    }
 
     func toPresentable() -> AnyView {
         AnyView(PinnedEventsTimelineScreen(context: viewModel.context, timelineContext: timelineViewModel.context))

ElementX/Sources/Screens/PinnedEventsTimelineScreen/PinnedEventsTimelineScreenModels.swift

@@ -8,10 +8,18 @@
 import Foundation
 
 enum PinnedEventsTimelineScreenViewModelAction {
+    case viewInRoomTimeline(itemID: TimelineItemIdentifier)
     case dismiss
 }
 
-struct PinnedEventsTimelineScreenViewState: BindableState { }
+struct PinnedEventsTimelineScreenViewState: BindableState {
+    var bindings = PinnedEventsTimelineScreenViewStateBindings()
+}
+
+struct PinnedEventsTimelineScreenViewStateBindings {
+    /// The view model used to present a QuickLook media preview.
+    var mediaPreviewViewModel: TimelineMediaPreviewViewModel?
+}
 
 enum PinnedEventsTimelineScreenViewAction {
     case close