A bunch of random tweaks. (#3317)

- Fix warning on single case enum. - Remove unused subjects. - Fix a lint warning. - Remove unused success value. - Fix warning about position of try await. - Add a note about the common tracing configuration. - Show an indicator when resolving a send failure fails. - Make sure the whole row is clickable in the GlobalSearchScreen.
element-hq · Sep 23, 2024 · e4736b1 · e4736b1
1 parent d48214a
commit e4736b1
Show file tree

Hide file tree

Showing 13 changed files with 48 additions and 35 deletions.
diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift
@@ -1366,7 +1366,8 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol {
     private func presentResolveSendFailure(failure: TimelineItemSendFailure.VerifiedUser, itemID: TimelineItemIdentifier) {
         let coordinator = ResolveVerifiedUserSendFailureScreenCoordinator(parameters: .init(failure: failure,
                                                                                             itemID: itemID,
-                                                                                            roomProxy: roomProxy))
+                                                                                            roomProxy: roomProxy,
+                                                                                            userIndicatorController: userIndicatorController))
         coordinator.actionsPublisher.sink { [weak self] action in
             guard let self else { return }
 

diff --git a/ElementX/Sources/Other/Logging/TracingConfiguration.swift b/ElementX/Sources/Other/Logging/TracingConfiguration.swift
@@ -67,7 +67,7 @@ struct TracingConfiguration {
     }
 
     static let targets: OrderedDictionary<Target, LogLevel> = [
-        .common: .info,
+        .common: .info, // Never set this lower than info - 3rd-party crates may start logging user data.
         .elementx: .info,
         .hyper: .warn,
         .matrix_sdk_ffi: .info,
@@ -106,7 +106,7 @@ struct TracingConfiguration {
 
         let overrides = Self.targets.keys.reduce(into: [Target: LogLevel]()) { partialResult, target in
             // Keep the defaults here
-            let ignoredTargets: [Target] = [.common,
+            let ignoredTargets: [Target] = [.common, // Never remove common from the ignored targets (see above for more info).
                                             .hyper,
                                             .matrix_sdk_ffi,
                                             .matrix_sdk_oidc,

diff --git a/ElementX/Sources/Screens/CallScreen/CallScreenModels.swift b/ElementX/Sources/Screens/CallScreen/CallScreenModels.swift
@@ -28,7 +28,7 @@ struct CallScreenViewState: BindableState {
 struct Bindings {
     var javaScriptMessageHandler: ((Any) -> Void)?
     var javaScriptEvaluator: ((String) async throws -> Any)?
-    var requestPictureInPictureHandler: (() async -> Result<AVPictureInPictureController, CallScreenError>)?
+    var requestPictureInPictureHandler: (() async -> Result<Void, CallScreenError>)?
 
     var alertInfo: AlertInfo<UUID>?
 }

diff --git a/ElementX/Sources/Screens/CallScreen/CallScreenViewModel.swift b/ElementX/Sources/Screens/CallScreen/CallScreenViewModel.swift
@@ -189,7 +189,7 @@ class CallScreenViewModel: CallScreenViewModelType, CallScreenViewModelProtocol
         }
 
         switch await requestPictureInPictureHandler() {
-        case .success(let controller):
+        case .success:
             actionsSubject.send(.pictureInPictureStarted)
         case .failure:
             actionsSubject.send(.dismiss)

diff --git a/ElementX/Sources/Screens/CallScreen/View/CallScreen.swift b/ElementX/Sources/Screens/CallScreen/View/CallScreen.swift
@@ -200,15 +200,15 @@ private struct CallView: UIViewRepresentable {
 
         // MARK: - Picture in Picture
 
-        func requestPictureInPicture() async -> Result<AVPictureInPictureController, CallScreenError> {
+        func requestPictureInPicture() async -> Result<Void, CallScreenError> {
             guard let pictureInPictureController,
                   pictureInPictureController.isPictureInPicturePossible,
                   case .success(true) = await webViewCanEnterPictureInPicture() else {
                 return .failure(.pictureInPictureNotAvailable)
             }
 
             pictureInPictureController.startPictureInPicture()
-            return .success(pictureInPictureController)
+            return .success(())
         }
 
         func stopPictureInPicture() {

diff --git a/ElementX/Sources/Screens/GlobalSearchScreen/View/GlobalSearchScreen.swift b/ElementX/Sources/Screens/GlobalSearchScreen/View/GlobalSearchScreen.swift
@@ -23,6 +23,7 @@ struct GlobalSearchScreen: View {
                     GlobalSearchScreenListRow(room: room, context: context)
                         .listRowBackground(backgroundColor(for: room))
                         .listRowInsets(.init())
+                        .contentShape(.rect)
                         .onTapGesture {
                             context.send(viewAction: .select(roomID: room.id))
                         }

diff --git a/...esolveVerifiedUserSendFailureScreen/ResolveVerifiedUserSendFailureScreenCoordinator.swift b/...esolveVerifiedUserSendFailureScreen/ResolveVerifiedUserSendFailureScreenCoordinator.swift
@@ -21,6 +21,7 @@ struct ResolveVerifiedUserSendFailureScreenCoordinatorParameters {
     let failure: TimelineItemSendFailure.VerifiedUser
     let itemID: TimelineItemIdentifier
     let roomProxy: JoinedRoomProxyProtocol
+    let userIndicatorController: UserIndicatorControllerProtocol
 }
 
 enum ResolveVerifiedUserSendFailureScreenCoordinatorAction {
@@ -43,7 +44,8 @@ final class ResolveVerifiedUserSendFailureScreenCoordinator: CoordinatorProtocol
 
         viewModel = ResolveVerifiedUserSendFailureScreenViewModel(failure: parameters.failure,
                                                                   itemID: parameters.itemID,
-                                                                  roomProxy: parameters.roomProxy)
+                                                                  roomProxy: parameters.roomProxy,
+                                                                  userIndicatorController: parameters.userIndicatorController)
     }
 
     func start() {

diff --git a/.../ResolveVerifiedUserSendFailureScreen/ResolveVerifiedUserSendFailureScreenViewModel.swift b/.../ResolveVerifiedUserSendFailureScreen/ResolveVerifiedUserSendFailureScreenViewModel.swift
@@ -26,12 +26,17 @@ class ResolveVerifiedUserSendFailureScreenViewModel: ResolveVerifiedUserSendFail
     private let roomProxy: JoinedRoomProxyProtocol
     private var members: [String: RoomMemberProxyProtocol]
 
+    private let userIndicatorController: UserIndicatorControllerProtocol
+
     private let actionsSubject: PassthroughSubject<ResolveVerifiedUserSendFailureScreenViewModelAction, Never> = .init()
     var actionsPublisher: AnyPublisher<ResolveVerifiedUserSendFailureScreenViewModelAction, Never> {
         actionsSubject.eraseToAnyPublisher()
     }
 
-    init(failure: TimelineItemSendFailure.VerifiedUser, itemID: TimelineItemIdentifier, roomProxy: JoinedRoomProxyProtocol) {
+    init(failure: TimelineItemSendFailure.VerifiedUser,
+         itemID: TimelineItemIdentifier,
+         roomProxy: JoinedRoomProxyProtocol,
+         userIndicatorController: UserIndicatorControllerProtocol) {
         iterator = switch failure {
         case .hasUnsignedDevice(let devices): UnsignedDeviceFailureIterator(devices: devices)
         case .changedIdentity(let users): ChangedIdentityFailureIterator(users: users)
@@ -40,6 +45,7 @@ class ResolveVerifiedUserSendFailureScreenViewModel: ResolveVerifiedUserSendFail
         self.failure = failure
         self.itemID = itemID
         self.roomProxy = roomProxy
+        self.userIndicatorController = userIndicatorController
 
         members = Dictionary(uniqueKeysWithValues: roomProxy.membersPublisher.value.map { ($0.userID, $0) })
 
@@ -77,7 +83,8 @@ class ResolveVerifiedUserSendFailureScreenViewModel: ResolveVerifiedUserSendFail
         }
 
         if case let .failure(error) = result {
-            #warning("Show the error?")
+            MXLog.error("Failed to resolve send failure: \(error)")
+            showFailureIndicator()
             return
         }
 
@@ -94,10 +101,20 @@ class ResolveVerifiedUserSendFailureScreenViewModel: ResolveVerifiedUserSendFail
         switch await roomProxy.resend(itemID: itemID) {
         case .success:
             actionsSubject.send(.dismiss)
-        case .failure(let failure):
-            #warning("Show the error?")
+        case .failure(let error):
+            MXLog.error("Failed to resend message: \(error)")
+            showFailureIndicator()
         }
     }
+
+    private static let failureIndicatorIdentifier = "\(ResolveVerifiedUserSendFailureScreenViewModel.self)-Failure"
+
+    private func showFailureIndicator() {
+        ServiceLocator.shared.userIndicatorController.submitIndicator(UserIndicator(id: Self.failureIndicatorIdentifier,
+                                                                                    type: .toast,
+                                                                                    title: L10n.errorUnknown,
+                                                                                    iconName: "xmark"))
+    }
 }
 
 // MARK: - Iterators

diff --git a/...eens/ResolveVerifiedUserSendFailureScreen/View/ResolveVerifiedUserSendFailureScreen.swift b/...eens/ResolveVerifiedUserSendFailureScreen/View/ResolveVerifiedUserSendFailureScreen.swift
@@ -95,14 +95,16 @@ struct ResolveVerifiedUserSendFailureScreen_Previews: PreviewProvider, TestableP
     static func makeViewModel(failure: TimelineItemSendFailure.VerifiedUser) -> ResolveVerifiedUserSendFailureScreenViewModel {
         ResolveVerifiedUserSendFailureScreenViewModel(failure: failure,
                                                       itemID: .random,
-                                                      roomProxy: JoinedRoomProxyMock(.init()))
+                                                      roomProxy: JoinedRoomProxyMock(.init()),
+                                                      userIndicatorController: UserIndicatorControllerMock())
     }
 }
 
 struct ResolveVerifiedUserSendFailureScreenSheet_Previews: PreviewProvider {
     static let viewModel = ResolveVerifiedUserSendFailureScreenViewModel(failure: .changedIdentity(users: ["@alice:matrix.org"]),
                                                                          itemID: .random,
-                                                                         roomProxy: JoinedRoomProxyMock(.init()))
+                                                                         roomProxy: JoinedRoomProxyMock(.init()),
+                                                                         userIndicatorController: UserIndicatorControllerMock())
 
     static var previews: some View {
         Text("Hello")

diff --git a/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift b/ElementX/Sources/Screens/Timeline/TimelineViewModel.swift
@@ -565,10 +565,8 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
     }
 
     private func slashCommand(message: String) -> SlashCommand? {
-        for command in SlashCommand.allCases {
-            if message.starts(with: command.rawValue) {
-                return command
-            }
+        for command in SlashCommand.allCases where message.starts(with: command.rawValue) {
+            return command
         }
         return nil
     }

diff --git a/ElementX/Sources/Services/Timeline/TimelineProxy.swift b/ElementX/Sources/Services/Timeline/TimelineProxy.swift
@@ -130,9 +130,9 @@ final class TimelineProxy: TimelineProxyProtocol {
         subject.send(.paginating)
 
         do {
-            let timelineEndReached = try await switch direction {
-            case .backwards: timeline.paginateBackwards(numEvents: requestSize)
-            case .forwards: timeline.focusedPaginateForwards(numEvents: requestSize)
+            let timelineEndReached = switch direction {
+            case .backwards: try await timeline.paginateBackwards(numEvents: requestSize)
+            case .forwards: try await timeline.focusedPaginateForwards(numEvents: requestSize)
             }
             MXLog.info("Finished paginating \(direction.rawValue)")
 

diff --git a/UnitTests/Sources/ResolveVerifiedUserSendFailureScreenViewModelTests.swift b/UnitTests/Sources/ResolveVerifiedUserSendFailureScreenViewModelTests.swift
@@ -60,7 +60,10 @@ class ResolveVerifiedUserSendFailureScreenViewModelTests: XCTestCase {
     // MARK: Helpers
 
     private func makeViewModel(with failure: TimelineItemSendFailure.VerifiedUser) -> ResolveVerifiedUserSendFailureScreenViewModel {
-        ResolveVerifiedUserSendFailureScreenViewModel(failure: failure, itemID: .random, roomProxy: roomProxy)
+        ResolveVerifiedUserSendFailureScreenViewModel(failure: failure,
+                                                      itemID: .random,
+                                                      roomProxy: roomProxy,
+                                                      userIndicatorController: UserIndicatorControllerMock())
     }
 
     private func verifyResolving(userIDs: [String], assertStrings: Bool = true) async throws {
@@ -73,7 +76,7 @@ class ResolveVerifiedUserSendFailureScreenViewModelTests: XCTestCase {
             }
 
             // When resolving the first failure.
-            let deferredFailure = deferFailure(viewModel.actionsPublisher, timeout: 1) { $0.isDismiss }
+            let deferredFailure = deferFailure(viewModel.actionsPublisher, timeout: 1) { $0 == .dismiss }
             context.send(viewAction: .resolveAndResend)
 
             // Then the sheet should remain open for the next failure.
@@ -88,7 +91,7 @@ class ResolveVerifiedUserSendFailureScreenViewModelTests: XCTestCase {
         }
 
         // When resolving the final failure.
-        let deferred = deferFulfillment(viewModel.actionsPublisher) { $0.isDismiss }
+        let deferred = deferFulfillment(viewModel.actionsPublisher) { $0 == .dismiss }
         context.send(viewAction: .resolveAndResend)
 
         // Then the sheet should be dismissed.
@@ -110,12 +113,3 @@ class ResolveVerifiedUserSendFailureScreenViewModelTests: XCTestCase {
         XCTAssertTrue(context.viewState.subtitle.contains(displayName))
     }
 }
-
-private extension ResolveVerifiedUserSendFailureScreenViewModelAction {
-    var isDismiss: Bool {
-        switch self {
-        case .dismiss: true
-        default: false
-        }
-    }
-}
diff --git a/UnitTests/Sources/RoomScreenViewModelTests.swift b/UnitTests/Sources/RoomScreenViewModelTests.swift
@@ -102,8 +102,6 @@ class RoomScreenViewModelTests: XCTestCase {
 
     func testPinnedEventsBannerSelection() async throws {
         ServiceLocator.shared.settings.pinningEnabled = true
-        let timelineSubject = PassthroughSubject<TimelineProxyProtocol, Never>()
-        let updateSubject = PassthroughSubject<JoinedRoomProxyAction, Never>()
         let roomProxyMock = JoinedRoomProxyMock(.init())
         // setup a way to inject the mock of the pinned events timeline
         let pinnedTimelineMock = TimelineProxyMock()