Minor quicklook fixes. (#6112)

* Only show a spinner when opening the media viewer is slow * Keep a sent local echo's page in the media viewer * Fix the caption visibility. --------- Co-authored-by: Stefan Ceriu <stefan.ceriu@gmail.com> Co-authored-by: Matthew Hodgson <matthew@matrix.org>
2026-10-31 16:24:11 +00:00 · 2026-09-08 09:40:43 +01:00
parent 1af16e92db
commit 15627e4512
4 changed files with 112 additions and 10 deletions
@@ -112,8 +112,9 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource {
            .flatMap { $0.previewableMedia(allowedGalleryItemTypes: allowedGalleryItemTypes) }
            .map { newItem in
                // If an item already exists use that instead to preserve the file handle, download error etc.
-                if let oldItem = previewItems.first(where: { $0.id == newItem.id }) {
+                if let oldItem = previewItems.first(where: { $0.id == newItem.id || $0.isLocalEcho(of: newItem) }) {
                    oldItem.content = newItem.content
+                    oldItem.id = newItem.id // Make sure a local echo's transaction ID → event ID.
                    return oldItem
                }
                
@@ -237,9 +238,22 @@ enum TimelineMediaPreviewItem: Equatable {
        
        var downloadError: Error?
        
-        /// A stable identifier that's unique per preview item — including individual gallery
-        /// attachments that would otherwise share their parent event's ID.
-        let id: MediaPreviewItemID
+        /// A stable identifier that's unique per preview item, even for gallery attachments that would
+        /// otherwise share their parent event's ID. Not stable for local echoes however, those will
+        /// transition from transaction ID to event ID once sent.
+        fileprivate(set) var id: MediaPreviewItemID
+        
+        /// Whether this is the local echo that `sentItem` is the remote echo of. Sending swaps the
+        /// transaction ID for an event ID but the unique ID remains stable.
+        fileprivate func isLocalEcho(of sentItem: Media) -> Bool {
+            guard timelineItem.id.transactionID != nil, sentItem.timelineItem.id.eventID != nil,
+                  timelineItem.id.uniqueID == sentItem.timelineItem.id.uniqueID else { return false }
+            switch (content, sentItem.content) {
+            case (.timelineItem, .timelineItem): return true
+            case (.galleryItem(_, let item), .galleryItem(_, let sentGalleryItem)): return item.id.mediaIndex == sentGalleryItem.id.mediaIndex
+            default: return false
+            }
+        }
        
        init(timelineItem: EventBasedMessageTimelineItemProtocol) {
            content = .timelineItem(timelineItem)
@@ -73,6 +73,11 @@ class TimelineMediaPreviewController: QLPreviewController {
        
        super.init(nibName: nil, bundle: nil)
        
+        headerHostingController.rootView.onMoveToWindow = { [weak self] in
+            guard #available(iOS 26, *) else { return }
+            self?.updateCaptionVisibility()
+        }
+        
        view.addSubview(captionView)
        // Constraints added later as the toolbar isn't available yet.
        
@@ -128,11 +133,9 @@ class TimelineMediaPreviewController: QLPreviewController {
        super.viewWillLayoutSubviews()
        
        if let bottomBarItemsContainer {
-            // Using the toolbar's visibility doesn't work so check its frame.
-            captionView.isHidden = if #available(iOS 26, *) {
-                navigationBar?.topItem?.leftBarButtonItem?.frame(in: view) == nil
-            } else {
-                bottomBarItemsContainer.frame.minY >= view.frame.maxY
+            if #unavailable(iOS 26) {
+                // Using the toolbar's visibility doesn't work so check its frame.
+                captionView.isHidden = bottomBarItemsContainer.frame.minY >= view.frame.maxY
            }
            
            if captionView.constraints.isEmpty {
@@ -176,6 +179,15 @@ class TimelineMediaPreviewController: QLPreviewController {
        barButtonTimer = nil
    }
    
+    @available(iOS 26, *)
+    private func updateCaptionVisibility() {
+        // The caption should be hidden alongside the header.
+        let isHeaderHidden = headerHostingController.view.window == nil
+        if captionView.isHidden != isHeaderHidden {
+            captionView.isHidden = isHeaderHidden
+        }
+    }
+    
    private func updateBarButtons() {
        guard let topItem = navigationBar?.topItem else { return }
        
@@ -306,11 +318,20 @@ class TimelineMediaPreviewController: QLPreviewController {

 private struct HeaderView: View {
    @ObservedObject var context: TimelineMediaPreviewViewModel.Context
+    /// Called whenever the header is added or removed from a window.
+    var onMoveToWindow: () -> Void = { }
+    
    private var currentItem: TimelineMediaPreviewItem {
        context.viewState.currentItem
    }
    
    var body: some View {
+        content
+            .background(WindowTrackingView(onMoveToWindow: onMoveToWindow))
+    }
+    
+    @ViewBuilder
+    private var content: some View {
        if let mediaItem = currentItem.mediaItem {
            VStack(spacing: 0) {
                Text(mediaItem.sender.displayName ?? mediaItem.sender.id)
@@ -475,6 +496,31 @@ private struct DownloadIndicatorView: View {

 // MARK: - Helpers

+/// An invisible view that reports when it is added to or removed from a window.
+private struct WindowTrackingView: UIViewRepresentable {
+    let onMoveToWindow: () -> Void
+    
+    func makeUIView(context: Context) -> TrackingView {
+        let view = TrackingView()
+        view.isUserInteractionEnabled = false
+        view.onMoveToWindow = onMoveToWindow
+        return view
+    }
+    
+    func updateUIView(_ uiView: TrackingView, context: Context) {
+        uiView.onMoveToWindow = onMoveToWindow
+    }
+    
+    class TrackingView: UIView {
+        var onMoveToWindow: (() -> Void)?
+        
+        override func didMoveToWindow() {
+            super.didMoveToWindow()
+            onMoveToWindow?()
+        }
+    }
+}
+
 private extension UIView {
    func firstScrollView() -> UIScrollView? {
        for view in subviews {
@@ -708,8 +708,15 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
    }
    
    private func handleMediaTapped(with itemID: TimelineItemIdentifier, galleryIndex: Int? = nil) async {
-        state.showLoading = true
+        // Building the media timeline takes ~100ms from the event cache, however its possible that
+        // a focussed timeline may hit /context so we need to show a spinner when it's actually slow.
+        let spinner = Task {
+            try? await Task.sleep(for: .milliseconds(300))
+            guard !Task.isCancelled else { return }
+            state.showLoading = true
+        }
        let action = await timelineInteractionHandler.processItemTap(itemID)
+        spinner.cancel()
        
        switch action {
        case .displayMediaPreview(let item, let timelineViewModelKind):
@@ -343,6 +343,41 @@ struct TimelineMediaPreviewDataSourceTests {
            .filter { !($0 is GalleryRoomTimelineItem) } // Galleries are previewed through their own data source.
    }
    
+    @Test
+    func sentLocalEchoKeepsItsPage() async throws {
+        // Given a viewer opened on an upload in flight: a local echo with a transaction ID.
+        let uniqueID = TimelineItemIdentifier.UniqueID(UUID().uuidString)
+        func makeUpload(id: TimelineItemIdentifier.EventOrTransactionID) -> ImageRoomTimelineItem {
+            ImageRoomTimelineItem(id: .event(uniqueID: uniqueID, eventOrTransactionID: id),
+                                  timestamp: .mock,
+                                  isOutgoing: true,
+                                  isEditable: false,
+                                  canBeRepliedTo: false,
+                                  sender: .init(id: "Alice"),
+                                  content: .init(filename: "upload.jpg", imageInfo: .mockImage, thumbnailInfo: nil, blurhash: nil))
+        }
+        let localEcho = makeUpload(id: .transactionID("t1"))
+        var items = newChunk()
+        items.append(localEcho)
+        let dataSource = TimelineMediaPreviewDataSource(itemViewStates: items.map { RoomTimelineItemViewState(item: $0, groupStyle: .single) },
+                                                        initialItem: localEcho,
+                                                        paginationState: .initial)
+        let page = try #require(dataSource.currentItem.mediaItem)
+        #expect(page.id == .timelineItem(.transactionID("t1")))
+        
+        // When the upload completes and the timeline swaps the echo's transaction ID for its event ID.
+        items[items.count - 1] = makeUpload(id: .eventID("$e1"))
+        let deferred = deferFailure(dataSource.previewItemsPaginationPublisher, timeout: .seconds(1)) { _ in true }
+        dataSource.updatePreviewItems(itemViewStates: items.map { RoomTimelineItemViewState(item: $0, groupStyle: .single) })
+        try await deferred.fulfill()
+        
+        // Then the same page carries on under the new ID (its file came from the local media cache), nothing rebuilt.
+        #expect(dataSource.currentItem.mediaItem === page)
+        #expect(page.id == .timelineItem(.eventID("$e1")))
+        #expect(dataSource.previewItems.map(\.id).last == .timelineItem(.eventID("$e1")))
+        #expect(dataSource.previewItems.count == items.count)
+    }
+    
    @discardableResult
    private func assertInitialDataSource() throws -> TimelineMediaPreviewDataSource {
        // Given a data source built with the initial items.