Make the attachment menu an actual menu. (#2199)

There was a bug with the sheet/popover when overriding the colour scheme where the button renders in light mode when the app is in dark mode.

ElementX/Sources/Screens/ComposerToolbar/ComposerToolbarModels.swift

@@ -32,11 +32,7 @@ enum ComposerToolbarVoiceMessageAction {
 
 enum ComposerToolbarViewModelAction {
     case sendMessage(plain: String, html: String?, mode: RoomScreenComposerMode, intentionalMentions: IntentionalMentions)
-    case displayCameraPicker
-    case displayMediaPicker
-    case displayDocumentPicker
-    case displayLocationPicker
-    case displayNewPollForm
+    case attach(ComposerAttachmentType)
 
     case handlePasteOrDrop(provider: NSItemProvider)
 
@@ -51,11 +47,7 @@ enum ComposerToolbarViewAction {
     case sendMessage
     case cancelReply
     case cancelEdit
-    case displayCameraPicker
-    case displayMediaPicker
-    case displayDocumentPicker
-    case displayLocationPicker
-    case displayNewPollForm
+    case attach(ComposerAttachmentType)
     case handlePasteOrDrop(provider: NSItemProvider)
     case enableTextFormatting
     case composerAction(action: ComposerAction)
@@ -64,6 +56,14 @@ enum ComposerToolbarViewAction {
     case voiceMessage(ComposerToolbarVoiceMessageAction)
 }
 
+enum ComposerAttachmentType {
+    case camera
+    case photoLibrary
+    case file
+    case location
+    case poll
+}
+
 struct ComposerToolbarViewState: BindableState {
     var composerMode: RoomScreenComposerMode = .default
     var composerEmpty = true
@@ -116,14 +116,6 @@ struct ComposerToolbarViewStateBindings {
     var composerExpanded = false
     var formatItems: [FormatItem] = .init()
     var alertInfo: AlertInfo<UUID>?
-
-    var showAttachmentPopover = false {
-        didSet {
-            if showAttachmentPopover {
-                composerFocused = false
-            }
-        }
-    }
 }
 
 /// An item in the toolbar

ElementX/Sources/Screens/ComposerToolbar/ComposerToolbarViewModel.swift

@@ -120,16 +120,9 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
         case .cancelEdit:
             set(mode: .default)
             set(text: "")
-        case .displayCameraPicker:
-            actionsSubject.send(.displayCameraPicker)
-        case .displayMediaPicker:
-            actionsSubject.send(.displayMediaPicker)
-        case .displayDocumentPicker:
-            actionsSubject.send(.displayDocumentPicker)
-        case .displayLocationPicker:
-            actionsSubject.send(.displayLocationPicker)
-        case .displayNewPollForm:
-            actionsSubject.send(.displayNewPollForm)
+        case .attach(let attachment):
+            state.bindings.composerFocused = false
+            actionsSubject.send(.attach(attachment))
         case .handlePasteOrDrop(let provider):
             actionsSubject.send(.handlePasteOrDrop(provider: provider))
         case .enableTextFormatting:

ElementX/Sources/Screens/ComposerToolbar/View/RoomAttachmentPicker.swift

@@ -20,13 +20,12 @@ import WysiwygComposer
 
 struct RoomAttachmentPicker: View {
     @ObservedObject var context: ComposerToolbarViewModel.Context
-    @Environment(\.isPresented) var isPresented
-
-    @State private var sheetContentFrame: CGRect = .zero
     
     var body: some View {
-        Button {
-            context.showAttachmentPopover = true
+        // Use a menu instead of the popover/sheet shown in Figma because overriding the colour scheme
+        // results in a rendering bug on 17.1: https://github.com/vector-im/element-x-ios/issues/2157
+        Menu {
+            menuContent
         } label: {
             CompoundIcon(asset: Asset.Images.composerAttachment, size: .custom(30), relativeTo: .title)
                 .scaledPadding(7, relativeTo: .title)
@@ -34,21 +33,12 @@ struct RoomAttachmentPicker: View {
         .buttonStyle(RoomAttachmentPickerButtonStyle())
         .accessibilityLabel(L10n.actionAddToTimeline)
         .accessibilityIdentifier(A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions)
-        .popover(isPresented: $context.showAttachmentPopover) {
-            menuContent
-                .padding(.top, isPresented ? 20 : 0)
-                .readFrame($sheetContentFrame)
-                .presentationDetents([.height(sheetContentFrame.height)])
-                .presentationBackground(.compound.bgCanvasDefault)
-                .presentationDragIndicator(.visible)
-        }
     }
     
     var menuContent: some View {
         VStack(alignment: .leading, spacing: 0.0) {
             Button {
-                context.showAttachmentPopover = false
-                context.send(viewAction: .displayMediaPicker)
+                context.send(viewAction: .attach(.photoLibrary))
             } label: {
                 Label(L10n.screenRoomAttachmentSourceGallery, icon: \.image)
                     .labelStyle(.menuSheet)
@@ -56,8 +46,7 @@ struct RoomAttachmentPicker: View {
             .accessibilityIdentifier(A11yIdentifiers.roomScreen.attachmentPickerPhotoLibrary)
             
             Button {
-                context.showAttachmentPopover = false
-                context.send(viewAction: .displayDocumentPicker)
+                context.send(viewAction: .attach(.file))
             } label: {
                 Label(L10n.screenRoomAttachmentSourceFiles, iconAsset: Asset.Images.attachment)
                     .labelStyle(.menuSheet)
@@ -65,8 +54,7 @@ struct RoomAttachmentPicker: View {
             .accessibilityIdentifier(A11yIdentifiers.roomScreen.attachmentPickerDocuments)
             
             Button {
-                context.showAttachmentPopover = false
-                context.send(viewAction: .displayCameraPicker)
+                context.send(viewAction: .attach(.camera))
             } label: {
                 Label(L10n.screenRoomAttachmentSourceCamera, iconAsset: Asset.Images.takePhoto)
                     .labelStyle(.menuSheet)
@@ -74,8 +62,7 @@ struct RoomAttachmentPicker: View {
             .accessibilityIdentifier(A11yIdentifiers.roomScreen.attachmentPickerCamera)
 
             Button {
-                context.showAttachmentPopover = false
-                context.send(viewAction: .displayLocationPicker)
+                context.send(viewAction: .attach(.location))
             } label: {
                 Label(L10n.screenRoomAttachmentSourceLocation, iconAsset: Asset.Images.addLocation)
                     .labelStyle(.menuSheet)
@@ -83,8 +70,7 @@ struct RoomAttachmentPicker: View {
             .accessibilityIdentifier(A11yIdentifiers.roomScreen.attachmentPickerLocation)
 
             Button {
-                context.showAttachmentPopover = false
-                context.send(viewAction: .displayNewPollForm)
+                context.send(viewAction: .attach(.poll))
             } label: {
                 Label(L10n.screenRoomAttachmentSourcePoll, iconAsset: Asset.Images.polls)
                     .labelStyle(.menuSheet)
@@ -93,7 +79,6 @@ struct RoomAttachmentPicker: View {
 
             if ServiceLocator.shared.settings.richTextEditorEnabled {
                 Button {
-                    context.showAttachmentPopover = false
                     context.send(viewAction: .enableTextFormatting)
                 } label: {
                     Label(L10n.screenRoomAttachmentTextFormatting, iconAsset: Asset.Images.textFormat)
@@ -109,10 +94,6 @@ private struct RoomAttachmentPickerButtonStyle: ButtonStyle {
     func makeBody(configuration: Configuration) -> some View {
         configuration.label
             .foregroundStyle(configuration.isPressed ? .compound.bgActionPrimaryPressed : .compound.bgActionPrimaryRest)
-            // Disable animations to fix a bug when the system is in Light mode but the app in Dark mode. For some
-            // reason the animation causes a glitch with sheet's colour scheme when there are presentation detents.
-            // https://github.com/vector-im/element-x-ios/issues/2157
-            .animation(.noAnimation, value: configuration.isPressed)
     }
 }
 

ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift

@@ -180,16 +180,8 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
                                          mode: mode,
                                          intentionalMentions: intentionalMentions)
             }
-        case .displayCameraPicker:
-            actionsSubject.send(.displayCameraPicker)
-        case .displayMediaPicker:
-            actionsSubject.send(.displayMediaPicker)
-        case .displayDocumentPicker:
-            actionsSubject.send(.displayDocumentPicker)
-        case .displayLocationPicker:
-            actionsSubject.send(.displayLocationPicker)
-        case .displayNewPollForm:
-            actionsSubject.send(.displayPollForm(mode: .new))
+        case .attach(let attachment):
+            attach(attachment)
         case .handlePasteOrDrop(let provider):
             roomScreenInteractionHandler.handlePasteOrDrop(provider)
         case .composerModeChanged(mode: let mode):
@@ -203,6 +195,21 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
     
     // MARK: - Private
     
+    private func attach(_ attachment: ComposerAttachmentType) {
+        switch attachment {
+        case .camera:
+            actionsSubject.send(.displayCameraPicker)
+        case .photoLibrary:
+            actionsSubject.send(.displayMediaPicker)
+        case .file:
+            actionsSubject.send(.displayDocumentPicker)
+        case .location:
+            actionsSubject.send(.displayLocationPicker)
+        case .poll:
+            actionsSubject.send(.displayPollForm(mode: .new))
+        }
+    }
+    
     private func processPollAction(_ action: RoomScreenViewPollAction) {
         switch action {
         case let .selectOption(pollStartID, optionID):

IntegrationTests/Sources/UserFlowTests.swift

@@ -59,13 +59,13 @@ class UserFlowTests: XCTestCase {
         for identifier in [A11yIdentifiers.roomScreen.attachmentPickerPhotoLibrary,
                            A11yIdentifiers.roomScreen.attachmentPickerDocuments,
                            A11yIdentifiers.roomScreen.attachmentPickerLocation] {
-            tapOnButton(A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions)
+            tapOnMenu(A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions)
             tapOnButton(identifier)
             tapOnButton("Cancel")
         }
                 
         // Open attachments picker
-        tapOnButton(A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions)
+        tapOnMenu(A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions)
 
         // Open photo library picker
         tapOnButton(A11yIdentifiers.roomScreen.attachmentPickerPhotoLibrary)
@@ -169,6 +169,12 @@ class UserFlowTests: XCTestCase {
         button.tap()
     }
     
+    private func tapOnMenu(_ identifier: String) {
+        let button = app.buttons[identifier]
+        XCTAssertTrue(button.waitForExistence(timeout: 10.0))
+        button.forceTap()
+    }
+    
     /// Taps on a back button that the system configured with a label but no identifier.
     ///
     /// When there are multiple buttons with the same label in the hierarchy, all the buttons we created

UITests/Sources/UserSessionScreenTests.swift

@@ -29,7 +29,7 @@ class UserSessionScreenTests: XCTestCase {
         try await Task.sleep(for: .seconds(1))
         try await app.assertScreenshot(.userSessionScreen, step: 2)
 
-        app.buttons[A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions].tap()
+        app.buttons[A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions].forceTap()
         try await app.assertScreenshot(.userSessionScreen, step: 3)
     }
 
@@ -52,7 +52,7 @@ class UserSessionScreenTests: XCTestCase {
         XCTAssert(app.staticTexts[firstRoomName].waitForExistence(timeout: 5.0))
         try await Task.sleep(for: .seconds(1))
 
-        app.buttons[A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions].tap()
+        app.buttons[A11yIdentifiers.roomScreen.composerToolbar.openComposeOptions].forceTap()
         try await app.assertScreenshot(.userSessionScreenRTE, step: 1)
 
         app.buttons[A11yIdentifiers.roomScreen.attachmentPickerTextFormatting].tap()