From 7ea9aeb1743e7ec547206c8f423c0c403a58c0c7 Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Mon, 27 Jul 2026 19:14:23 +0200 Subject: [PATCH] Open a full-screen preview when tapping a gallery item Tapping a gallery tile/row opens the existing media preview scoped to that gallery's attachments, paging between them. Introduces a typed MediaPreviewItemID (replacing the stringly-typed preview identifier) and a gallery-scoped TimelineMediaPreviewDataSource; the tap routes through TimelineInteractionHandler/TimelineViewModel. Non-previewable (.other) items are skipped without shifting the tapped index. Co-authored-by: svetoslav.mitov just use id instead of previewID removed horrible multiline ternary pr suggestions remove redundant code remove empty caption display + gallery uses the appropriate media timeline actions --- .../TimelineMediaPreviewDataSource.swift | 212 ++++++++++++------ .../TimelineMediaPreviewModels.swift | 15 +- .../TimelineMediaPreviewViewModel.swift | 47 +++- .../View/TimelineMediaPreviewController.swift | 2 +- .../Timeline/TimelineInteractionHandler.swift | 14 ++ .../Screens/Timeline/TimelineModels.swift | 1 + .../Screens/Timeline/TimelineViewModel.swift | 27 +++ .../GalleryItemTileView.swift | 1 + .../GalleryRoomTimelineView.swift | 14 +- .../Services/Room/JoinedRoomProxy.swift | 1 + .../TimelineControllerProtocol.swift | 1 + ...ventBasedMessageTimelineItemProtocol.swift | 2 +- .../GalleryRoomTimelineItemContent.swift | 10 + .../Timeline/TimelineProxyProtocol.swift | 2 +- .../TimelineMediaPreviewDataSourceTests.swift | 106 +++++++-- 15 files changed, 343 insertions(+), 112 deletions(-) diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift index 892748e14..deab9f5b9 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift @@ -35,13 +35,16 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource { var paginationState: TimelinePaginationState + /// Builds a data source spanning every previewable attachment in the timeline, paginating + /// as the user swipes past the loaded range. Used when tapping a standalone media message. init(itemViewStates: [RoomTimelineItemViewState], initialItem: EventBasedMessageTimelineItemProtocol, initialPadding: Int = 100, paginationState: TimelinePaginationState) { previewItems = itemViewStates.compactMap(TimelineMediaPreviewItem.Media.init) - if let initialItemArrayIndex = previewItems.firstIndex(where: { $0.id == initialItem.id.eventOrTransactionID }) { + let initialPreviewID = TimelineMediaPreviewItem.Media(timelineItem: initialItem).id + if let initialItemArrayIndex = previewItems.firstIndex(where: { $0.id == initialPreviewID }) { initialItemIndex = initialItemArrayIndex + initialPadding currentItem = .media(previewItems[initialItemArrayIndex]) } else { @@ -58,6 +61,33 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource { self.paginationState = paginationState } + /// Builds a data source scoped to a single gallery's previewable attachments. + /// Used when the user taps a tile inside a gallery message — paging is local to that + /// gallery and there's no timeline pagination to drive. + init(galleryItem: GalleryRoomTimelineItem, + initialIndex: Int) { + let media = galleryItem.content.items.map { item in + TimelineMediaPreviewItem.Media(galleryParent: galleryItem, item: item) + } + + if media.indices.contains(initialIndex) { + // We set the entire gallery here up front. + previewItems = media + initialItemIndex = initialIndex + currentItem = .media(media[initialIndex]) + } else { + // Fall back to a synthetic placeholder for empty galleries — shouldn't happen in practice. + previewItems = [.init(timelineItem: galleryItem)] + initialItemIndex = 0 + currentItem = .media(previewItems[0]) + } + + // And we disable any use of the timeline by configuring the data source as though everything has paginated. + backwardPadding = 0 + forwardPadding = 0 + paginationState = .init(backward: .endReached, forward: .endReached) + } + func updateCurrentItem(_ item: TimelineMediaPreviewItem) { currentItem = item } @@ -68,7 +98,7 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource { // 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 }) { - oldItem.timelineItem = newItem.timelineItem + oldItem.content = newItem.content return oldItem } @@ -168,21 +198,48 @@ enum TimelineMediaPreviewItem: Equatable { /// Wraps a media file and title to be previewed with QuickLook. @Observable class Media: NSObject, QLPreviewItem, Identifiable { - fileprivate(set) var timelineItem: EventBasedMessageTimelineItemProtocol { + /// A standalone timeline message, or a single gallery attachment with its parent event. + fileprivate enum Content { + case timelineItem(EventBasedMessageTimelineItemProtocol) + case galleryItem(parent: EventBasedMessageTimelineItemProtocol, item: GalleryItem) + } + + fileprivate var content: Content { didSet { updatePreviewItemValues() } } + /// The parent event, so menu actions (redact, reply, …) target the gallery as a whole. + var timelineItem: EventBasedMessageTimelineItemProtocol { + switch content { + case .timelineItem(let timelineItem): timelineItem + case .galleryItem(let parent, _): parent + } + } + + var galleryItem: GalleryItem? { + switch content { + case .timelineItem: nil + case .galleryItem(_, let item): item + } + } + var fileHandle: MediaFileHandleProxy? { didSet { updatePreviewItemValues() } } 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 + init(timelineItem: EventBasedMessageTimelineItemProtocol) { - self.timelineItem = timelineItem + content = .timelineItem(timelineItem) + id = MediaPreviewItemID(timelineItem: timelineItem) } init?(roomTimelineItemViewState: RoomTimelineItemViewState) { + let timelineItem: EventBasedMessageTimelineItemProtocol switch roomTimelineItemViewState.type { case .audio(let audioRoomTimelineItem): timelineItem = audioRoomTimelineItem @@ -195,18 +252,15 @@ enum TimelineMediaPreviewItem: Equatable { default: return nil } + content = .timelineItem(timelineItem) + id = MediaPreviewItemID(timelineItem: timelineItem) } - // MARK: Identifiable - - /// 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 + /// Wraps a single attachment of a gallery message. `.other` items have no media source, + /// so QuickLook falls back to its default unsupported-item screen. + init(galleryParent: GalleryRoomTimelineItem, item: GalleryItem) { + content = .galleryItem(parent: galleryParent, item: item) + id = .galleryItem(item.id) } // MARK: QLPreviewItem @@ -241,46 +295,59 @@ enum TimelineMediaPreviewItem: Equatable { // MARK: Media details + /// The kind of media, used to decide how it should be saved. + enum Kind { case image, video, file } + + var kind: Kind { + if let galleryItem { + switch galleryItem { + case .image: return .image + case .video: return .video + case .audio, .file, .other: return .file + } + } + return switch timelineItem { + case is ImageRoomTimelineItem: .image + case is VideoRoomTimelineItem: .video + default: .file + } + } + var mediaSource: MediaSourceProxy? { - switch timelineItem { - case let audioItem as AudioRoomTimelineItem: - audioItem.content.source - case let fileItem as FileRoomTimelineItem: - fileItem.content.source - case let imageItem as ImageRoomTimelineItem: - imageItem.content.imageInfo.source - case let videoItem as VideoRoomTimelineItem: - videoItem.content.videoInfo.source - default: - nil + if let galleryItem { + return galleryItem.mediaSource + } + return switch timelineItem { + case let audioItem as AudioRoomTimelineItem: audioItem.content.source + case let fileItem as FileRoomTimelineItem: fileItem.content.source + case let imageItem as ImageRoomTimelineItem: imageItem.content.imageInfo.source + case let videoItem as VideoRoomTimelineItem: videoItem.content.videoInfo.source + default: nil } } var thumbnailMediaSource: MediaSourceProxy? { - switch timelineItem { - case let fileItem as FileRoomTimelineItem: - fileItem.content.thumbnailSource - case let imageItem as ImageRoomTimelineItem: - imageItem.content.thumbnailInfo?.source - case let videoItem as VideoRoomTimelineItem: - videoItem.content.thumbnailInfo?.source - default: - nil + if let galleryItem { + return galleryItem.thumbnailSource + } + return switch timelineItem { + case let fileItem as FileRoomTimelineItem: fileItem.content.thumbnailSource + case let imageItem as ImageRoomTimelineItem: imageItem.content.thumbnailInfo?.source + case let videoItem as VideoRoomTimelineItem: videoItem.content.thumbnailInfo?.source + default: nil } } var filename: String? { - switch timelineItem { - case let audioItem as AudioRoomTimelineItem: - audioItem.content.filename - case let fileItem as FileRoomTimelineItem: - fileItem.content.filename - case let imageItem as ImageRoomTimelineItem: - imageItem.content.filename - case let videoItem as VideoRoomTimelineItem: - videoItem.content.filename - default: - nil + if let galleryItem { + return galleryItem.filename + } + return switch timelineItem { + case let audioItem as AudioRoomTimelineItem: audioItem.content.filename + case let fileItem as FileRoomTimelineItem: fileItem.content.filename + case let imageItem as ImageRoomTimelineItem: imageItem.content.filename + case let videoItem as VideoRoomTimelineItem: videoItem.content.filename + default: nil } } @@ -289,21 +356,20 @@ enum TimelineMediaPreviewItem: Equatable { } private var expectedFileSize: UInt? { - switch timelineItem { - case let audioItem as AudioRoomTimelineItem: - audioItem.content.fileSize - case let fileItem as FileRoomTimelineItem: - fileItem.content.fileSize - case let imageItem as ImageRoomTimelineItem: - imageItem.content.imageInfo.fileSize - case let videoItem as VideoRoomTimelineItem: - videoItem.content.videoInfo.fileSize - default: - nil + if let galleryItem { + return galleryItem.fileSize + } + return switch timelineItem { + case let audioItem as AudioRoomTimelineItem: audioItem.content.fileSize + case let fileItem as FileRoomTimelineItem: fileItem.content.fileSize + case let imageItem as ImageRoomTimelineItem: imageItem.content.imageInfo.fileSize + case let videoItem as VideoRoomTimelineItem: videoItem.content.videoInfo.fileSize + default: nil } } var hasCaption: Bool { + // No need to special-case gallery items here, captions live on the gallery event itself timelineItem.hasMediaCaption } @@ -316,28 +382,26 @@ enum TimelineMediaPreviewItem: Equatable { } var contentType: String? { - switch timelineItem { - case let audioItem as AudioRoomTimelineItem: - audioItem.content.contentType?.localizedDescription - case let fileItem as FileRoomTimelineItem: - fileItem.content.contentType?.localizedDescription - case let imageItem as ImageRoomTimelineItem: - imageItem.content.contentType?.localizedDescription - case let videoItem as VideoRoomTimelineItem: - videoItem.content.contentType?.localizedDescription - default: - nil + if let galleryItem { + return galleryItem.contentType?.localizedDescription + } + return switch timelineItem { + case let audioItem as AudioRoomTimelineItem: audioItem.content.contentType?.localizedDescription + case let fileItem as FileRoomTimelineItem: fileItem.content.contentType?.localizedDescription + case let imageItem as ImageRoomTimelineItem: imageItem.content.contentType?.localizedDescription + case let videoItem as VideoRoomTimelineItem: videoItem.content.contentType?.localizedDescription + default: nil } } var blurhash: String? { - switch timelineItem { - case let imageItem as ImageRoomTimelineItem: - imageItem.content.blurhash - case let videoItem as VideoRoomTimelineItem: - videoItem.content.blurhash - default: - nil + if let galleryItem { + return galleryItem.blurhash + } + return switch timelineItem { + case let imageItem as ImageRoomTimelineItem: imageItem.content.blurhash + case let videoItem as VideoRoomTimelineItem: videoItem.content.blurhash + default: nil } } } diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift index fc9bc24fd..02852d146 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift @@ -15,8 +15,21 @@ enum TimelineMediaPreviewViewModelAction: Equatable { case dismiss } +/// Identifies a media preview item — either a whole timeline item or a single gallery attachment. +enum MediaPreviewItemID: Hashable { + case timelineItem(TimelineItemIdentifier.EventOrTransactionID) + case galleryItem(GalleryItemID) + + /// Identifies by event/transaction ID only, since the unique ID differs across timelines + /// and matching must survive rebuilding a filtered timeline to fetch the other media. + init(timelineItem: EventBasedMessageTimelineItemProtocol) { + guard let id = timelineItem.id.eventOrTransactionID else { fatalError("Virtual items cannot be previewed.") } + self = .timelineItem(id) + } +} + enum TimelineMediaPreviewDriverAction { - case itemLoaded(TimelineItemIdentifier.EventOrTransactionID) + case itemLoaded(MediaPreviewItemID) case showItemDetails(TimelineMediaPreviewItem.Media) case exportFile(TimelineMediaPreviewFileExportPicker.File) case authorizationRequired(appMediator: AppMediatorProtocol) diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift index 91558a760..15e585607 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift @@ -22,6 +22,10 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { private let userIndicatorController: UserIndicatorControllerProtocol private let appMediator: AppMediatorProtocol + /// Overrides the timeline kind used to build the context menu. Gallery previews use `.media` so the + /// standard download/forward/delete actions appear, since their underlying room timeline is `.live`. + private let menuTimelineKind: TimelineKind? + private var contentScannerService: ContentScannerServiceProtocol? { timelineViewModel.context.contentScannerService } @@ -31,6 +35,7 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { actionsSubject.eraseToAnyPublisher() } + /// Initialises a preview spanning the whole timeline's media, staying in sync with it as it paginates. init(initialItem: EventBasedMessageTimelineItemProtocol, timelineViewModel: TimelineViewModelProtocol, mediaProvider: MediaProviderProtocol, @@ -42,6 +47,7 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { self.photoLibraryManager = photoLibraryManager self.userIndicatorController = userIndicatorController self.appMediator = appMediator + menuTimelineKind = nil let timelineState = timelineViewModel.context.viewState.timelineState @@ -78,6 +84,35 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { .store(in: &cancellables) } + /// Initialises the preview scoped to a single gallery's attachments. The data source is + /// built from the gallery's items directly and isn't kept in sync with the underlying + /// timeline — gallery contents don't change without the event being replaced or redacted. + init(galleryItem: GalleryRoomTimelineItem, + initialIndex: Int, + timelineViewModel: TimelineViewModelProtocol, + mediaProvider: MediaProviderProtocol, + photoLibraryManager: PhotoLibraryManagerProtocol, + userIndicatorController: UserIndicatorControllerProtocol, + appMediator: AppMediatorProtocol) { + self.timelineViewModel = timelineViewModel + self.mediaProvider = mediaProvider + self.photoLibraryManager = photoLibraryManager + self.userIndicatorController = userIndicatorController + self.appMediator = appMediator + menuTimelineKind = .media(.roomScreenLive) + + super.init(initialViewState: TimelineMediaPreviewViewState(dataSource: .init(galleryItem: galleryItem, + initialIndex: initialIndex)), + mediaProvider: mediaProvider) + + rebuildCurrentItemActions() + + timelineViewModel.context.$viewState.map(\.canCurrentUserRedactSelf) + .merge(with: timelineViewModel.context.$viewState.map(\.canCurrentUserRedactOthers)) + .sink { [weak self] _ in self?.rebuildCurrentItemActions() } + .store(in: &cancellables) + } + override func process(viewAction: TimelineMediaPreviewViewAction) { switch viewAction { case .updateCurrentItem(let item): @@ -201,7 +236,7 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { pinnedEventIDs: timelineContext.viewState.pinnedEventIDs, isViewSourceEnabled: timelineContext.viewState.isViewSourceEnabled, areThreadsEnabled: timelineContext.viewState.areThreadsEnabled, - timelineKind: timelineContext.viewState.timelineKind, + timelineKind: menuTimelineKind ?? timelineContext.viewState.timelineKind, emojiProvider: timelineContext.viewState.emojiProvider) .makeActions() } @@ -217,16 +252,14 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { state.previewControllerDriver.send(.dismissDetailsSheet) do { - switch mediaItem.timelineItem { - case is AudioRoomTimelineItem, is FileRoomTimelineItem: + switch mediaItem.kind { + case .file: state.previewControllerDriver.send(.exportFile(.init(url: fileURL))) return // Don't show the indicator. - case is ImageRoomTimelineItem: + case .image: try await photoLibraryManager.addResource(.photo, at: fileURL).get() - case is VideoRoomTimelineItem: + case .video: try await photoLibraryManager.addResource(.video, at: fileURL).get() - default: - break } showSavedIndicator() diff --git a/ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewController.swift b/ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewController.swift index f0e788f13..364cf61b9 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewController.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewController.swift @@ -251,7 +251,7 @@ class TimelineMediaPreviewController: QLPreviewController { } } - private func handleFileLoaded(itemID: TimelineItemIdentifier.EventOrTransactionID) { + private func handleFileLoaded(itemID: MediaPreviewItemID) { guard (currentPreviewItem as? TimelineMediaPreviewItem.Media)?.id == itemID else { return } // There's a bug where refreshCurrentPreviewItem completely breaks the QLPreviewController diff --git a/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift b/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift index 8480c91c8..d240db9f5 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift @@ -239,6 +239,10 @@ class TimelineInteractionHandler { text = content.caption ?? "" htmlText = content.formattedCaptionHTMLString editType = text.isEmpty ? .addCaption : .editCaption + case .gallery(let content): + text = content.caption ?? "" + htmlText = content.formattedCaptionHTMLString + editType = text.isEmpty ? .addCaption : .editCaption default: text = messageTimelineItem.body } @@ -569,6 +573,16 @@ class TimelineInteractionHandler { } } + /// Opens a media preview scoped to a single gallery's attachments. The preview pages + /// only between the items of that one gallery — siblings in the wider timeline aren't + /// reachable from this entry point (use the regular media tap for that). + func processGalleryItemTap(itemID: TimelineItemIdentifier, index: Int) -> TimelineControllerAction { + guard let galleryItem = timelineController.timelineItems.firstUsingStableID(itemID) as? GalleryRoomTimelineItem else { + return .none + } + return .displayGalleryPreview(galleryItem: galleryItem, initialIndex: index) + } + // MARK: - Private private func buildReplyInfo(for item: EventBasedTimelineItemProtocol) -> ReplyInfo { diff --git a/ElementX/Sources/Screens/Timeline/TimelineModels.swift b/ElementX/Sources/Screens/Timeline/TimelineModels.swift index 70f953a8a..406fa78e2 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineModels.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineModels.swift @@ -53,6 +53,7 @@ enum TimelineViewAction { case itemDisappeared(itemID: TimelineItemIdentifier) case mediaTapped(itemID: TimelineItemIdentifier) + case galleryItemTapped(itemID: TimelineItemIdentifier, index: Int) case itemSendInfoTapped(itemID: TimelineItemIdentifier) case toggleReaction(key: String, itemID: TimelineItemIdentifier) case sendReadReceiptIfNeeded(TimelineItemIdentifier) diff --git a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift index b508e54f2..f69a4e6b4 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift @@ -159,6 +159,8 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { Task { await timelineController.processItemDisappearance(id) } case .mediaTapped(let id): Task { await handleMediaTapped(with: id) } + case .galleryItemTapped(let id, let index): + handleGalleryItemTapped(itemID: id, index: index) case .itemSendInfoTapped(let itemID): handleItemSendInfoTapped(itemID: itemID) case .toggleReaction(let emoji, let itemID): @@ -708,6 +710,8 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { let mediaPreviewViewModel = makeMediaPreviewViewModel(item: item, timelineViewModelKind: timelineViewModelKind) actionsSubject.send(.displayMediaPreview(mediaPreviewViewModel)) + case .displayGalleryPreview: + fatalError("Galleries open through the dedicated galleryItemTapped path, which carries the tapped index.") case .displayLocation(let location): actionsSubject.send(.displayLocation(location)) case .displayLiveLocation(let sender, let initialLiveLocationShare): @@ -718,6 +722,18 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { state.showLoading = false } + private func handleGalleryItemTapped(itemID: TimelineItemIdentifier, index: Int) { + if case let .displayGalleryPreview(galleryItem, initialIndex) = timelineInteractionHandler.processGalleryItemTap(itemID: itemID, index: index) { + displayGalleryPreview(galleryItem: galleryItem, initialIndex: initialIndex) + } + } + + private func displayGalleryPreview(galleryItem: GalleryRoomTimelineItem, initialIndex: Int) { + actionsSubject.send(.composer(action: .removeFocus)) + let mediaPreviewViewModel = makeGalleryPreviewViewModel(galleryItem: galleryItem, initialIndex: initialIndex) + actionsSubject.send(.displayMediaPreview(mediaPreviewViewModel)) + } + private func handleItemSendInfoTapped(itemID: TimelineItemIdentifier) { guard let timelineItem = timelineController.timelineItems.firstUsingStableID(itemID) else { MXLog.warning("Couldn't find timeline item.") @@ -833,6 +849,17 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { appMediator: appMediator) } + private func makeGalleryPreviewViewModel(galleryItem: GalleryRoomTimelineItem, + initialIndex: Int) -> TimelineMediaPreviewViewModel { + TimelineMediaPreviewViewModel(galleryItem: galleryItem, + initialIndex: initialIndex, + timelineViewModel: self, + mediaProvider: userSession.mediaProvider, + photoLibraryManager: PhotoLibraryManager(), + userIndicatorController: userIndicatorController, + appMediator: appMediator) + } + // MARK: - Timeline Item Building private func buildTimelineViews(timelineItems: [RoomTimelineItemProtocol], isSwitchingTimelines: Bool = false) { diff --git a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryItemTileView.swift b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryItemTileView.swift index 3b8c95d5f..cfdb05b8d 100644 --- a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryItemTileView.swift +++ b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryItemTileView.swift @@ -107,6 +107,7 @@ struct GalleryItemTileView: View { .font(.compound.headingSMSemibold) .foregroundStyle(.compound.textPrimary) } + .allowsHitTesting(false) // Let the tap fall through to the tile below. } private func formatted(duration: TimeInterval) -> String { diff --git a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryRoomTimelineView.swift b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryRoomTimelineView.swift index 28dd9a25b..b39dabe70 100644 --- a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryRoomTimelineView.swift +++ b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryRoomTimelineView.swift @@ -27,15 +27,13 @@ struct GalleryRoomTimelineView: View { if usesListLayout { GalleryListView(items: timelineItem.content.items, mediaProvider: context?.mediaProvider, - contentScannerService: context?.contentScannerService) { _ in - // Tapping to open the full-screen preview lands in a follow-up. - } + contentScannerService: context?.contentScannerService, + onItemTap: tap) } else { GalleryGridView(items: timelineItem.content.items, mediaProvider: context?.mediaProvider, - contentScannerService: context?.contentScannerService) { _ in - // Tapping to open the full-screen preview lands in a follow-up. - } + contentScannerService: context?.contentScannerService, + onItemTap: tap) } if hasMediaCaption { @@ -61,6 +59,10 @@ struct GalleryRoomTimelineView: View { boostFontSize: timelineItem.shouldBoost) } } + + private func tap(index: Int) { + context?.send(viewAction: .galleryItemTapped(itemID: timelineItem.id, index: index)) + } } // MARK: - Previews diff --git a/ElementX/Sources/Services/Room/JoinedRoomProxy.swift b/ElementX/Sources/Services/Room/JoinedRoomProxy.swift index 550fe7a85..114045b7a 100644 --- a/ElementX/Sources/Services/Room/JoinedRoomProxy.swift +++ b/ElementX/Sources/Services/Room/JoinedRoomProxy.swift @@ -227,6 +227,7 @@ class JoinedRoomProxy: JoinedRoomProxyProtocol { case .file: .file case .image: .image case .video: .video + case .gallery: .gallery } } diff --git a/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift b/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift index 419bd31d2..b5f7c129a 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift @@ -26,6 +26,7 @@ enum TimelineControllerAction { } case displayMediaPreview(item: EventBasedMessageTimelineItemProtocol, timelineViewModel: TimelineViewModelKind) + case displayGalleryPreview(galleryItem: GalleryRoomTimelineItem, initialIndex: Int) case displayLocation(StaticLocationData) case displayLiveLocation(sender: TimelineItemSender, initialLiveLocationShare: LiveLocationShare) case none diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/EventBasedMessageTimelineItemProtocol.swift b/ElementX/Sources/Services/Timeline/TimelineItems/EventBasedMessageTimelineItemProtocol.swift index a33aca26b..87217a5c0 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/EventBasedMessageTimelineItemProtocol.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/EventBasedMessageTimelineItemProtocol.swift @@ -36,7 +36,7 @@ nonisolated extension EventBasedMessageTimelineItemProtocol { } var hasMediaCaption: Bool { - mediaCaption != nil + mediaCaption?.isBlank == false } var mediaCaption: String? { diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift index b52e27bfb..65df2f500 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift @@ -111,6 +111,16 @@ nonisolated enum GalleryItem: Hashable, Identifiable { } } + var fileSize: UInt? { + switch self { + case .image(_, let content): content.imageInfo.fileSize + case .video(_, let content): content.videoInfo.fileSize + case .audio(_, let content): content.fileSize + case .file(_, let content): content.fileSize + case .other: nil + } + } + var blurhash: String? { switch self { case .image(_, let content): content.blurhash diff --git a/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift b/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift index d321c0d67..c0d5f5e1f 100644 --- a/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift +++ b/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift @@ -41,7 +41,7 @@ enum TimelineFocus { } enum TimelineAllowedMessageType { - case audio, file, image, video + case audio, file, image, video, gallery } enum TimelineProxyError: Error { diff --git a/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift b/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift index 449f6932a..5c062e0c7 100644 --- a/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift +++ b/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift @@ -29,6 +29,62 @@ struct TimelineMediaPreviewDataSourceTests { try assertInitialDataSource() } + @Test + func galleryPreview() throws { + // Given a gallery message with three previewable attachments. + let items = (0..<3).map { index in + GalleryItem.mockImage(index: index, filename: "image-\(index).jpg") + } + let gallery = GalleryRoomTimelineItem(id: .randomEvent, + timestamp: .mock, + isOutgoing: false, + isEditable: false, + canBeRepliedTo: true, + sender: .init(id: "Bob"), + content: .init(body: "Gallery", caption: nil, items: items), + properties: .init()) + + // When opening the preview scoped to that gallery on the second attachment. + let dataSource = TimelineMediaPreviewDataSource(galleryItem: gallery, initialIndex: 1) + + // Then it contains exactly the gallery's attachments with no surrounding pagination… + #expect(dataSource.previewItems.count == 3) + #expect(dataSource.numberOfPreviewItems(in: previewController) == 3) + #expect(dataSource.initialItemIndex == 1) + + // …and the initial item is the tapped attachment. + let media = try #require(dataSource.currentItem.mediaItem, "The current item should be a media item.") + #expect(media.id == .galleryItem(items[1].id)) + } + + @Test + func galleryPreviewIncludesNonPreviewableItems() throws { + // Given a gallery whose first attachment is an unknown type (QuickLook shows its default screen). + let items: [GalleryItem] = [ + .other(id: .mock(0), filename: "unknown.bin"), + .mockImage(index: 1, filename: "image-1.jpg"), + .mockImage(index: 2, filename: "image-2.jpg") + ] + let gallery = GalleryRoomTimelineItem(id: .randomEvent, + timestamp: .mock, + isOutgoing: false, + isEditable: false, + canBeRepliedTo: true, + sender: .init(id: "Bob"), + content: .init(body: "Gallery", caption: nil, items: items), + properties: .init()) + + // When tapping the first image (index 1 in the full items array). + let dataSource = TimelineMediaPreviewDataSource(galleryItem: gallery, initialIndex: 1) + + // Then every attachment is kept — indices line up 1:1 — and the unknown item has no media source. + #expect(dataSource.previewItems.count == 3) + #expect(dataSource.previewItems[0].id == .galleryItem(items[0].id)) + #expect(dataSource.previewItems[0].mediaSource == nil) + let media = try #require(dataSource.currentItem.mediaItem, "The current item should be a media item.") + #expect(media.id == .galleryItem(items[1].id)) + } + @Test func currentUpdateItem() throws { // Given a data source built with the initial items. @@ -69,8 +125,8 @@ struct TimelineMediaPreviewDataSourceTests { let previewItemCount = dataSource.numberOfPreviewItems(in: previewController) let displayedItem = try #require(dataSource.previewController(previewController, previewItemAt: dataSource.initialItemIndex) as? TimelineMediaPreviewItem.Media) - #expect(displayedItem.id == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The displayed item should not change.") - #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The current item should not change.") + #expect(displayedItem.id == initialMediaItems[initialItemIndex].previewID, "The displayed item should not change.") + #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].previewID, "The current item should not change.") #expect(dataSource.previewItems.count == initialMediaViewStates.count, "The number of items should not change.") #expect(previewItemCount == initialMediaViewStates.count + (2 * initialPadding), "The padded number of items should not change.") @@ -93,8 +149,8 @@ struct TimelineMediaPreviewDataSourceTests { var previewItemCount = dataSource.numberOfPreviewItems(in: previewController) var displayedItem = try #require(dataSource.previewController(previewController, previewItemAt: dataSource.initialItemIndex) as? TimelineMediaPreviewItem.Media) - #expect(displayedItem.id == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The displayed item should not change.") - #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The current item should not change.") + #expect(displayedItem.id == initialMediaItems[initialItemIndex].previewID, "The displayed item should not change.") + #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].previewID, "The current item should not change.") #expect(previewItemCount == initialMediaViewStates.count + (2 * initialPadding), "The number of items should not change") // When more items are loaded in a forward pagination or sync. @@ -109,8 +165,8 @@ struct TimelineMediaPreviewDataSourceTests { previewItemCount = dataSource.numberOfPreviewItems(in: previewController) displayedItem = try #require(dataSource.previewController(previewController, previewItemAt: dataSource.initialItemIndex) as? TimelineMediaPreviewItem.Media) - #expect(displayedItem.id == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The displayed item should not change.") - #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The current item should not change.") + #expect(displayedItem.id == initialMediaItems[initialItemIndex].previewID, "The displayed item should not change.") + #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].previewID, "The current item should not change.") #expect(previewItemCount == initialMediaViewStates.count + (2 * initialPadding), "The number of items should not change") } @@ -133,8 +189,8 @@ struct TimelineMediaPreviewDataSourceTests { var previewItemCount = dataSource.numberOfPreviewItems(in: previewController) var displayedItem = try #require(dataSource.previewController(previewController, previewItemAt: dataSource.initialItemIndex) as? TimelineMediaPreviewItem.Media) - #expect(displayedItem.id == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The displayed item should not change.") - #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The current item should not change.") + #expect(displayedItem.id == initialMediaItems[initialItemIndex].previewID, "The displayed item should not change.") + #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].previewID, "The current item should not change.") #expect(previewItemCount == initialMediaViewStates.count + (2 * initialPadding), "The number of items should not change") // When paginating forwards by more than the available padding. @@ -149,8 +205,8 @@ struct TimelineMediaPreviewDataSourceTests { previewItemCount = dataSource.numberOfPreviewItems(in: previewController) displayedItem = try #require(dataSource.previewController(previewController, previewItemAt: dataSource.initialItemIndex) as? TimelineMediaPreviewItem.Media) - #expect(displayedItem.id == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The displayed item should not change.") - #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The current item should not change.") + #expect(displayedItem.id == initialMediaItems[initialItemIndex].previewID, "The displayed item should not change.") + #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].previewID, "The current item should not change.") #expect(previewItemCount == initialMediaViewStates.count + (2 * initialPadding), "The number of items should not change") } @@ -173,8 +229,8 @@ struct TimelineMediaPreviewDataSourceTests { #expect(previewItemCount == 1 + (2 * initialPadding), "The initial item count should be padded for the preview controller.") #expect(dataSource.initialItemIndex == initialPadding, "The initial item index should be padded for the preview controller.") - #expect(displayedItem.id == initialItem.id.eventOrTransactionID, "The displayed item should be the initial item.") - #expect(dataSource.currentMediaItemID == initialItem.id.eventOrTransactionID, "The current item should also be the initial item.") + #expect(displayedItem.id == initialItem.previewID, "The displayed item should be the initial item.") + #expect(dataSource.currentMediaItemID == initialItem.previewID, "The current item should also be the initial item.") // When the timeline loads the initial items. let deferred = deferFulfillment(dataSource.previewItemsPaginationPublisher) { _ in true } @@ -191,8 +247,8 @@ struct TimelineMediaPreviewDataSourceTests { #expect(previewItemCount == 1 + (2 * initialPadding), "The item count should not change as the padding will be reduced.") #expect(dataSource.initialItemIndex == initialPadding, "The item index should not change.") - #expect(displayedItem.id == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The displayed item should not change.") - #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The current item should not change.") + #expect(displayedItem.id == initialMediaItems[initialItemIndex].previewID, "The displayed item should not change.") + #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].previewID, "The current item should not change.") } @Test @@ -214,8 +270,8 @@ struct TimelineMediaPreviewDataSourceTests { #expect(previewItemCount == 1 + (2 * initialPadding), "The initial item count should be padded for the preview controller.") #expect(dataSource.initialItemIndex == initialPadding, "The initial item index should be padded for the preview controller.") - #expect(displayedItem.id == initialItem.id.eventOrTransactionID, "The displayed item should be the initial item.") - #expect(dataSource.currentMediaItemID == initialItem.id.eventOrTransactionID, "The current item should also be the initial item.") + #expect(displayedItem.id == initialItem.previewID, "The displayed item should be the initial item.") + #expect(dataSource.currentMediaItemID == initialItem.previewID, "The current item should also be the initial item.") // When the timeline loads more items but still doesn't include the initial item. let failure = deferFailure(dataSource.previewItemsPaginationPublisher, timeout: .seconds(1)) { _ in true } @@ -232,8 +288,8 @@ struct TimelineMediaPreviewDataSourceTests { #expect(previewItemCount == 1 + (2 * initialPadding), "The initial item count should not change.") #expect(dataSource.initialItemIndex == initialPadding, "The initial item index should not change.") - #expect(displayedItem.id == initialItem.id.eventOrTransactionID, "The displayed item should not change.") - #expect(dataSource.currentMediaItemID == initialItem.id.eventOrTransactionID, "The current item not change.") + #expect(displayedItem.id == initialItem.previewID, "The displayed item should not change.") + #expect(dataSource.currentMediaItemID == initialItem.previewID, "The current item not change.") } // MARK: Helpers @@ -259,8 +315,8 @@ struct TimelineMediaPreviewDataSourceTests { // Then the preview controller should be showing the initial item and the data source should reflect this. #expect(dataSource.initialItemIndex == initialItemIndex + initialPadding, "The initial item index should be padded for the preview controller.") - #expect(displayedItem.id == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The displayed item should be the initial item.") - #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].id.eventOrTransactionID, "The current item should also be the initial item.") + #expect(displayedItem.id == initialMediaItems[initialItemIndex].previewID, "The displayed item should be the initial item.") + #expect(dataSource.currentMediaItemID == initialMediaItems[initialItemIndex].previewID, "The current item should also be the initial item.") #expect(dataSource.previewItems.count == initialMediaViewStates.count, "The initial count of preview items should be correct.") #expect(previewItemCount == initialMediaViewStates.count + (2 * initialPadding), "The initial item count should be padded for the preview controller.") @@ -270,7 +326,15 @@ struct TimelineMediaPreviewDataSourceTests { } private extension TimelineMediaPreviewDataSource { - var currentMediaItemID: TimelineItemIdentifier.EventOrTransactionID? { + var currentMediaItemID: MediaPreviewItemID? { currentItem.mediaItem?.id } } + +private extension EventBasedMessageTimelineItemProtocol { + /// Test helper that derives the same preview ID used by `TimelineMediaPreviewItem.Media` — + /// avoids reaching into the private encoding from the tests. + var previewID: MediaPreviewItemID { + TimelineMediaPreviewItem.Media(timelineItem: self).id + } +}