From b04476431010e514e3f87029f86c8a14c90f005c Mon Sep 17 00:00:00 2001 From: Mauro Romito Date: Thu, 30 Jul 2026 15:18:59 +0200 Subject: [PATCH] unified the filtering of the gallery items --- .../TimelineMediaPreviewDataSource.swift | 10 +++----- .../MediaEventsTimelineScreenModels.swift | 2 +- .../MediaEventsTimelineScreenViewModel.swift | 25 +++++++++---------- .../View/MediaEventsTimelineScreen.swift | 10 +++++++- .../GalleryRoomTimelineItemContent.swift | 15 +++++++++++ ...iaEventsTimelineScreenViewModelTests.swift | 12 +++++---- 6 files changed, 47 insertions(+), 27 deletions(-) diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift index 75422c44e..6d67a0c9f 100644 --- a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift +++ b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewDataSource.swift @@ -449,12 +449,8 @@ private extension RoomTimelineItemViewState { 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) } + return galleryItem.content.attachments(allowedTypes: allowedGalleryItemTypes).map { + TimelineMediaPreviewItem.Media(galleryParent: galleryItem, item: $0.item) + } } } diff --git a/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenModels.swift b/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenModels.swift index 4292c160c..6528c3332 100644 --- a/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenModels.swift +++ b/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenModels.swift @@ -13,7 +13,7 @@ enum MediaEventsTimelineScreenViewModelAction { case viewInRoomTimeline(TimelineItemIdentifier) } -nonisolated enum MediaEventsTimelineScreenMode { +enum MediaEventsTimelineScreenMode { case media case files } diff --git a/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenViewModel.swift b/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenViewModel.swift index 5fb46aee1..966b6e412 100644 --- a/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenViewModel.swift +++ b/ElementX/Sources/Screens/MediaEventsTimelineScreen/MediaEventsTimelineScreenViewModel.swift @@ -204,7 +204,7 @@ class MediaEventsTimelineScreenViewModel: MediaEventsTimelineScreenViewModelType return [itemViewState] case .gallery(let galleryItem): // A gallery's attachments are displayed flattened, listed individually. - let flattenedItems = galleryItem.itemsAsIndividualMessages(for: state.screenMode) + let flattenedItems = galleryItem.itemsAsIndividualMessages(allowedTypes: timelineViewState.allowedGalleryItemTypes) for (mediaIndex, item) in flattenedItems { newGalleryItemLookup[item.id.uniqueID] = .init(parentID: galleryItem.id, mediaIndex: mediaIndex) } @@ -285,32 +285,31 @@ class MediaEventsTimelineScreenViewModel: MediaEventsTimelineScreenViewModelType } private extension GalleryRoomTimelineItem { - /// Represents the gallery's attachments as though each had been sent as an individual message, so - /// that the screen can list them alongside the room's other media, keeping only those that belong - /// in the given mode. Each one keeps the gallery's event ID but is given its own unique ID so that - /// the attachments remain distinct within the list. - func itemsAsIndividualMessages(for screenMode: MediaEventsTimelineScreenMode) -> [(mediaIndex: Int, item: EventBasedMessageTimelineItemProtocol)] { + /// Represents the gallery's attachments of the given types as though each had been sent as an + /// individual message, so that the screen can list them alongside the room's other media. Each one + /// keeps the gallery's event ID but is given its own unique ID so that they remain distinct. + func itemsAsIndividualMessages(allowedTypes: [TimelineAllowedGalleryItemType]?) -> [(mediaIndex: Int, item: EventBasedMessageTimelineItemProtocol)] { guard let eventOrTransactionID = id.eventOrTransactionID else { return [] } - return content.items.enumerated().compactMap { mediaIndex, galleryItem in + return content.attachments(allowedTypes: allowedTypes).compactMap { mediaIndex, galleryItem in let itemID = TimelineItemIdentifier.event(uniqueID: .init("\(id.uniqueID.value)-\(mediaIndex)"), eventOrTransactionID: eventOrTransactionID) let item: EventBasedMessageTimelineItemProtocol? = switch galleryItem { - case .image(_, let content) where screenMode == .media: + case .image(_, let content): ImageRoomTimelineItem(id: itemID, timestamp: timestamp, isOutgoing: isOutgoing, isEditable: isEditable, canBeRepliedTo: canBeRepliedTo, sender: sender, content: content, properties: properties) - case .video(_, let content) where screenMode == .media: + case .video(_, let content): VideoRoomTimelineItem(id: itemID, timestamp: timestamp, isOutgoing: isOutgoing, isEditable: isEditable, canBeRepliedTo: canBeRepliedTo, sender: sender, content: content, properties: properties) - case .audio(_, let content) where screenMode == .files: + case .audio(_, let content): AudioRoomTimelineItem(id: itemID, timestamp: timestamp, isOutgoing: isOutgoing, isEditable: isEditable, canBeRepliedTo: canBeRepliedTo, sender: sender, content: content, properties: properties) - case .file(_, let content) where screenMode == .files: + case .file(_, let content): FileRoomTimelineItem(id: itemID, timestamp: timestamp, isOutgoing: isOutgoing, isEditable: isEditable, canBeRepliedTo: canBeRepliedTo, sender: sender, content: content, properties: properties) - default: - nil // Attachments of an unknown type, or that belong in the other mode. + case .other: + nil // Filtered out above, as there's nothing to show for it. } guard let item else { return nil } diff --git a/ElementX/Sources/Screens/MediaEventsTimelineScreen/View/MediaEventsTimelineScreen.swift b/ElementX/Sources/Screens/MediaEventsTimelineScreen/View/MediaEventsTimelineScreen.swift index 6bb544f17..a390d5757 100644 --- a/ElementX/Sources/Screens/MediaEventsTimelineScreen/View/MediaEventsTimelineScreen.swift +++ b/ElementX/Sources/Screens/MediaEventsTimelineScreen/View/MediaEventsTimelineScreen.swift @@ -361,7 +361,15 @@ struct MediaEventsTimelineScreen_Previews: PreviewProvider, TestablePreview { timelineItems.append(contentsOf: [makeFileItem(url: .mockMXCScanning), makeFileItem(url: .mockMXCUnsafe)]) } - return TimelineControllerMock(.init(timelineKind: .media(.mediaFilesScreen), timelineItems: timelineItems)) + // Mirrors the message types that the flow coordinator filters each timeline to. + let allowedGalleryItemTypes: [TimelineAllowedGalleryItemType] = switch screenMode { + case .media: [.image, .video] + case .files: [.file, .audio] + } + + return TimelineControllerMock(.init(timelineKind: .media(.mediaFilesScreen), + timelineItems: timelineItems, + allowedGalleryItemTypes: allowedGalleryItemTypes)) } private static func makeImageItem(url: URL) -> ImageRoomTimelineItem { diff --git a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift index ab59254bf..385e281fa 100644 --- a/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift +++ b/ElementX/Sources/Services/Timeline/TimelineItems/Items/Messages/GalleryRoomTimelineItemContent.swift @@ -15,6 +15,21 @@ nonisolated struct GalleryRoomTimelineItemContent: Hashable { /// The original textual representation of the formatted caption directly from the event (usually HTML code) var formattedCaptionHTMLString: String? let items: [GalleryItem] + + /// The attachments of the given types, paired with their position in the gallery so that they remain + /// identifiable once separated from it. Attachments of an unknown type are never included as there's + /// nothing to show for them. + /// - Parameter allowedTypes: The types to include, or `nil` to include them all. + func attachments(allowedTypes: [TimelineAllowedGalleryItemType]?) -> [(mediaIndex: Int, item: GalleryItem)] { + items.enumerated().compactMap { mediaIndex, item in + guard let allowedItemType = item.allowedItemType, + allowedTypes?.contains(allowedItemType) ?? true else { + return nil + } + + return (mediaIndex, item) + } + } } /// Identifies a single attachment within a gallery message. diff --git a/UnitTests/Sources/MediaEventsTimelineScreenViewModelTests.swift b/UnitTests/Sources/MediaEventsTimelineScreenViewModelTests.swift index 85d36f206..223d9cd28 100644 --- a/UnitTests/Sources/MediaEventsTimelineScreenViewModelTests.swift +++ b/UnitTests/Sources/MediaEventsTimelineScreenViewModelTests.swift @@ -12,8 +12,8 @@ import Testing struct MediaEventsTimelineScreenViewModelTests { @Test func changingScreenModeSwitchesTimelines() { - let mediaTimelineViewModel = makeTimelineViewModel() - let filesTimelineViewModel = makeTimelineViewModel() + let mediaTimelineViewModel = makeTimelineViewModel(allowedGalleryItemTypes: [.image, .video]) + let filesTimelineViewModel = makeTimelineViewModel(allowedGalleryItemTypes: [.file, .audio]) let viewModel = MediaEventsTimelineScreenViewModel(mediaTimelineViewModel: mediaTimelineViewModel, filesTimelineViewModel: filesTimelineViewModel, @@ -45,10 +45,12 @@ struct MediaEventsTimelineScreenViewModelTests { // MARK: - Helpers - // Both timelines are given the same mixed chunk of items so that the mode based filtering is what's under test. - private func makeTimelineViewModel() -> TimelineViewModel { + // Both timelines are given the same mixed chunk of items, filtered as they are in the app, so that + // the filtering of the items is what's under test. + private func makeTimelineViewModel(allowedGalleryItemTypes: [TimelineAllowedGalleryItemType]) -> TimelineViewModel { let timelineController = TimelineControllerMock(.init(timelineKind: .media(.mediaFilesScreen), - timelineItems: [TimelineFixtures.separator] + TimelineFixtures.mediaChunk)) + timelineItems: [TimelineFixtures.separator] + TimelineFixtures.mediaChunk, + allowedGalleryItemTypes: allowedGalleryItemTypes)) return TimelineViewModel.mock(timelineKind: .media(.mediaFilesScreen), timelineController: timelineController) }