Attach Screenshot in Bug Report Screen

ElementX/Resources/Localizations/en.lproj/Untranslated.strings

@@ -82,7 +82,9 @@
 "bug_report_screen_description" = "Please describe the bug. What did you do? What did you expect to happen? What actually happened. Please go into as much detail as you can.";
 "bug_report_screen_include_logs" = "Send logs to help";
 "bug_report_screen_logs_description" = "To check things work as intended, logs will be sent with your message. These will be private. To just send your message, turn off this setting.";
-
+"bug_report_screen_attach_screenshot" = "Attach Screenshot";
+"bug_report_screen_edit_screenshot" = "Edit Screenshot";
+"bug_report_screen_sending" = "Sending...";
 
 // Session verification
 /*

ElementX/Sources/Generated/Strings+Untranslated.swift

@@ -16,12 +16,18 @@ extension ElementL10n {
   public static let actionConfirm = ElementL10n.tr("Untranslated", "action_confirm")
   /// Match
   public static let actionMatch = ElementL10n.tr("Untranslated", "action_match")
+  /// Attach Screenshot
+  public static let bugReportScreenAttachScreenshot = ElementL10n.tr("Untranslated", "bug_report_screen_attach_screenshot")
   /// Please describe the bug. What did you do? What did you expect to happen? What actually happened. Please go into as much detail as you can.
   public static let bugReportScreenDescription = ElementL10n.tr("Untranslated", "bug_report_screen_description")
+  /// Edit Screenshot
+  public static let bugReportScreenEditScreenshot = ElementL10n.tr("Untranslated", "bug_report_screen_edit_screenshot")
   /// Send logs to help
   public static let bugReportScreenIncludeLogs = ElementL10n.tr("Untranslated", "bug_report_screen_include_logs")
   /// To check things work as intended, logs will be sent with your message. These will be private. To just send your message, turn off this setting.
   public static let bugReportScreenLogsDescription = ElementL10n.tr("Untranslated", "bug_report_screen_logs_description")
+  /// Sending...
+  public static let bugReportScreenSending = ElementL10n.tr("Untranslated", "bug_report_screen_sending")
   /// Report a bug
   public static let bugReportScreenTitle = ElementL10n.tr("Untranslated", "bug_report_screen_title")
   /// %@ iOS

ElementX/Sources/Other/AccessibilityIdentifiers.swift

@@ -32,6 +32,7 @@ struct A11yIdentifiers {
         let screenshot = "bug_report-screenshot"
         let removeScreenshot = "bug_report-remove_screenshot"
         let send = "bug_report-send"
+        let attachScreenshot = "bug-report-attach_screenshot"
     }
 
     struct ChangeServer {

ElementX/Sources/Other/SwiftUI/Views/SettingsDefaultRow.swift

@@ -26,24 +26,19 @@ struct SettingsDefaultRow: View {
 
     // MARK: Private
 
-    @ScaledMetric private var menuIconSize = 30.0
     private let listRowInsets = EdgeInsets(top: 8, leading: 16, bottom: 8, trailing: 16)
 
     // MARK: Views
 
     var body: some View {
         Button(action: action) {
             HStack(spacing: 16) {
-                image
-                    .foregroundColor(.element.systemGray)
-                    .padding(4)
-                    .background(Color.element.systemGray6)
-                    .clipShape(RoundedRectangle(cornerRadius: 8))
-                    .frame(width: menuIconSize, height: menuIconSize)
-
-                Text(title)
-                    .font(.element.body)
-                    .foregroundColor(.element.primaryContent)
+                Label(title: {
+                    Text(title)
+                }, icon: {
+                    image
+                })
+                .labelStyle(SettingsRowLabelStyle())
 
                 Spacer()
 

ElementX/Sources/Other/SwiftUI/Views/SettingsRowLabelStyle.swift

@@ -0,0 +1,37 @@
+//
+// Copyright 2023 New Vector Ltd
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+//
+
+import SwiftUI
+
+struct SettingsRowLabelStyle: LabelStyle {
+    @ScaledMetric private var menuIconSize = 30.0
+
+    var alignment: VerticalAlignment = .firstTextBaseline
+
+    func makeBody(configuration: Configuration) -> some View {
+        HStack(alignment: alignment, spacing: 16) {
+            configuration.icon
+                .foregroundColor(.element.secondaryContent)
+                .padding(4)
+                .background(Color.element.formBackground)
+                .clipShape(RoundedRectangle(cornerRadius: 8))
+                .frame(width: menuIconSize, height: menuIconSize)
+            configuration.title
+                .font(.element.body)
+                .foregroundColor(.element.primaryContent)
+        }
+    }
+}

ElementX/Sources/Screens/BugReport/BugReportCoordinator.swift

@@ -52,7 +52,7 @@ final class BugReportCoordinator: CoordinatorProtocol {
             case .cancel:
                 self.completion?(.cancel)
             case let .submitStarted(progressTracker):
-                self.startLoading(progressPublisher: progressTracker)
+                self.startLoading(label: ElementL10n.bugReportScreenSending, progressPublisher: progressTracker)
             case .submitFinished:
                 self.stopLoading()
                 self.completion?(.finish)

ElementX/Sources/Screens/BugReport/BugReportModels.swift

@@ -39,4 +39,5 @@ enum BugReportViewAction {
     case cancel
     case submit
     case removeScreenshot
+    case attachScreenshot(UIImage)
 }

ElementX/Sources/Screens/BugReport/BugReportViewModel.swift

@@ -43,6 +43,8 @@ class BugReportViewModel: BugReportViewModelType, BugReportViewModelProtocol {
             await submitBugReport()
         case .removeScreenshot:
             state.screenshot = nil
+        case let .attachScreenshot(image):
+            state.screenshot = image
         }
     }
 
@@ -52,11 +54,18 @@ class BugReportViewModel: BugReportViewModelType, BugReportViewModelProtocol {
         let progressTracker = ProgressTracker()
         callback?(.submitStarted(progressTracker: progressTracker))
         do {
+            var files: [URL] = []
+            if let screenshot = context.viewState.screenshot {
+                let imageURL = URL.temporaryDirectory.appendingPathComponent("Screenshot.png")
+                let pngData = screenshot.pngData()
+                try pngData?.write(to: imageURL)
+                files.append(imageURL)
+            }
             let bugReport = BugReport(text: context.reportText,
                                       includeLogs: context.sendingLogsEnabled,
                                       includeCrashLog: true,
                                       githubLabels: [],
-                                      files: [])
+                                      files: files)
             let result = try await bugReportService.submitBugReport(bugReport,
                                                                     progressListener: progressTracker)
             MXLog.info("SubmitBugReport succeeded, result: \(result.reportUrl)")

ElementX/Sources/Screens/BugReport/View/BugReportScreen.swift

@@ -14,6 +14,7 @@
 // limitations under the License.
 //
 
+import PhotosUI
 import SwiftUI
 
 struct BugReportScreen: View {
@@ -22,6 +23,8 @@ struct BugReportScreen: View {
     private var horizontalPadding: CGFloat {
         horizontalSizeClass == .regular ? 50 : 16
     }
+
+    @State private var selectedScreenshot: PhotosPickerItem?
 
     @ObservedObject var context: BugReportViewModel.Context
 
@@ -37,14 +40,24 @@ struct BugReportScreen: View {
         .navigationBarTitleDisplayMode(.inline)
         .toolbar { toolbar }
         .interactiveDismissDisabled()
+        .onChange(of: selectedScreenshot) { newItem in
+            Task {
+                guard let data = try? await newItem?.loadTransferable(type: Data.self),
+                      let image = UIImage(data: data)
+                else {
+                    return
+                }
+                context.send(viewAction: .attachScreenshot(image))
+            }
+        }
     }
 
     /// The main content of the view to be shown in a scroll view.
     var mainContent: some View {
         VStack(alignment: .leading, spacing: 24) {
             descriptionTextEditor
+            attachScreenshot
             sendLogsToggle
-            screenshot
         }
     }
 
@@ -75,7 +88,7 @@ struct BugReportScreen: View {
                 .stroke(Color.element.quaternaryContent)
         }
         .frame(maxWidth: .infinity)
-        .frame(height: 300)
+        .frame(height: 220)
         .font(.body)
     }
 
@@ -95,24 +108,40 @@ struct BugReportScreen: View {
                 .padding(.horizontal, -8)
         }
     }
-    
+
     @ViewBuilder
-    private var screenshot: some View {
-        if let screenshot = context.viewState.screenshot {
-            ZStack(alignment: .topTrailing) {
-                Image(uiImage: screenshot)
-                    .resizable()
-                    .scaledToFit()
-                    .frame(width: 100)
-                    .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.screenshot)
-                Button { context.send(viewAction: .removeScreenshot) } label: {
-                    Image(uiImage: Asset.Images.closeCircle.image)
+    private var attachScreenshot: some View {
+        VStack(alignment: .leading, spacing: 16) {
+            PhotosPicker(selection: $selectedScreenshot,
+                         matching: .screenshots,
+                         photoLibrary: .shared()) {
+                HStack(spacing: 16) {
+                    Label(context.viewState.screenshot == nil ? ElementL10n.bugReportScreenAttachScreenshot : ElementL10n.bugReportScreenEditScreenshot, systemImage: "camera")
+                        .labelStyle(SettingsRowLabelStyle())
+                    Spacer()
                 }
-                .offset(x: 10, y: -10)
-                .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.removeScreenshot)
             }
-            .padding(.vertical, 16)
             .padding(.horizontal, 16)
+            .padding(.vertical, 11)
+            .background(RoundedRectangle(cornerRadius: 14).fill(Color.element.formRowBackground))
+            .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.attachScreenshot)
+            if let screenshot = context.viewState.screenshot {
+                ZStack(alignment: .topTrailing) {
+                    Image(uiImage: screenshot)
+                        .resizable()
+                        .scaledToFit()
+                        .frame(width: 100)
+                        .cornerRadius(4)
+                        .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.screenshot)
+                    Button { context.send(viewAction: .removeScreenshot) } label: {
+                        Image(Asset.Images.closeCircle.name)
+                    }
+                    .offset(x: 10, y: -10)
+                    .accessibilityIdentifier(A11yIdentifiers.bugReportScreen.removeScreenshot)
+                }
+                .padding(.vertical, 16)
+                .padding(.horizontal, 16)
+            }
         }
     }
 
@@ -140,10 +169,19 @@ struct BugReportScreen: View {
 
 struct BugReport_Previews: PreviewProvider {
     static let viewModel = BugReportViewModel(bugReportService: MockBugReportService(),
-                                              screenshot: Asset.Images.appLogo.image,
+                                              screenshot: nil,
                                               isModallyPresented: false)
 
     static var previews: some View {
-        BugReportScreen(context: viewModel.context)
+        Group {
+            BugReportScreen(context: BugReportViewModel(bugReportService: MockBugReportService(),
+                                                        screenshot: nil,
+                                                        isModallyPresented: false).context)
+                .previewDisplayName("Without Screenshot")
+            BugReportScreen(context: BugReportViewModel(bugReportService: MockBugReportService(),
+                                                        screenshot: Asset.Images.appLogo.image,
+                                                        isModallyPresented: false).context)
+                .previewDisplayName("With Screenshot")
+        }
     }
 }

ElementX/Sources/Screens/RoomDetails/View/RoomDetailsScreen.swift

@@ -17,7 +17,6 @@
 import SwiftUI
 
 struct RoomDetailsScreen: View {
-    @ScaledMetric private var menuIconSize = 30.0
     private let listRowInsets = EdgeInsets(top: 8, leading: 16, bottom: 8, trailing: 16)
 
     @ObservedObject var context: RoomDetailsViewModel.Context
@@ -102,16 +101,8 @@ struct RoomDetailsScreen: View {
                 context.send(viewAction: .processTapPeople)
             } label: {
                 HStack {
-                    Image(systemName: "person")
-                        .foregroundColor(.element.secondaryContent)
-                        .padding(4)
-                        .background(Color.element.formBackground)
-                        .cornerRadius(8)
-                        .frame(width: menuIconSize, height: menuIconSize)
-
-                    Text(ElementL10n.bottomActionPeople)
-                        .foregroundColor(.element.primaryContent)
-                        .font(.body)
+                    Label(ElementL10n.bottomActionPeople, systemImage: "person")
+                        .labelStyle(SettingsRowLabelStyle())
 
                     Spacer()
 
@@ -138,22 +129,17 @@ struct RoomDetailsScreen: View {
     private var securitySection: some View {
         Section {
             HStack(alignment: .top) {
-                Image(systemName: "lock.shield")
-                    .foregroundColor(.element.secondaryContent)
-                    .padding(4)
-                    .background(Color.element.formBackground)
-                    .cornerRadius(8)
-                    .frame(width: menuIconSize, height: menuIconSize)
-
-                VStack(alignment: .leading, spacing: 2) {
-                    Text(ElementL10n.encryptionEnabled)
-                        .foregroundColor(.element.primaryContent)
-                        .font(.element.body)
-
-                    Text(ElementL10n.encryptionEnabledTileDescription)
-                        .foregroundColor(.element.secondaryContent)
-                        .font(.element.footnote)
-                }
+                Label(title: {
+                    VStack(alignment: .leading, spacing: 2) {
+                        Text(ElementL10n.encryptionEnabled)
+                        Text(ElementL10n.encryptionEnabledTileDescription)
+                            .foregroundColor(.element.secondaryContent)
+                            .font(.element.footnote)
+                    }
+                }, icon: {
+                    Image(systemName: "lock.shield")
+                })
+                .labelStyle(SettingsRowLabelStyle(alignment: .top))
 
                 Spacer()
 

ElementX/Sources/Screens/Settings/View/SettingsScreen.swift

@@ -20,7 +20,6 @@ struct SettingsScreen: View {
     @State private var showingLogoutConfirmation = false
 
     @ScaledMetric private var avatarSize = AvatarSize.user(on: .settings).value
-    @ScaledMetric private var menuIconSize = 30.0
 
     private let listRowInsets = EdgeInsets(top: 8, leading: 16, bottom: 8, trailing: 16)
 

ElementX/Sources/Services/BugReport/BugReportService.swift

@@ -189,7 +189,7 @@ class BugReportService: NSObject, BugReportServiceProtocol {
         var zippedFiles: [URL] = []
 
         for url in filesToCompress {
-            let zippedFileURL = URL(fileURLWithPath: NSTemporaryDirectory())
+            let zippedFileURL = URL.temporaryDirectory
                 .appendingPathComponent(url.lastPathComponent)
 
             //  remove old zipped file if exists

UITests/Sources/BugReportUITests.swift

@@ -33,7 +33,7 @@ class BugReportUITests: XCTestCase {
         let sendingLogsToggle = app.switches[A11yIdentifiers.bugReportScreen.sendLogs]
         XCTAssert(sendingLogsToggle.exists)
         XCTAssertFalse(sendingLogsToggle.isOn)
-        
+
         app.assertScreenshot(.bugReport, step: 1)
     }