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).
This commit is contained in:
Doug 2024-12-19 14:15:31 +00:00 committed by GitHub
parent 21a15936a9
commit 3aa7edc508
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
25 changed files with 127 additions and 89 deletions

View File

@ -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" */ = {

View File

@ -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"
}
},
{

View File

@ -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 wont 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…";

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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)"
}
}

View File

@ -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())

View File

@ -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()

View File

@ -63,8 +63,6 @@ struct RoomDetailsScreenViewState: BindableState {
knockingEnabled && dmRecipient == nil && canEditRolesOrPermissions
}
var mediaBrowserEnabled = false
var canEdit: Bool {
!isDirect && (canEditRoomName || canEditRoomTopic || canEditRoomAvatar)
}

View File

@ -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)

View File

@ -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)
})
}
}

View File

@ -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()

View File

@ -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 }
}

View File

@ -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") {

View File

@ -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.

View File

@ -81,7 +81,6 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
timelineState: TimelineState(focussedEvent: focussedEventID.map { .init(eventID: $0, appearance: .immediate) }),
ownUserID: roomProxy.ownUserID,
isViewSourceEnabled: appSettings.viewSourceEnabled,
isCreateMediaCaptionsEnabled: appSettings.createMediaCaptionsEnabled,
hideTimelineMedia: appSettings.hideTimelineMedia,
pinnedEventIDs: roomProxy.infoPublisher.value.pinnedEventIDs,
bindings: .init(reactionsCollapsed: [:]),
@ -449,10 +448,6 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
.weakAssign(to: \.state.isViewSourceEnabled, on: self)
.store(in: &cancellables)
appSettings.$createMediaCaptionsEnabled
.weakAssign(to: \.state.isCreateMediaCaptionsEnabled, on: self)
.store(in: &cancellables)
appSettings.$hideTimelineMedia
.weakAssign(to: \.state.hideTimelineMedia, on: self)
.store(in: &cancellables)

View File

@ -344,7 +344,6 @@ struct TimelineItemMenu_Previews: PreviewProvider, TestablePreview {
pinnedEventIDs: [],
isDM: true,
isViewSourceEnabled: true,
isCreateMediaCaptionsEnabled: true,
timelineKind: .live,
emojiProvider: EmojiProvider(appSettings: ServiceLocator.shared.settings))
guard let actions = provider.makeActions() else { return nil }

View File

@ -16,7 +16,6 @@ struct TimelineItemMenuActionProvider {
let pinnedEventIDs: Set<String>
let isDM: Bool
let isViewSourceEnabled: Bool
let isCreateMediaCaptionsEnabled: Bool
let timelineKind: TimelineKind
let emojiProvider: EmojiProviderProtocol
@ -63,7 +62,7 @@ struct TimelineItemMenuActionProvider {
if item.supportsMediaCaption {
if item.hasMediaCaption {
actions.append(.editCaption)
} else if isCreateMediaCaptionsEnabled {
} else {
actions.append(.addCaption)
}
} else if item is PollRoomTimelineItem {

View File

@ -149,7 +149,6 @@ struct TimelineItemBubbledStylerView<Content: View>: View {
pinnedEventIDs: context.viewState.pinnedEventIDs,
isDM: context.viewState.isEncryptedOneToOneRoom,
isViewSourceEnabled: context.viewState.isViewSourceEnabled,
isCreateMediaCaptionsEnabled: context.viewState.isCreateMediaCaptionsEnabled,
timelineKind: context.viewState.timelineKind,
emojiProvider: context.viewState.emojiProvider)
TimelineItemMacContextMenu(item: timelineItem, actionProvider: provider) { action in

View File

@ -36,6 +36,48 @@ class TimelineMediaPreviewViewModelTests: XCTestCase {
XCTAssertNotNil(context.viewState.currentItemActions)
}
func testLoadingItemFailure() async throws {
// Given a fresh view model.
setupViewModel()
XCTAssertFalse(mediaProvider.loadFileFromSourceFilenameCalled)
XCTAssertEqual(context.viewState.currentItem, context.viewState.previewItems[0])
XCTAssertNil(context.viewState.currentItem.downloadError)
// When the preview controller sets an item that fails to load.
mediaProvider.loadFileFromSourceFilenameClosure = { _, _ in .failure(.failedRetrievingFile) }
let failure = deferFailure(viewModel.state.fileLoadedPublisher, timeout: 1) { _ in true }
context.send(viewAction: .updateCurrentItem(context.viewState.previewItems[0]))
try await failure.fulfill()
// Then the view model should load the item and update its view state.
XCTAssertTrue(mediaProvider.loadFileFromSourceFilenameCalled)
XCTAssertEqual(context.viewState.currentItem, context.viewState.previewItems[0])
XCTAssertNotNil(context.viewState.currentItem.downloadError)
}
func testSwipingBetweenItems() async throws {
// Given a view model with a loaded item.
try await testLoadingItem()
// When swiping to another item.
let deferred = deferFulfillment(viewModel.state.fileLoadedPublisher) { _ in true }
context.send(viewAction: .updateCurrentItem(context.viewState.previewItems[1]))
try await deferred.fulfill()
// Then the view model should load the item and update its view state.
XCTAssertEqual(mediaProvider.loadFileFromSourceFilenameCallsCount, 2)
XCTAssertEqual(context.viewState.currentItem, context.viewState.previewItems[1])
// When swiping back to the first item.
let failure = deferFailure(viewModel.state.fileLoadedPublisher, timeout: 1) { _ in true }
context.send(viewAction: .updateCurrentItem(context.viewState.previewItems[0]))
try await failure.fulfill()
// Then the view model should not need to load the item, but should still update its view state.
XCTAssertEqual(mediaProvider.loadFileFromSourceFilenameCallsCount, 2)
XCTAssertEqual(context.viewState.currentItem, context.viewState.previewItems[0])
}
func testViewInRoomTimeline() async throws {
// Given a view model with a loaded item.
try await testLoadingItem()

View File

@ -61,7 +61,7 @@ packages:
# Element/Matrix dependencies
MatrixRustSDK:
url: https://github.com/element-hq/matrix-rust-components-swift
exactVersion: 1.0.82
exactVersion: 1.0.83
# path: ../matrix-rust-sdk
Compound:
url: https://github.com/element-hq/compound-ios