Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enable the media browser feature 🖼️ #3642

Merged
merged 6 commits into from
Dec 19, 2024
Merged

Enable the media browser feature 🖼️ #3642

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
76ed202
Overlay a progress indicator for downloads instead of using a toast i…
pixlwave Dec 19, 2024
c731bd9
Update the SDK.
pixlwave Dec 19, 2024
c7f995a
Remove the feature flag for the media browser.
pixlwave Dec 19, 2024
ebf4bd4
Remove the media captions feature flag too.
pixlwave Dec 19, 2024
269f942
Add unit test cases for download failure and swiping between items.
pixlwave Dec 19, 2024
b5b63c1
Snapshots (with the media browser visible in the screen).
pixlwave Dec 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion ElementX.xcodeproj/project.pbxproj
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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" */ = {
Expand Down
4 changes: 2 additions & 2 deletions ElementX.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
},
{
Expand Down
4 changes: 4 additions & 0 deletions ElementX/Resources/Localizations/en.lproj/Localizable.strings
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand All @@ -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";
Expand Down Expand Up @@ -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…";
Expand Down
8 changes: 0 additions & 8 deletions ElementX/Sources/Application/AppSettings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,6 @@ final class AppSettings {
case fuzzyRoomListSearchEnabled
case enableOnlySignedDeviceIsolationMode
case knockingEnabled
case createMediaCaptionsEnabled
case mediaBrowserEnabled
case eventCacheEnabled
}

Expand Down Expand Up @@ -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
Expand Down
8 changes: 8 additions & 0 deletions ElementX/Sources/Generated/Strings.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
41 changes: 6 additions & 35 deletions ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Expand All @@ -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()
Expand Down Expand Up @@ -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"))
}

Expand All @@ -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)"
}
}
53 changes: 51 additions & 2 deletions ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand All @@ -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())
Expand Down
1 change: 0 additions & 1 deletion ElementX/Sources/Screens/PinnedEventsTimelineScreen/View/PinnedEventsTimelineScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 0 additions & 2 deletions ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenModels.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,6 @@ struct RoomDetailsScreenViewState: BindableState {
knockingEnabled && dmRecipient == nil && canEditRolesOrPermissions
}

var mediaBrowserEnabled = false

var canEdit: Bool {
!isDirect && (canEditRoomName || canEditRoomTopic || canEditRoomAvatar)
}
Expand Down
4 changes: 0 additions & 4 deletions ElementX/Sources/Screens/RoomDetailsScreen/RoomDetailsScreenViewModel.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 4 additions & 6 deletions ElementX/Sources/Screens/RoomDetailsScreen/View/RoomDetailsScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
})
}
}

Expand Down
1 change: 0 additions & 1 deletion ElementX/Sources/Screens/RoomScreen/View/RoomScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
2 changes: 0 additions & 2 deletions ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/DeveloperOptionsScreenModels.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 }
}

Expand Down
8 changes: 0 additions & 8 deletions ElementX/Sources/Screens/Settings/DeveloperOptionsScreen/View/DeveloperOptionsScreen.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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") {
Expand Down
1 change: 0 additions & 1 deletion ElementX/Sources/Screens/Timeline/TimelineModels.swift
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Loading
Loading