From ae795df53045c318c81c91eadc4464e7cdb25965 Mon Sep 17 00:00:00 2001 From: Mauro <34335419+Velin92@users.noreply.github.com> Date: Thu, 30 Jul 2026 13:49:22 +0200 Subject: [PATCH] Allow previews in the timeline to include filtered galleries (#5943) * remove the usage of the timeline kind override in favour of getting it properly from the timeline view model * allow the previews to include gallery items filtered by the allowed types when started outside the gallery sss asa * fix for actions not being forwarded to the timeline right * pr suggestions --- .../Mocks/Generated/GeneratedMocks.swift | 1 + .../Mocks/TimelineControllerMock.swift | 2 + .../TimelineMediaPreviewDataSource.swift | 52 ++++++++++---- .../TimelineMediaPreviewViewModel.swift | 11 +-- .../Timeline/TimelineInteractionHandler.swift | 52 +++++++++----- .../Screens/Timeline/TimelineModels.swift | 2 + .../Screens/Timeline/TimelineViewModel.swift | 65 ++++++++--------- .../GalleryItemTileView.swift | 2 +- .../TimelineItemViews/GalleryListView.swift | 2 +- .../TimelineController.swift | 6 +- .../TimelineControllerFactory.swift | 3 +- .../TimelineControllerProtocol.swift | 5 +- .../GalleryRoomTimelineItemContent.swift | 18 ++++- .../RoomTimelineItemFactory.swift | 8 +-- .../Services/Timeline/TimelineProxy.swift | 22 +++--- .../Timeline/TimelineProxyProtocol.swift | 30 +++++++- .../TimelineMediaPreviewDataSourceTests.swift | 72 +++++++++++++++---- 17 files changed, 243 insertions(+), 110 deletions(-) diff --git a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift index b48775c25..cc6cb5c89 100644 --- a/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift +++ b/ElementX/Sources/Mocks/Generated/GeneratedMocks.swift @@ -11491,6 +11491,7 @@ nonisolated class TimelineControllerMock: TimelineControllerProtocol, @unchecked set(value) { underlyingTimelineKind = value } } nonisolated(unsafe) var underlyingTimelineKind: TimelineKind! + nonisolated(unsafe) var allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? nonisolated(unsafe) var timelineItems: [RoomTimelineItemProtocol] = [] var paginationState: TimelinePaginationState { get { return underlyingPaginationState } diff --git a/ElementX/Sources/Mocks/TimelineControllerMock.swift b/ElementX/Sources/Mocks/TimelineControllerMock.swift index 5cc90a60c..d4b9bf188 100644 --- a/ElementX/Sources/Mocks/TimelineControllerMock.swift +++ b/ElementX/Sources/Mocks/TimelineControllerMock.swift @@ -18,6 +18,7 @@ struct TimelineControllerMockConfiguration { var timelineProxy: TimelineProxyProtocol? var timelineItemsTimestamps: [TimelineItemIdentifier: Date] = [:] var paginationState: TimelinePaginationState = .initial + var allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? } @MainActor extension TimelineControllerMock { @@ -44,6 +45,7 @@ struct TimelineControllerMockConfiguration { callbacks = PassthroughSubject() timelineKind = configuration.timelineKind + allowedGalleryItemTypes = configuration.allowedGalleryItemTypes paginationState = configuration.paginationState timelineItems = configuration.timelineItems diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift index b1f9130e8..75422c44e 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift @@ -35,13 +35,19 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource { var paginationState: TimelinePaginationState + /// The gallery attachments to include, so that a gallery only contributes the media being browsed. + private let allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? + /// 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) + paginationState: TimelinePaginationState, + allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? = nil) { + self.allowedGalleryItemTypes = allowedGalleryItemTypes + + previewItems = itemViewStates.flatMap { $0.previewableMedia(allowedGalleryItemTypes: allowedGalleryItemTypes) } let initialPreviewID = TimelineMediaPreviewItem.Media(timelineItem: initialItem).id if let initialItemArrayIndex = previewItems.firstIndex(where: { $0.id == initialPreviewID }) { @@ -66,6 +72,8 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource { /// gallery and there's no timeline pagination to drive. init(galleryItem: GalleryRoomTimelineItem, initialIndex: Int) { + allowedGalleryItemTypes = nil // All of the gallery's attachments are shown. + let media = galleryItem.content.items.map { item in TimelineMediaPreviewItem.Media(galleryParent: galleryItem, item: item) } @@ -93,17 +101,17 @@ class TimelineMediaPreviewDataSource: NSObject, QLPreviewControllerDataSource { } func updatePreviewItems(itemViewStates: [RoomTimelineItemViewState]) { - let newItems: [TimelineMediaPreviewItem.Media] = itemViewStates.compactMap { itemViewState in - guard let newItem = TimelineMediaPreviewItem.Media(roomTimelineItemViewState: itemViewState) else { return nil } - - // 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.content = newItem.content - return oldItem + let newItems: [TimelineMediaPreviewItem.Media] = itemViewStates + .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 }) { + oldItem.content = newItem.content + return oldItem + } + + return newItem } - - return newItem - } var hasPaginated = false if let range = newItems.map(\.id).firstRange(of: previewItems.map(\.id)) { @@ -430,3 +438,23 @@ enum TimelineMediaPreviewItem: Equatable { } } } + +private extension RoomTimelineItemViewState { + /// The media of this item that can be previewed, flattening a gallery message into its individual + /// attachments so that they can be browsed alongside the timeline's other media. + /// - Parameter allowedGalleryItemTypes: Restricts a gallery's attachments to the media being + /// browsed, as the SDK can only filter the gallery event as a whole. + func previewableMedia(allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]?) -> [TimelineMediaPreviewItem.Media] { + guard case .gallery(let galleryItem) = type else { + return [TimelineMediaPreviewItem.Media(roomTimelineItemViewState: self)].compactMap { $0 } + } + + return galleryItem.content.items.filter { item in + guard let allowedItemType = item.allowedItemType else { + return false // Nothing to preview, and unlike a tile there's no index to line up with. + } + + return allowedGalleryItemTypes?.contains(allowedItemType) ?? true + }.map { TimelineMediaPreviewItem.Media(galleryParent: galleryItem, item: $0) } + } +} diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift index 15e585607..0802d2920 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift @@ -22,10 +22,6 @@ 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 } @@ -47,13 +43,13 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { self.photoLibraryManager = photoLibraryManager self.userIndicatorController = userIndicatorController self.appMediator = appMediator - menuTimelineKind = nil let timelineState = timelineViewModel.context.viewState.timelineState super.init(initialViewState: TimelineMediaPreviewViewState(dataSource: .init(itemViewStates: timelineState.itemViewStates, initialItem: initialItem, - paginationState: timelineState.paginationState)), + paginationState: timelineState.paginationState, + allowedGalleryItemTypes: timelineViewModel.context.viewState.allowedGalleryItemTypes)), mediaProvider: mediaProvider) rebuildCurrentItemActions() @@ -99,7 +95,6 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { self.photoLibraryManager = photoLibraryManager self.userIndicatorController = userIndicatorController self.appMediator = appMediator - menuTimelineKind = .media(.roomScreenLive) super.init(initialViewState: TimelineMediaPreviewViewState(dataSource: .init(galleryItem: galleryItem, initialIndex: initialIndex)), @@ -236,7 +231,7 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType { pinnedEventIDs: timelineContext.viewState.pinnedEventIDs, isViewSourceEnabled: timelineContext.viewState.isViewSourceEnabled, areThreadsEnabled: timelineContext.viewState.areThreadsEnabled, - timelineKind: menuTimelineKind ?? timelineContext.viewState.timelineKind, + timelineKind: timelineContext.viewState.timelineKind, emojiProvider: timelineContext.viewState.emojiProvider) .makeActions() } diff --git a/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift b/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift index d240db9f5..3bb361619 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift @@ -107,6 +107,14 @@ class TimelineInteractionHandler { // swiftlint:disable:next cyclomatic_complexity func handleTimelineItemMenuAction(_ action: TimelineItemMenuAction, itemID: TimelineItemIdentifier) { + // Redacting needs the event alone, so it works even when the item isn't part of this timeline, + // such as one held by a media preview that was built from a different one. + if case .redact = action { + guard case let .event(_, eventOrTransactionID) = itemID else { fatalError() } + Task { await timelineController.redact(eventOrTransactionID) } + return + } + guard let timelineItem = timelineController.timelineItems.firstUsingStableID(itemID), let eventTimelineItem = timelineItem as? EventBasedTimelineItemProtocol else { return @@ -156,8 +164,7 @@ class TimelineInteractionHandler { UIPasteboard.general.url = permalinkURL } case .redact: - guard case let .event(_, eventOrTransactionID) = itemID else { fatalError() } - Task { await timelineController.redact(eventOrTransactionID) } + break // Handled above, before the timeline item is looked up. case .reply: guard let eventID = eventTimelineItem.id.eventID else { return } @@ -560,29 +567,23 @@ class TimelineInteractionHandler { timestamp: item.timestamp, timeoutDate: item.content.timeoutDate) return .displayLiveLocation(sender: item.sender, initialLiveLocationShare: initialLiveLocationShare) + // Galleries are included so that their attachments can be browsed as individual media. case let item as ImageRoomTimelineItem: - return await mediaPreviewAction(for: item, messageTypes: [.image, .video]) + return await mediaPreviewAction(for: item, messageTypes: [.image, .video, .gallery]) case let item as VideoRoomTimelineItem: - return await mediaPreviewAction(for: item, messageTypes: [.image, .video]) + return await mediaPreviewAction(for: item, messageTypes: [.image, .video, .gallery]) case let item as AudioRoomTimelineItem: - return await mediaPreviewAction(for: item, messageTypes: [.audio, .file]) + return await mediaPreviewAction(for: item, messageTypes: [.audio, .file, .gallery]) case let item as FileRoomTimelineItem: - return await mediaPreviewAction(for: item, messageTypes: [.audio, .file]) + return await mediaPreviewAction(for: item, messageTypes: [.audio, .file, .gallery]) + case let item as GalleryRoomTimelineItem: + // Only galleries are needed as the preview is scoped to the attachments of the tapped one. + return await mediaPreviewAction(for: item, messageTypes: [.gallery]) default: return .none } } - /// 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 { @@ -613,7 +614,10 @@ class TimelineInteractionHandler { case .pinned: newTimelineFocus = .pinned newTimelinePresentation = .pinnedEventsScreen - case .media, .thread: + case .thread(let rootEventID): + newTimelineFocus = .thread(eventID: rootEventID) + newTimelinePresentation = .roomScreenThread + case .media: break // We don't need to create a new timeline as it is already filtered. } @@ -644,11 +648,21 @@ class TimelineInteractionHandler { linkMetadataProvider: linkMetadataProvider, timelineControllerFactory: timelineControllerFactory) - return .displayMediaPreview(item: item, timelineViewModel: .new(timelineViewModel)) + return previewAction(for: item, timelineViewModel: .new(timelineViewModel)) } else { - return .displayMediaPreview(item: item, timelineViewModel: .active) + return previewAction(for: item, timelineViewModel: .active) } } + + /// A gallery is previewed scoped to its own attachments rather than the wider timeline's media. + private func previewAction(for item: EventBasedMessageTimelineItemProtocol, + timelineViewModel: TimelineControllerAction.TimelineViewModelKind) -> TimelineControllerAction { + guard let galleryItem = item as? GalleryRoomTimelineItem else { + return .displayMediaPreview(item: item, timelineViewModel: timelineViewModel) + } + + return .displayGalleryPreview(galleryItem: galleryItem, timelineViewModel: timelineViewModel) + } } private struct ReplyInfo { diff --git a/ElementX/Sources/Screens/Timeline/TimelineModels.swift b/ElementX/Sources/Screens/Timeline/TimelineModels.swift index 406fa78e2..bfa2fdd30 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineModels.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineModels.swift @@ -104,6 +104,8 @@ enum TimelineComposerAction { struct TimelineViewState: BindableState { let timelineKind: TimelineKind + /// The gallery attachments the timeline includes, or `nil` when it isn't filtered. + let allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? var roomID: String var members: [String: RoomMemberState] = [:] var typingMembers: [String] = [] diff --git a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift index f69a4e6b4..12d61cc5c 100644 --- a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift +++ b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift @@ -92,6 +92,7 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { true } super.init(initialViewState: TimelineViewState(timelineKind: timelineController.timelineKind, + allowedGalleryItemTypes: timelineController.allowedGalleryItemTypes, roomID: roomProxy.id, isDM: roomProxy.infoPublisher.value.isDM, timelineState: TimelineState(focussedEvent: focussedEventID.map { .init(eventID: $0, appearance: .immediate) }), @@ -160,7 +161,7 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { case .mediaTapped(let id): Task { await handleMediaTapped(with: id) } case .galleryItemTapped(let id, let index): - handleGalleryItemTapped(itemID: id, index: index) + Task { await handleMediaTapped(with: id, galleryIndex: index) } case .itemSendInfoTapped(let itemID): handleItemSendInfoTapped(itemID: itemID) case .toggleReaction(let emoji, let itemID): @@ -700,7 +701,7 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { await timelineController.sendReadReceipt(for: lastVisibleItemID) } - private func handleMediaTapped(with itemID: TimelineItemIdentifier) async { + private func handleMediaTapped(with itemID: TimelineItemIdentifier, galleryIndex: Int? = nil) async { state.showLoading = true let action = await timelineInteractionHandler.processItemTap(itemID) @@ -710,8 +711,13 @@ 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 .displayGalleryPreview(let galleryItem, let timelineViewModelKind): + actionsSubject.send(.composer(action: .removeFocus)) + + let mediaPreviewViewModel = makeGalleryPreviewViewModel(galleryItem: galleryItem, + timelineViewModelKind: timelineViewModelKind, + initialIndex: galleryIndex ?? 0) + actionsSubject.send(.displayMediaPreview(mediaPreviewViewModel)) case .displayLocation(let location): actionsSubject.send(.displayLocation(location)) case .displayLiveLocation(let sender, let initialLiveLocationShare): @@ -722,18 +728,6 @@ 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.") @@ -836,30 +830,33 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol { private func makeMediaPreviewViewModel(item: EventBasedMessageTimelineItemProtocol, timelineViewModelKind: TimelineControllerAction.TimelineViewModelKind) -> TimelineMediaPreviewViewModel { - let timelineViewModel = switch timelineViewModelKind { - case .active: self - case .new(let newViewModel): newViewModel - } - - return TimelineMediaPreviewViewModel(initialItem: item, - timelineViewModel: timelineViewModel, - mediaProvider: userSession.mediaProvider, - photoLibraryManager: PhotoLibraryManager(), - userIndicatorController: userIndicatorController, - appMediator: appMediator) - } - - private func makeGalleryPreviewViewModel(galleryItem: GalleryRoomTimelineItem, - initialIndex: Int) -> TimelineMediaPreviewViewModel { - TimelineMediaPreviewViewModel(galleryItem: galleryItem, - initialIndex: initialIndex, - timelineViewModel: self, + TimelineMediaPreviewViewModel(initialItem: item, + timelineViewModel: timelineViewModel(for: timelineViewModelKind), mediaProvider: userSession.mediaProvider, photoLibraryManager: PhotoLibraryManager(), userIndicatorController: userIndicatorController, appMediator: appMediator) } + private func makeGalleryPreviewViewModel(galleryItem: GalleryRoomTimelineItem, + timelineViewModelKind: TimelineControllerAction.TimelineViewModelKind, + initialIndex: Int) -> TimelineMediaPreviewViewModel { + TimelineMediaPreviewViewModel(galleryItem: galleryItem, + initialIndex: initialIndex, + timelineViewModel: timelineViewModel(for: timelineViewModelKind), + mediaProvider: userSession.mediaProvider, + photoLibraryManager: PhotoLibraryManager(), + userIndicatorController: userIndicatorController, + appMediator: appMediator) + } + + private func timelineViewModel(for kind: TimelineControllerAction.TimelineViewModelKind) -> TimelineViewModel { + switch kind { + case .active: self + case .new(let newViewModel): newViewModel + } + } + // 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 cfdb05b8d..26fd01ad2 100644 --- a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryItemTileView.swift +++ b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryItemTileView.swift @@ -52,7 +52,7 @@ struct GalleryItemTileView: View { private var content: some View { if let source = item.thumbnailSource ?? item.mediaSource { LoadableImage(mediaSource: source, - mediaType: .timelineItem(uniqueID: item.id.timelineItemUniqueID), + mediaType: .timelineItem(uniqueID: item.id.timelineItemID.uniqueID), blurhash: item.blurhash, size: item.size, mediaProvider: mediaProvider) { imageView in diff --git a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryListView.swift b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryListView.swift index 513117ed3..e88021929 100644 --- a/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryListView.swift +++ b/ElementX/Sources/Screens/Timeline/View/TimelineItemViews/GalleryListView.swift @@ -110,7 +110,7 @@ private struct GalleryListRow: View { private var icon: some View { if item.hasThumbnail, let source = item.thumbnailSource ?? item.mediaSource { LoadableImage(mediaSource: source, - mediaType: .timelineItem(uniqueID: item.id.timelineItemUniqueID), + mediaType: .timelineItem(uniqueID: item.id.timelineItemID.uniqueID), blurhash: item.blurhash, size: item.size, mediaProvider: mediaProvider) { imageView in diff --git a/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift b/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift index eed3c0ec6..a568d958f 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/TimelineController.swift @@ -44,12 +44,16 @@ class TimelineController: TimelineControllerProtocol { activeTimelineItemProvider.kind } + let allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? + init(roomProxy: JoinedRoomProxyProtocol, timelineProxy: TimelineProxyProtocol, initialFocussedEventID: String?, timelineItemFactory: RoomTimelineItemFactoryProtocol, mediaProvider: MediaProviderProtocol, - appSettings: AppSettings) { + appSettings: AppSettings, + allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? = nil) { + self.allowedGalleryItemTypes = allowedGalleryItemTypes self.roomProxy = roomProxy liveTimelineItemProvider = timelineProxy.timelineItemProvider self.timelineItemFactory = timelineItemFactory diff --git a/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerFactory.swift b/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerFactory.swift index 6b07d1d2f..7917773db 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerFactory.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerFactory.swift @@ -71,7 +71,8 @@ struct TimelineControllerFactory: TimelineControllerFactoryProtocol { initialFocussedEventID: nil, timelineItemFactory: timelineItemFactory, mediaProvider: mediaProvider, - appSettings: appSettings)) + appSettings: appSettings, + allowedGalleryItemTypes: allowedMessageTypes.allowedGalleryItemTypes)) case .failure(let error): return .failure(.roomProxyError(error)) } diff --git a/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift b/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift index b5f7c129a..c90b07fc6 100644 --- a/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift +++ b/ElementX/Sources/Services/Timeline/TimelineController/TimelineControllerProtocol.swift @@ -26,7 +26,7 @@ enum TimelineControllerAction { } case displayMediaPreview(item: EventBasedMessageTimelineItemProtocol, timelineViewModel: TimelineViewModelKind) - case displayGalleryPreview(galleryItem: GalleryRoomTimelineItem, initialIndex: Int) + case displayGalleryPreview(galleryItem: GalleryRoomTimelineItem, timelineViewModel: TimelineViewModelKind) case displayLocation(StaticLocationData) case displayLiveLocation(sender: TimelineItemSender, initialLiveLocationShare: LiveLocationShare) case none @@ -47,6 +47,9 @@ enum TimelineControllerError: Error { protocol TimelineControllerProtocol: Sendable { var timelineKind: TimelineKind { get } + /// The gallery attachments this timeline includes, or `nil` when it isn't filtered. + var allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? { get } + /// The currently known items, use only for setting up the intial state. var timelineItems: [RoomTimelineItemProtocol] { get } diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift index 65df2f500..ab59254bf 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift @@ -19,8 +19,9 @@ nonisolated struct GalleryRoomTimelineItemContent: Hashable { /// Identifies a single attachment within a gallery message. nonisolated struct GalleryItemID: Hashable { - /// The unique ID of the parent gallery timeline item. - let timelineItemUniqueID: TimelineItemIdentifier.UniqueID + /// The identifier of the parent gallery timeline item. The whole identifier is used so that an + /// attachment remains identifiable across timelines, where the unique ID alone wouldn't be. + let timelineItemID: TimelineItemIdentifier /// The item's position within the gallery. let mediaIndex: Int } @@ -152,13 +153,24 @@ nonisolated enum GalleryItem: Hashable, Identifiable { var hasThumbnail: Bool { isImage || thumbnailSource != nil } + + /// The type this attachment is allowed by, or `nil` when its type is unknown. + var allowedItemType: TimelineAllowedGalleryItemType? { + switch self { + case .image: .image + case .video: .video + case .audio: .audio + case .file: .file + case .other: nil + } + } } // MARK: - Mocks nonisolated extension GalleryItemID { static func mock(_ index: Int) -> GalleryItemID { - .init(timelineItemUniqueID: .init("gallery-mock"), mediaIndex: index) + .init(timelineItemID: .randomEvent, mediaIndex: index) } } diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift index 0376a1176..05434ab28 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/RoomTimelineItemFactory.swift @@ -149,7 +149,7 @@ nonisolated struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol { case .location(let content): .location(buildLocationTimelineItemContent(content)) case .gallery(let content): - .gallery(buildGalleryTimelineItemContent(content, uniqueID: .init(senderID))) + .gallery(buildGalleryTimelineItemContent(content, timelineItemID: .randomEvent)) case .other(_, let body): .text(.init(body: body)) case .none: @@ -417,7 +417,7 @@ nonisolated struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol { canBeRepliedTo: eventItemProxy.canBeRepliedTo, shouldBoost: eventItemProxy.shouldBoost, sender: eventItemProxy.sender, - content: buildGalleryTimelineItemContent(galleryMessageContent, uniqueID: eventItemProxy.id.uniqueID), + content: buildGalleryTimelineItemContent(galleryMessageContent, timelineItemID: eventItemProxy.id), properties: .init(replyDetails: buildTimelineItemReplyDetails(messageLikeContent.inReplyTo), isThreaded: messageLikeContent.threadRoot != nil, threadSummary: buildTimelineItemThreadSummary(messageLikeContent.threadSummary), @@ -429,13 +429,13 @@ nonisolated struct RoomTimelineItemFactory: RoomTimelineItemFactoryProtocol { encryptionForwarder: eventItemProxy.forwarder)) } - private func buildGalleryTimelineItemContent(_ messageContent: GalleryMessageContent, uniqueID: TimelineItemIdentifier.UniqueID) -> GalleryRoomTimelineItemContent { + private func buildGalleryTimelineItemContent(_ messageContent: GalleryMessageContent, timelineItemID: TimelineItemIdentifier) -> GalleryRoomTimelineItemContent { let htmlCaption = messageContent.formatted?.format == .html ? messageContent.formatted?.body : nil let plainCaption = messageContent.formatted?.format != .html ? messageContent.formatted?.body : nil let formattedCaption = htmlCaption != nil ? attributedStringBuilder.fromHTML(htmlCaption) : (plainCaption.flatMap(attributedStringBuilder.fromPlain)) let items = messageContent.itemtypes.enumerated().map { index, itemType in - buildGalleryItem(itemType, id: GalleryItemID(timelineItemUniqueID: uniqueID, mediaIndex: index)) + buildGalleryItem(itemType, id: GalleryItemID(timelineItemID: timelineItemID, mediaIndex: index)) } return GalleryRoomTimelineItemContent(body: messageContent.body, diff --git a/ElementX/Sources/Services/Timeline/TimelineProxy.swift b/ElementX/Sources/Services/Timeline/TimelineProxy.swift index 477c560e6..8acd269d8 100644 --- a/ElementX/Sources/Services/Timeline/TimelineProxy.swift +++ b/ElementX/Sources/Services/Timeline/TimelineProxy.swift @@ -94,7 +94,7 @@ final class TimelineProxy: TimelineProxyProtocol { } func messageEventContent(for timelineItemID: TimelineItemIdentifier) async -> RoomMessageEventContentWithoutRelation? { - guard let content = timelineItemProvider.itemProxies.firstEventTimelineItemUsingStableID(timelineItemID)?.content, + guard let content = timelineItemProvider.itemProxies.firstEventTimelineItem(matching: timelineItemID)?.content, case let .msgLike(messageLikeContent) = content, case let .message(messageContent) = messageLikeContent.kind else { return nil @@ -663,15 +663,19 @@ private extension MatrixRustSDK.PollKind { } extension Array where Element == TimelineItemProxy { - func firstEventTimelineItemUsingStableID(_ id: TimelineItemIdentifier) -> EventTimelineItem? { - for item in self { - if case let .event(eventTimelineItem) = item { - if eventTimelineItem.id.uniqueID == id.uniqueID { - return eventTimelineItem.item - } - } + /// The event matching the given identifier, found by the event itself as that is the only part of + /// the identifier that can be compared across timelines. Identifiers that don't represent an event + /// fall back to the unique ID. + func firstEventTimelineItem(matching id: TimelineItemIdentifier) -> EventTimelineItem? { + let eventTimelineItems = compactMap { item -> EventTimelineItemProxy? in + guard case let .event(eventTimelineItem) = item else { return nil } + return eventTimelineItem } - return nil + guard let eventOrTransactionID = id.eventOrTransactionID else { + return eventTimelineItems.first { $0.id.uniqueID == id.uniqueID }?.item + } + + return eventTimelineItems.first { $0.id.eventOrTransactionID == eventOrTransactionID }?.item } } diff --git a/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift b/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift index c0d5f5e1f..0f406969c 100644 --- a/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift +++ b/ElementX/Sources/Services/Timeline/TimelineProxyProtocol.swift @@ -16,7 +16,7 @@ enum TimelineKind: Equatable { case pinned case thread(rootEventID: String) - enum MediaPresentation { case roomScreenLive, roomScreenDetached, pinnedEventsScreen, mediaFilesScreen } + enum MediaPresentation { case roomScreenLive, roomScreenDetached, roomScreenThread, pinnedEventsScreen, mediaFilesScreen } case media(MediaPresentation) var isThread: Bool { @@ -42,6 +42,34 @@ enum TimelineFocus { enum TimelineAllowedMessageType { case audio, file, image, video, gallery + + /// The gallery attachments this type allows, so that a gallery event's attachments can be + /// filtered client side to match. A gallery can't contain another gallery, hence the `nil`. + var allowedGalleryItemType: TimelineAllowedGalleryItemType? { + switch self { + case .audio: .audio + case .file: .file + case .image: .image + case .video: .video + case .gallery: nil + } + } +} + +/// The gallery attachments a timeline allows, mirroring the message types it is filtered to. The SDK can +/// only filter whole events, so a gallery's attachments are filtered by these client side. Unlike a +/// message type there's no gallery case, as a gallery can't contain another one. +enum TimelineAllowedGalleryItemType { + case audio, file, image, video +} + +extension [TimelineAllowedMessageType] { + /// The gallery attachments these types allow, or `nil` when they place no restriction on them, + /// which is the case for a timeline made up of galleries alone. + var allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]? { + let types = compactMap(\.allowedGalleryItemType) + return types.isEmpty ? nil : types + } } enum TimelineProxyError: Error { diff --git a/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift b/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift index 5c062e0c7..131b60028 100644 --- a/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift +++ b/UnitTests/Sources/TimelineMediaPreviewDataSourceTests.swift @@ -29,27 +29,58 @@ struct TimelineMediaPreviewDataSourceTests { try assertInitialDataSource() } + @Test + func timelinePreviewFlattensGalleries() throws { + // Given a timeline containing an image, a gallery of 2 attachments and a gallery of 3, one of + // which is a file that belongs in the other timeline. + let image = ImageRoomTimelineItem(id: .randomEvent, + timestamp: .mock, + isOutgoing: false, + isEditable: false, + canBeRepliedTo: true, + sender: .init(id: "Bob"), + content: .init(filename: "image.jpg", imageInfo: .mockImage, thumbnailInfo: nil, blurhash: nil)) + let firstGalleryItems: [GalleryItem] = [.mockImage(index: 0), .mockVideo(index: 1)] + let secondGalleryItems: [GalleryItem] = [.mockImage(index: 0), .mockFile(index: 1), .mockImage(index: 2)] + let timelineItems: [RoomTimelineItemProtocol] = [image, + makeGallery(items: firstGalleryItems), + makeGallery(items: secondGalleryItems)] + let itemViewStates = timelineItems.map { RoomTimelineItemViewState(item: $0, groupStyle: .single) } + + // When opening the preview from the image, which isn't part of a gallery. + let dataSource = TimelineMediaPreviewDataSource(itemViewStates: itemViewStates, + initialItem: image, + paginationState: .initial, + allowedGalleryItemTypes: [.image, .video]) + + // Then the galleries' visual attachments are browsable as though they were individual messages, + // whilst the file is left to the timeline that shows those. + let imageID = try #require(image.id.eventOrTransactionID) + let expectedIDs: [MediaPreviewItemID] = [.timelineItem(imageID), + .galleryItem(firstGalleryItems[0].id), + .galleryItem(firstGalleryItems[1].id), + .galleryItem(secondGalleryItems[0].id), + .galleryItem(secondGalleryItems[2].id)] + #expect(dataSource.previewItems.map(\.id) == expectedIDs) + + // …starting from the tapped image. + #expect(dataSource.currentMediaItemID == .timelineItem(imageID)) + } + @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()) + // Given a gallery message whose attachments are a mix of media types. + let items: [GalleryItem] = [.mockImage(index: 0), .mockFile(index: 1), .mockVideo(index: 2), .mockAudio(index: 3)] + let gallery = makeGallery(items: items) // 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) + // Then it is self contained, holding every one of the attachments without filtering any of + // them out and with no surrounding pagination… + let expectedIDs: [MediaPreviewItemID] = items.map { .galleryItem($0.id) } + #expect(dataSource.previewItems.map(\.id) == expectedIDs) + #expect(dataSource.numberOfPreviewItems(in: previewController) == items.count) #expect(dataSource.initialItemIndex == 1) // …and the initial item is the tapped attachment. @@ -294,6 +325,17 @@ struct TimelineMediaPreviewDataSourceTests { // MARK: Helpers + private func makeGallery(items: [GalleryItem]) -> GalleryRoomTimelineItem { + .init(id: .randomEvent, + timestamp: .mock, + isOutgoing: false, + isEditable: false, + canBeRepliedTo: true, + sender: .init(id: "Bob"), + content: .init(body: "Gallery", caption: nil, items: items), + properties: .init()) + } + func newChunk() -> [EventBasedMessageTimelineItemProtocol] { TimelineFixtures.mediaChunk .compactMap { $0 as? EventBasedMessageTimelineItemProtocol }