Enable the media browser feature 🖼️ (#3642)

* Overlay a progress indicator for downloads instead of using a toast indicator. * Update the SDK. * Remove the feature flag for the media browser. * Remove the media captions feature flag too. * Add unit test cases for download failure and swiping between items. * Snapshots (with the media browser visible in the screen).
element-hq · Dec 19, 2024 · 3aa7edc · 3aa7edc
1 parent 21a1593
commit 3aa7edc
Show file tree

Hide file tree

Showing 25 changed files with 127 additions and 89 deletions.
diff --git a/ElementX.xcodeproj/project.pbxproj b/ElementX.xcodeproj/project.pbxproj
@@ -8427,7 +8427,7 @@
 			repositoryURL = "https://github.com/element-hq/matrix-rust-components-swift";
 			requirement = {
 				kind = exactVersion;
-				version = 1.0.82;
+				version = 1.0.83;
 			};
 		};
 		701C7BEF8F70F7A83E852DCC /* XCRemoteSwiftPackageReference "GZIP" */ = {

diff --git a/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
@@ -149,8 +149,8 @@
       "kind" : "remoteSourceControl",
       "location" : "https://github.com/element-hq/matrix-rust-components-swift",
       "state" : {
-        "revision" : "043fcb8c80bbb6257f3f2075f2a72b8c9fdcbed2",
-        "version" : "1.0.82"
+        "revision" : "0d20974d1c44225596b24af1ec1f36716dd6e512",
+        "version" : "1.0.83"
       }
     },
     {

diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings
@@ -145,6 +145,7 @@
 "common_developer_options" = "Developer options";
 "common_device_id" = "Device ID";
 "common_direct_chat" = "Direct chat";
+"common_download_failed" = "Download failed";
 "common_downloading" = "Downloading";
 "common_edited_suffix" = "(edited)";
 "common_editing" = "Editing";
@@ -160,6 +161,8 @@
 "common_favourite" = "Favourite";
 "common_favourited" = "Favourited";
 "common_file" = "File";
+"common_file_deleted" = "File deleted";
+"common_file_saved" = "File saved";
 "common_forward_message" = "Forward message";
 "common_frequently_used" = "Frequently used";
 "common_gif" = "GIF";
@@ -625,6 +628,7 @@
 "screen_login_title_with_homeserver" = "Sign in to %1$@";
 "screen_media_browser_delete_confirmation_subtitle" = "This file will be removed from the room and members won’t have access to it.";
 "screen_media_browser_delete_confirmation_title" = "Delete file?";
+"screen_media_browser_download_error_message" = "Check your internet connection and try again.";
 "screen_media_browser_files_empty_state_subtitle" = "Documents, audio files, and voice messages uploaded to this room will be shown here.";
 "screen_media_browser_files_empty_state_title" = "No files uploaded yet";
 "screen_media_browser_list_loading_files" = "Loading files…";

diff --git a/ElementX/Sources/Application/AppSettings.swift b/ElementX/Sources/Application/AppSettings.swift
@@ -49,8 +49,6 @@ final class AppSettings {
         case fuzzyRoomListSearchEnabled
         case enableOnlySignedDeviceIsolationMode
         case knockingEnabled
-        case createMediaCaptionsEnabled
-        case mediaBrowserEnabled
         case eventCacheEnabled
     }
 
@@ -292,12 +290,6 @@ final class AppSettings {
     @UserPreference(key: UserDefaultsKeys.knockingEnabled, defaultValue: false, storageType: .userDefaults(store))
     var knockingEnabled
 
-    @UserPreference(key: UserDefaultsKeys.createMediaCaptionsEnabled, defaultValue: false, storageType: .userDefaults(store))
-    var createMediaCaptionsEnabled
-
-    @UserPreference(key: UserDefaultsKeys.mediaBrowserEnabled, defaultValue: false, storageType: .userDefaults(store))
-    var mediaBrowserEnabled
-
     #endif
 
     // MARK: - Shared

diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift
@@ -330,6 +330,8 @@ internal enum L10n {
   internal static var commonDeviceId: String { return L10n.tr("Localizable", "common_device_id") }
   /// Direct chat
   internal static var commonDirectChat: String { return L10n.tr("Localizable", "common_direct_chat") }
+  /// Download failed
+  internal static var commonDownloadFailed: String { return L10n.tr("Localizable", "common_download_failed") }
   /// Downloading
   internal static var commonDownloading: String { return L10n.tr("Localizable", "common_downloading") }
   /// (edited)
@@ -362,6 +364,10 @@ internal enum L10n {
   internal static var commonFavourited: String { return L10n.tr("Localizable", "common_favourited") }
   /// File
   internal static var commonFile: String { return L10n.tr("Localizable", "common_file") }
+  /// File deleted
+  internal static var commonFileDeleted: String { return L10n.tr("Localizable", "common_file_deleted") }
+  /// File saved
+  internal static var commonFileSaved: String { return L10n.tr("Localizable", "common_file_saved") }
   /// Forward message
   internal static var commonForwardMessage: String { return L10n.tr("Localizable", "common_forward_message") }
   /// Frequently used
@@ -1396,6 +1402,8 @@ internal enum L10n {
   internal static var screenMediaBrowserDeleteConfirmationSubtitle: String { return L10n.tr("Localizable", "screen_media_browser_delete_confirmation_subtitle") }
   /// Delete file?
   internal static var screenMediaBrowserDeleteConfirmationTitle: String { return L10n.tr("Localizable", "screen_media_browser_delete_confirmation_title") }
+  /// Check your internet connection and try again.
+  internal static var screenMediaBrowserDownloadErrorMessage: String { return L10n.tr("Localizable", "screen_media_browser_download_error_message") }
   /// Documents, audio files, and voice messages uploaded to this room will be shown here.
   internal static var screenMediaBrowserFilesEmptyStateSubtitle: String { return L10n.tr("Localizable", "screen_media_browser_files_empty_state_subtitle") }
   /// No files uploaded yet

diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift
@@ -52,6 +52,7 @@ enum TimelineMediaPreviewAlertType {
 class TimelineMediaPreviewItem: NSObject, QLPreviewItem, Identifiable {
     let timelineItem: EventBasedMessageTimelineItemProtocol
     var fileHandle: MediaFileHandleProxy?
+    var downloadError: Error?
 
     init(timelineItem: EventBasedMessageTimelineItemProtocol) {
         self.timelineItem = timelineItem

diff --git a/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift b/ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift
@@ -80,21 +80,21 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
     }
 
     private func updateCurrentItem(_ previewItem: TimelineMediaPreviewItem) async {
+        previewItem.downloadError = nil // Clear any existing error.
         state.currentItem = previewItem
         currentItemIDHandler?(previewItem.id)
+
         rebuildCurrentItemActions()
 
         if previewItem.fileHandle == nil, let source = previewItem.mediaSource {
-            showDownloadingIndicator(itemID: previewItem.id)
-            defer { hideDownloadingIndicator(itemID: previewItem.id) }
-
             switch await mediaProvider.loadFileFromSource(source, filename: previewItem.filename) {
             case .success(let handle):
                 previewItem.fileHandle = handle
                 state.fileLoadedPublisher.send(previewItem.id)
             case .failure(let error):
                 MXLog.error("Failed loading media: \(error)")
-                showDownloadErrorIndicator()
+                context.objectWillChange.send() // Manually trigger the SwiftUI view update.
+                previewItem.downloadError = error
             }
         }
     }
@@ -108,7 +108,6 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
                                                       pinnedEventIDs: timelineContext.viewState.pinnedEventIDs,
                                                       isDM: timelineContext.viewState.isEncryptedOneToOneRoom,
                                                       isViewSourceEnabled: timelineContext.viewState.isViewSourceEnabled,
-                                                      isCreateMediaCaptionsEnabled: timelineContext.viewState.isCreateMediaCaptionsEnabled,
                                                       timelineKind: timelineContext.viewState.timelineKind,
                                                       emojiProvider: timelineContext.viewState.emojiProvider)
         state.currentItemActions = provider.makeActions()
@@ -156,39 +155,17 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
 
     // MARK: - Indicators
 
-    private func showDownloadingIndicator(itemID: TimelineItemIdentifier) {
-        let indicatorID = makeDownloadIndicatorID(itemID: itemID)
-        userIndicatorController.submitIndicator(UserIndicator(id: indicatorID,
-                                                              type: .toast(progress: .indeterminate),
-                                                              title: L10n.commonDownloading,
-                                                              persistent: true),
-                                                delay: .seconds(0.1)) // Don't show the indicator when the SDK loads the file from the store.
-    }
-
-    private func hideDownloadingIndicator(itemID: TimelineItemIdentifier) {
-        let indicatorID = makeDownloadIndicatorID(itemID: itemID)
-        userIndicatorController.retractIndicatorWithId(indicatorID)
-    }
-
-    // FIXME: Add the strings and correct indicator types
-    private func showDownloadErrorIndicator() {
-        userIndicatorController.submitIndicator(UserIndicator(id: downloadErrorIndicatorID,
-                                                              type: .modal,
-                                                              title: L10n.errorUnknown,
-                                                              iconName: "exclamationmark.circle.fill"))
-    }
-
     private func showRedactedIndicator() {
         userIndicatorController.submitIndicator(UserIndicator(id: statusIndicatorID,
                                                               type: .toast,
-                                                              title: "File deleted",
+                                                              title: L10n.commonFileDeleted,
                                                               iconName: "checkmark"))
     }
 
     private func showSavedIndicator() {
         userIndicatorController.submitIndicator(UserIndicator(id: statusIndicatorID,
                                                               type: .toast,
-                                                              title: "File saved",
+                                                              title: L10n.commonFileSaved,
                                                               iconName: "checkmark"))
     }
 
@@ -200,10 +177,4 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
     }
 
     private var statusIndicatorID: String { "\(Self.self)-Status" }
-
-    // Separate indicator IDs for downloads as these can be triggered in the background when swiping between items
-    private var downloadErrorIndicatorID: String { "\(Self.self)-DownloadError" }
-    private func makeDownloadIndicatorID(itemID: TimelineItemIdentifier) -> String {
-        "\(Self.self)-Download-\(itemID.uniqueID.id)"
-    }
 }
diff --git a/ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewScreen.swift b/ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewScreen.swift
@@ -47,6 +47,7 @@ struct TimelineMediaPreviewScreen: View {
         Color.clear // A completely clear view breaks any SwiftUI gestures (such as drag to dismiss).
             .background { QuickLookView(viewModelContext: context).ignoresSafeArea() } // Not the root view to stop QL hijacking the toolbar.
             .overlay(alignment: .topTrailing) { fullScreenButton }
+            .overlay { downloadStatusIndicator }
             .toolbar { toolbar }
             .toolbar(toolbarVisibility, for: .navigationBar)
             .toolbar(toolbarVisibility, for: .bottomBar)
@@ -69,6 +70,36 @@ struct TimelineMediaPreviewScreen: View {
         .padding(.trailing, 14)
     }
 
+    @ViewBuilder
+    private var downloadStatusIndicator: some View {
+        if currentItem.downloadError != nil {
+            VStack(spacing: 24) {
+                CompoundIcon(\.error, size: .custom(48), relativeTo: .compound.headingLG)
+                    .foregroundStyle(.compound.iconCriticalPrimary)
+                    .padding(.vertical, 24.5)
+                    .padding(.horizontal, 28.5)
+
+                VStack(spacing: 2) {
+                    Text(L10n.commonDownloadFailed)
+                        .font(.compound.headingMDBold)
+                        .foregroundStyle(.compound.textPrimary)
+                        .multilineTextAlignment(.center)
+                    Text(L10n.screenMediaBrowserDownloadErrorMessage)
+                        .font(.compound.bodyMD)
+                        .foregroundStyle(.compound.textPrimary)
+                        .multilineTextAlignment(.center)
+                }
+            }
+            .padding(.horizontal, 24)
+            .padding(.vertical, 40)
+            .background(.compound.bgSubtlePrimary, in: RoundedRectangle(cornerRadius: 14))
+        } else if currentItem.fileHandle == nil {
+            ProgressView()
+                .controlSize(.large)
+                .tint(.compound.iconPrimary)
+        }
+    }
+
     @ViewBuilder
     private var caption: some View {
         if let caption = currentItem.caption, !isFullScreen {
@@ -220,12 +251,19 @@ struct TimelineMediaPreviewScreen_Previews: PreviewProvider {
     @Namespace private static var namespace
 
     static let viewModel = makeViewModel()
+    static let downloadingViewModel = makeViewModel(isDownloading: true)
+    static let downloadErrorViewModel = makeViewModel(isDownloadError: true)
 
     static var previews: some View {
         TimelineMediaPreviewScreen(context: viewModel.context)
+            .previewDisplayName("Normal")
+        TimelineMediaPreviewScreen(context: downloadingViewModel.context)
+            .previewDisplayName("Downloading")
+        TimelineMediaPreviewScreen(context: downloadErrorViewModel.context)
+            .previewDisplayName("Download Error")
     }
 
-    static func makeViewModel() -> TimelineMediaPreviewViewModel {
+    static func makeViewModel(isDownloading: Bool = false, isDownloadError: Bool = false) -> TimelineMediaPreviewViewModel {
         let item = FileRoomTimelineItem(id: .randomEvent,
                                         timestamp: .mock,
                                         isOutgoing: false,
@@ -243,11 +281,22 @@ struct TimelineMediaPreviewScreen_Previews: PreviewProvider {
         let timelineController = MockRoomTimelineController(timelineKind: .media(.mediaFilesScreen))
         timelineController.timelineItems = [item]
 
+        let mediaProvider = MediaProviderMock(configuration: .init())
+
+        if isDownloading {
+            mediaProvider.loadFileFromSourceFilenameClosure = { _, _ in
+                try? await Task.sleep(for: .seconds(3600))
+                return .failure(.failedRetrievingFile)
+            }
+        } else if isDownloadError {
+            mediaProvider.loadFileFromSourceFilenameClosure = { _, _ in .failure(.failedRetrievingFile) }
+        }
+
         return TimelineMediaPreviewViewModel(context: .init(item: item,
                                                             viewModel: TimelineViewModel.mock(timelineKind: timelineController.timelineKind,
                                                                                               timelineController: timelineController),
                                                             namespace: namespace),
-                                             mediaProvider: MediaProviderMock(configuration: .init()),
+                                             mediaProvider: mediaProvider,
                                              photoLibraryManager: PhotoLibraryManagerMock(.init()),
                                              userIndicatorController: UserIndicatorControllerMock(),
                                              appMediator: AppMediatorMock())

diff --git a/ElementX/Sources/Screens/PinnedEventsTimelineScreen/View/PinnedEventsTimelineScreen.swift b/ElementX/Sources/Screens/PinnedEventsTimelineScreen/View/PinnedEventsTimelineScreen.swift
@@ -37,7 +37,6 @@ struct PinnedEventsTimelineScreen: View {
                                                              pinnedEventIDs: timelineContext.viewState.pinnedEventIDs,
                                                              isDM: timelineContext.viewState.isEncryptedOneToOneRoom,
                                                              isViewSourceEnabled: timelineContext.viewState.isViewSourceEnabled,
-                                                             isCreateMediaCaptionsEnabled: timelineContext.viewState.isCreateMediaCaptionsEnabled,
                                                              timelineKind: timelineContext.viewState.timelineKind,
                                                              emojiProvider: timelineContext.viewState.emojiProvider)
                     .makeActions()

diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift
@@ -63,8 +63,6 @@ struct RoomDetailsScreenViewState: BindableState {
         knockingEnabled && dmRecipient == nil && canEditRolesOrPermissions
     }
 
-    var mediaBrowserEnabled = false
-
     var canEdit: Bool {
         !isDirect && (canEditRoomName || canEditRoomTopic || canEditRoomAvatar)
     }

diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift b/ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift
@@ -79,10 +79,6 @@ class RoomDetailsScreenViewModel: RoomDetailsScreenViewModelType, RoomDetailsScr
             .weakAssign(to: \.state.knockingEnabled, on: self)
             .store(in: &cancellables)
 
-        appSettings.$mediaBrowserEnabled
-            .weakAssign(to: \.state.mediaBrowserEnabled, on: self)
-            .store(in: &cancellables)
-
         appMediator.networkMonitor.reachabilityPublisher
             .filter { $0 == .reachable }
             .receive(on: DispatchQueue.main)

diff --git a/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift b/ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift
@@ -170,12 +170,10 @@ struct RoomDetailsScreen: View {
                     })
                     .accessibilityIdentifier(A11yIdentifiers.roomDetailsScreen.pollsHistory)
 
-            if context.viewState.mediaBrowserEnabled {
-                ListRow(label: .default(title: L10n.screenMediaBrowserTitle, icon: \.image),
-                        kind: .navigationLink {
-                            context.send(viewAction: .processTapMediaEvents)
-                        })
-            }
+            ListRow(label: .default(title: L10n.screenMediaBrowserTitle, icon: \.image),
+                    kind: .navigationLink {
+                        context.send(viewAction: .processTapMediaEvents)
+                    })
         }
     }
 

diff --git a/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift b/ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift
@@ -75,7 +75,6 @@ struct RoomScreen: View {
                                                              pinnedEventIDs: timelineContext.viewState.pinnedEventIDs,
                                                              isDM: timelineContext.viewState.isEncryptedOneToOneRoom,
                                                              isViewSourceEnabled: timelineContext.viewState.isViewSourceEnabled,
-                                                             isCreateMediaCaptionsEnabled: timelineContext.viewState.isCreateMediaCaptionsEnabled,
                                                              timelineKind: timelineContext.viewState.timelineKind,
                                                              emojiProvider: timelineContext.viewState.emojiProvider)
                     .makeActions()

diff --git a/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/DeveloperOptionsScreenModels.swift b/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/DeveloperOptionsScreenModels.swift
@@ -50,8 +50,6 @@ protocol DeveloperOptionsProtocol: AnyObject {
     var enableOnlySignedDeviceIsolationMode: Bool { get set }
     var elementCallBaseURLOverride: URL? { get set }
     var knockingEnabled: Bool { get set }
-    var createMediaCaptionsEnabled: Bool { get set }
-    var mediaBrowserEnabled: Bool { get set }
     var eventCacheEnabled: Bool { get set }
 }
 

diff --git a/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/View/DeveloperOptionsScreen.swift b/ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/View/DeveloperOptionsScreen.swift
@@ -62,14 +62,6 @@ struct DeveloperOptionsScreen: View {
                 Toggle(isOn: $context.hideTimelineMedia) {
                     Text("Hide image & video previews")
                 }
-
-                Toggle(isOn: $context.createMediaCaptionsEnabled) {
-                    Text("Allow creation of media captions")
-                }
-
-                Toggle(isOn: $context.mediaBrowserEnabled) {
-                    Text("Enable the media browser")
-                }
             }
 
             Section("Join rules") {

diff --git a/ElementX/Sources/Screens/Timeline/TimelineModels.swift b/ElementX/Sources/Screens/Timeline/TimelineModels.swift
@@ -99,7 +99,6 @@ struct TimelineViewState: BindableState {
     var canCurrentUserRedactSelf = false
     var canCurrentUserPin = false
     var isViewSourceEnabled: Bool
-    var isCreateMediaCaptionsEnabled: Bool
     var hideTimelineMedia: Bool
 
     // The `pinnedEventIDs` are used only to determine if an item is already pinned or not.