Media browser tweaks (#3692)

* Move the media actions from the bottom bar into the details sheet. * Allow the media type picker to fill the width of the screen.
2026-11-03 16:10:20 +00:00 · 2025-01-21 17:00:04 +00:00
parent 55e399aaf7
commit 909ee4abf2
43 changed files with 203 additions and 160 deletions
@@ -9,46 +9,26 @@ import Foundation
 import SwiftUI

 extension View {
-    /// Reads the frame of the view and store it in `frame` binding.
+    /// Reads the frame of the view and stores it in the `frame` binding.
    /// - Parameters:
    ///   - frame: a `CGRect` binding
    ///   - coordinateSpace: the coordinate space of the frame.
    func readFrame(_ frame: Binding<CGRect>, in coordinateSpace: CoordinateSpace = .local) -> some View {
-        background(ViewFrameReader(frame: frame, coordinateSpace: coordinateSpace))
-    }
-}
-
-/// Used to calculate the frame of a view.
-///
-/// Useful in situations as with `ZStack` where you might want to layout views using alignment guides.
-/// ```
-/// @State private var frame: CGRect = CGRect.zero
-/// ...
-/// SomeView()
-///    .background(ViewFrameReader(frame: $frame))
-/// ```
-private struct ViewFrameReader: View {
-    @Binding var frame: CGRect
-    var coordinateSpace: CoordinateSpace = .local
-    
-    var body: some View {
-        GeometryReader { geometry in
-            Color.clear
-                .preference(key: FramePreferenceKey.self,
-                            value: geometry.frame(in: coordinateSpace))
+        onGeometryChange(for: CGRect.self) { geometry in
+            geometry.frame(in: coordinateSpace)
+        } action: { newValue in
+            frame.wrappedValue = newValue
        }
-        .onPreferenceChange(FramePreferenceKey.self) { newValue in
-            guard frame != newValue else { return }
-            frame = newValue
+    }
+    
+    /// Reads the height of the view and stores it in the `height` binding.
+    /// - Parameters:
+    ///   - height: a `CGFloat` binding
+    func readHeight(_ height: Binding<CGFloat>) -> some View {
+        onGeometryChange(for: CGFloat.self) { geometry in
+            geometry.size.height
+        } action: { newValue in
+            height.wrappedValue = newValue
        }
    }
 }
-
-/// A SwiftUI `PreferenceKey` for `CGRect` values such as a view's frame.
-private struct FramePreferenceKey: PreferenceKey {
-    static var defaultValue: CGRect = .zero
-    
-    static func reduce(value: inout CGRect, nextValue: () -> CGRect) {
-        value = nextValue()
-    }
-}
@@ -198,7 +198,6 @@ class TimelineMediaPreviewItem: NSObject, QLPreviewItem, Identifiable {

 enum TimelineMediaPreviewViewAction {
    case updateCurrentItem(TimelineMediaPreviewItem)
-    case saveCurrentItem
    case showCurrentItemDetails
    case menuAction(TimelineItemMenuAction, item: TimelineMediaPreviewItem)
    case redactConfirmation(item: TimelineMediaPreviewItem)
@@ -59,14 +59,14 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
        switch viewAction {
        case .updateCurrentItem(let item):
            Task { await updateCurrentItem(item) }
-        case .saveCurrentItem:
-            Task { await saveCurrentItem() }
        case .showCurrentItemDetails:
            state.bindings.mediaDetailsItem = state.currentItem
        case .menuAction(let action, let item):
            switch action {
            case .viewInRoomTimeline:
                actionsSubject.send(.viewInRoomTimeline(item.id))
+            case .save:
+                Task { await saveCurrentItem() }
            case .redact:
                state.bindings.redactConfirmationItem = item
            default:
@@ -119,6 +119,9 @@ class TimelineMediaPreviewViewModel: TimelineMediaPreviewViewModelType {
            return
        }
        
+        // Dismiss the details sheet (nicer flow for images/video but _required_ in order to select a file directory).
+        state.bindings.mediaDetailsItem = nil
+        
        do {
            switch state.currentItem.timelineItem {
            case is AudioRoomTimelineItem, is FileRoomTimelineItem:
@@ -12,6 +12,9 @@ struct TimelineMediaPreviewDetailsView: View {
    let item: TimelineMediaPreviewItem
    @ObservedObject var context: TimelineMediaPreviewViewModel.Context
    
+    @State private var sheetHeight: CGFloat = .zero
+    private let topPadding: CGFloat = 19
+    
    var body: some View {
        ScrollView {
            VStack(alignment: .leading, spacing: 0) {
@@ -19,10 +22,12 @@ struct TimelineMediaPreviewDetailsView: View {
                actions
            }
            .frame(maxWidth: .infinity, alignment: .leading)
+            .readHeight($sheetHeight)
        }
-        .presentationDetents([.medium])
+        .scrollBounceBehavior(.basedOnSize)
+        .padding(.top, topPadding) // For the drag indicator
+        .presentationDetents([.height(sheetHeight + topPadding)])
        .presentationDragIndicator(.visible)
-        .padding(.top, 19) // For the drag indicator
        .presentationBackground(.compound.bgCanvasDefault)
        .preferredColorScheme(.dark)
        .sheet(item: $context.redactConfirmationItem) { item in
@@ -95,12 +100,7 @@ struct TimelineMediaPreviewDetailsView: View {
                }
                
                ForEach(actions.actions, id: \.self) { action in
-                    Button(role: action.isDestructive ? .destructive : nil) {
-                        context.send(viewAction: .menuAction(action, item: item))
-                    } label: {
-                        action.label
-                    }
-                    .buttonStyle(.menuSheet)
+                    ActionButton(item: item, action: action, context: context)
                }
                
                if !actions.secondaryActions.isEmpty {
@@ -109,12 +109,7 @@ struct TimelineMediaPreviewDetailsView: View {
                }
                
                ForEach(actions.secondaryActions, id: \.self) { action in
-                    Button(role: action.isDestructive ? .destructive : nil) {
-                        context.send(viewAction: .menuAction(action, item: item))
-                    } label: {
-                        action.label
-                    }
-                    .buttonStyle(.menuSheet)
+                    ActionButton(item: item, action: action, context: context)
                }
            }
        }
@@ -135,6 +130,38 @@ struct TimelineMediaPreviewDetailsView: View {
            }
        }
    }
+    
+    private struct ActionButton: View {
+        let item: TimelineMediaPreviewItem
+        let action: TimelineItemMenuAction
+        let context: TimelineMediaPreviewViewModel.Context
+        
+        var body: some View {
+            if action == .share {
+                if let itemURL = item.fileHandle?.url {
+                    ShareLink(item: itemURL, message: item.caption.map(Text.init)) {
+                        action.label
+                    }
+                    .buttonStyle(.menuSheet)
+                }
+            } else if action == .save {
+                if item.fileHandle?.url != nil {
+                    button
+                }
+            } else {
+                button
+            }
+        }
+        
+        var button: some View {
+            Button(role: action.isDestructive ? .destructive : nil) {
+                context.send(viewAction: .menuAction(action, item: item))
+            } label: {
+                action.label
+            }
+            .buttonStyle(.menuSheet)
+        }
+    }
 }

 // MARK: - Previews
@@ -145,6 +172,7 @@ struct TimelineMediaPreviewDetailsView_Previews: PreviewProvider, TestablePrevie
    @Namespace private static var previewNamespace
    
    static let viewModel = makeViewModel(contentType: .jpeg, isOutgoing: true)
+    static let loadingViewModel = makeViewModel(contentType: .jpeg, isOutgoing: true, isDownloaded: false)
    static let unknownTypeViewModel = makeViewModel()
    static let presentedOnRoomViewModel = makeViewModel(isPresentedOnRoomScreen: true)
    
@@ -156,6 +184,13 @@ struct TimelineMediaPreviewDetailsView_Previews: PreviewProvider, TestablePrevie
                state.currentItemActions?.secondaryActions.contains(.redact) ?? false
            })
        
+        TimelineMediaPreviewDetailsView(item: loadingViewModel.state.currentItem,
+                                        context: loadingViewModel.context)
+            .previewDisplayName("Loading")
+            .snapshotPreferences(expect: loadingViewModel.context.$viewState.map { state in
+                state.currentItemActions?.secondaryActions.contains(.redact) ?? false
+            })
+        
        TimelineMediaPreviewDetailsView(item: unknownTypeViewModel.state.currentItem,
                                        context: unknownTypeViewModel.context)
            .previewDisplayName("Unknown type")
@@ -165,7 +200,10 @@ struct TimelineMediaPreviewDetailsView_Previews: PreviewProvider, TestablePrevie
            .previewDisplayName("Incoming on Room")
    }
    
-    static func makeViewModel(contentType: UTType? = nil, isOutgoing: Bool = false, isPresentedOnRoomScreen: Bool = false) -> TimelineMediaPreviewViewModel {
+    static func makeViewModel(contentType: UTType? = nil,
+                              isOutgoing: Bool = false,
+                              isDownloaded: Bool = true,
+                              isPresentedOnRoomScreen: Bool = false) -> TimelineMediaPreviewViewModel {
        let item = ImageRoomTimelineItem(id: .randomEvent,
                                         timestamp: .mock,
                                         isOutgoing: isOutgoing,
@@ -183,13 +221,20 @@ struct TimelineMediaPreviewDetailsView_Previews: PreviewProvider, TestablePrevie
        let timelineKind = TimelineKind.media(isPresentedOnRoomScreen ? .roomScreen : .mediaFilesScreen)
        let timelineController = MockRoomTimelineController(timelineKind: timelineKind)
        timelineController.timelineItems = [item]
-        return TimelineMediaPreviewViewModel(context: .init(item: item,
-                                                            viewModel: TimelineViewModel.mock(timelineKind: timelineKind,
-                                                                                              timelineController: timelineController),
-                                                            namespace: previewNamespace),
-                                             mediaProvider: MediaProviderMock(configuration: .init()),
-                                             photoLibraryManager: PhotoLibraryManagerMock(.init()),
-                                             userIndicatorController: UserIndicatorControllerMock(),
-                                             appMediator: AppMediatorMock())
+        
+        let viewModel = TimelineMediaPreviewViewModel(context: .init(item: item,
+                                                                     viewModel: TimelineViewModel.mock(timelineKind: timelineKind,
+                                                                                                       timelineController: timelineController),
+                                                                     namespace: previewNamespace),
+                                                      mediaProvider: MediaProviderMock(configuration: .init()),
+                                                      photoLibraryManager: PhotoLibraryManagerMock(.init()),
+                                                      userIndicatorController: UserIndicatorControllerMock(),
+                                                      appMediator: AppMediatorMock())
+        
+        if isDownloaded {
+            viewModel.context.send(viewAction: .updateCurrentItem(viewModel.state.currentItem))
+        }
+        
+        return viewModel
    }
 }
@@ -14,6 +14,9 @@ struct TimelineMediaPreviewRedactConfirmationView: View {
    let item: TimelineMediaPreviewItem
    @ObservedObject var context: TimelineMediaPreviewViewModel.Context
    
+    @State private var sheetHeight: CGFloat = .zero
+    private let topPadding: CGFloat = 19
+    
    var body: some View {
        ScrollView {
            VStack(spacing: 0) {
@@ -21,10 +24,12 @@ struct TimelineMediaPreviewRedactConfirmationView: View {
                preview
                buttons
            }
+            .readHeight($sheetHeight)
        }
-        .presentationDetents([.medium])
+        .scrollBounceBehavior(.basedOnSize)
+        .padding(.top, topPadding) // For the drag indicator
+        .presentationDetents([.height(sheetHeight + topPadding)])
        .presentationDragIndicator(.visible)
-        .padding(.top, 19) // For the drag indicator
        .presentationBackground(.compound.bgCanvasDefault)
        .preferredColorScheme(.dark)
    }
@@ -50,9 +50,7 @@ struct TimelineMediaPreviewScreen: View {
            .overlay { downloadStatusIndicator }
            .toolbar { toolbar }
            .toolbar(toolbarVisibility, for: .navigationBar)
-            .toolbar(toolbarVisibility, for: .bottomBar)
            .toolbarBackground(.visible, for: .navigationBar) // The toolbar's scrollEdgeAppearance isn't aware of the quicklook view 🤷‍♂️
-            .toolbarBackground(.visible, for: .bottomBar)
            .navigationBarTitleDisplayMode(.inline)
            .safeAreaInset(edge: .bottom, spacing: 0) { caption }
    }
@@ -112,6 +110,7 @@ struct TimelineMediaPreviewScreen: View {
                .padding(16)
                .background {
                    BlurEffectView(style: .systemChromeMaterial) // Darkest material available, matches the bottom bar when content is beneath.
+                        .ignoresSafeArea()
                }
                .transition(.move(edge: .bottom).combined(with: .opacity))
        }
@@ -137,11 +136,6 @@ struct TimelineMediaPreviewScreen: View {
            }
            .tint(.compound.textActionPrimary)
        }
-        
-        ToolbarItem(placement: .bottomBar) {
-            bottomBarContent
-                .tint(.compound.textActionPrimary)
-        }
    }
    
    private var toolbarHeader: some View {
@@ -155,22 +149,6 @@ struct TimelineMediaPreviewScreen: View {
                .textCase(.uppercase)
        }
    }
-    
-    private var bottomBarContent: some View {
-        HStack(spacing: 8) {
-            if let url = currentItem.fileHandle?.url {
-                ShareLink(item: url, subject: nil, message: currentItem.caption.map(Text.init)) {
-                    CompoundIcon(\.shareIos)
-                }
-                
-                Spacer()
-                
-                Button { context.send(viewAction: .saveCurrentItem) } label: {
-                    CompoundIcon(\.downloadIos)
-                }
-            }
-        }
-    }
 }

 // MARK: - QuickLook
@@ -19,19 +19,7 @@ struct MediaEventsTimelineScreen: View {
            .background(.compound.bgCanvasDefault)
            // Doesn't play well with the transformed scrollView
            .toolbarBackground(.visible, for: .navigationBar)
-            .toolbar {
-                ToolbarItem(placement: .principal) {
-                    Picker("", selection: $context.screenMode) {
-                        Text(L10n.screenMediaBrowserListModeMedia)
-                            .padding()
-                            .tag(MediaEventsTimelineScreenMode.media)
-                        Text(L10n.screenMediaBrowserListModeFiles)
-                            .padding()
-                            .tag(MediaEventsTimelineScreenMode.files)
-                    }
-                    .pickerStyle(.segmented)
-                }
-            }
+            .toolbar { toolbar }
            .environmentObject(context.viewState.activeTimelineContextProvider())
            .environment(\.timelineContext, context.viewState.activeTimelineContextProvider())
            .onChange(of: context.screenMode) { _, _ in
@@ -206,6 +194,27 @@ struct MediaEventsTimelineScreen: View {
        }
    }
    
+    @ToolbarContentBuilder
+    private var toolbar: some ToolbarContent {
+        ToolbarItem(placement: .principal) {
+            Picker("", selection: $context.screenMode) {
+                Text(L10n.screenMediaBrowserListModeMedia)
+                    .padding()
+                    .tag(MediaEventsTimelineScreenMode.media)
+                Text(L10n.screenMediaBrowserListModeFiles)
+                    .padding()
+                    .tag(MediaEventsTimelineScreenMode.files)
+            }
+            .pickerStyle(.segmented)
+            .frame(idealWidth: .greatestFiniteMagnitude)
+        }
+        
+        ToolbarItem(placement: .primaryAction) {
+            // Reserve the space trailing space to match the back button.
+            CompoundIcon(\.search).hidden()
+        }
+    }
+    
    func tappedItem(_ item: RoomTimelineItemViewState) {
        context.send(viewAction: .tappedItem(item: item, namespace: zoomTransition))
    }
@@ -180,6 +180,10 @@ class TimelineInteractionHandler {
            analyticsService.trackInteraction(name: .PinnedMessageListViewTimeline)
            guard let eventID = itemID.eventID else { return }
            actionsSubject.send(.viewInRoomTimeline(eventID: eventID))
+        case .share:
+            break // Handled inline in the media preview screen with a ShareLink.
+        case .save:
+            break // Handled inline in the media preview screen.
        }
        
        if action.switchToDefaultComposer {
@@ -74,6 +74,8 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
    case pin
    case unpin
    case viewInRoomTimeline
+    case share
+    case save
    
    var id: Self { self }
    
@@ -128,7 +130,7 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
    
    var canAppearInMediaDetails: Bool {
        switch self {
-        case .viewInRoomTimeline, .redact:
+        case .viewInRoomTimeline, .share, .save, .redact:
            true
        default:
            false
@@ -178,6 +180,10 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
            Label(L10n.actionUnpin, icon: \.unpin)
        case .viewInRoomTimeline:
            Label(L10n.actionViewInTimeline, icon: \.visibilityOn)
+        case .share:
+            Label(L10n.actionShare, icon: \.shareIos)
+        case .save:
+            Label(L10n.actionSave, icon: \.downloadIos)
        }
    }
 }
@@ -107,6 +107,8 @@ struct TimelineItemMenuActionProvider {
            actions = actions.filter(\.canAppearInPinnedEventsTimeline)
            secondaryActions = secondaryActions.filter(\.canAppearInPinnedEventsTimeline)
        case .media:
+            actions.append(.share)
+            actions.append(.save)
            actions = actions.filter(\.canAppearInMediaDetails)
            secondaryActions = secondaryActions.filter(\.canAppearInMediaDetails)
        case .live, .detached: