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.

ElementX/Sources/Other/SwiftUI/Layout/ViewFrameReader.swift

@@ -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))
-        }
-        .onPreferenceChange(FramePreferenceKey.self) { newValue in
-            guard frame != newValue else { return }
-            frame = newValue
+        onGeometryChange(for: CGRect.self) { geometry in
+            geometry.frame(in: coordinateSpace)
+        } action: { newValue in
+            frame.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()
+    /// 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
+        }
     }
 }

ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewModels.swift

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

ElementX/Sources/Screens/FilePreviewScreen/TimelineMediaPreviewViewModel.swift

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

ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewDetailsView.swift

@@ -12,17 +12,22 @@ 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) {
                 details
                 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
     }
 }

ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewRedactConfirmationView.swift

@@ -14,17 +14,22 @@ 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) {
                 header
                 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)
     }

ElementX/Sources/Screens/FilePreviewScreen/View/TimelineMediaPreviewScreen.swift

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

ElementX/Sources/Screens/MediaEventsTimelineScreen/View/MediaEventsTimelineScreen.swift

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

ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift

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