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.
2026-11-02 21:33:26 +00:00 · 2025-02-05 13:27:23 +00:00
parent 921d1c627d
commit cfaa1b455a
45 changed files with 490 additions and 202 deletions
@@ -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
@@ -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)
@@ -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)
@@ -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
        }
    }
    
@@ -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]
        
@@ -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