From 888a61ace1008db5bb8349ddd90dbaa21950b83d Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Fri, 20 Dec 2024 10:34:22 +0000 Subject: [PATCH] Media upload tweaks (#3643) * Focus the media caption composer if a hardware keyboard is available Not worrying about live connections, this screen is short lived. * Fix a bug where you could hit the send button multiple times while waiting for the media to be converted. * Begin processing media as soon as the media upload screen is shown. --- .../MediaUploadPreviewScreenCoordinator.swift | 4 +++ .../MediaUploadPreviewScreenViewModel.swift | 34 +++++++++++-------- ...UploadPreviewScreenViewModelProtocol.swift | 3 ++ .../View/MediaUploadPreviewScreen.swift | 28 +++++++++++++-- ...diaUploadPreviewScreenViewModelTests.swift | 5 +++ 5 files changed, 58 insertions(+), 16 deletions(-) diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift index 4d2610314..1de5470e8 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift @@ -52,6 +52,10 @@ final class MediaUploadPreviewScreenCoordinator: CoordinatorProtocol { .store(in: &cancellables) } + func stop() { + viewModel.stopProcessing() + } + func toPresentable() -> AnyView { AnyView(MediaUploadPreviewScreen(context: viewModel.context)) } diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift index f3741f6e0..d7dcc6640 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift @@ -16,11 +16,9 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType, private let roomProxy: JoinedRoomProxyProtocol private let mediaUploadingPreprocessor: MediaUploadingPreprocessor private let url: URL - private var requestHandle: SendAttachmentJoinHandleProtocol? { - didSet { - state.shouldDisableInteraction = requestHandle != nil - } - } + + private var processingTask: Task, Never> + private var requestHandle: SendAttachmentJoinHandleProtocol? private var actionsSubject: PassthroughSubject = .init() @@ -39,6 +37,9 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType, self.mediaUploadingPreprocessor = mediaUploadingPreprocessor self.url = url + // Start processing the media whilst the user is reviewing it/adding a caption. + processingTask = Task { await mediaUploadingPreprocessor.processMedia(at: url) } + super.init(initialViewState: MediaUploadPreviewScreenViewState(url: url, title: title, shouldShowCaptionWarning: shouldShowCaptionWarning)) } @@ -48,10 +49,10 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType, switch viewAction { case .send: + startLoading() + Task { - startLoading() - - switch await mediaUploadingPreprocessor.processMedia(at: url) { + switch await processingTask.value { case .success(let mediaInfo): switch await sendAttachment(mediaInfo: mediaInfo, caption: caption) { case .success: @@ -75,6 +76,10 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType, } } + func stopProcessing() { + processingTask.cancel() + } + // MARK: - Private private func sendAttachment(mediaInfo: MediaInfo, caption: String?) async -> Result { @@ -111,16 +116,17 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType, private static let loadingIndicatorIdentifier = "\(MediaUploadPreviewScreenViewModel.self)-Loading" private func startLoading() { - userIndicatorController.submitIndicator( - UserIndicator(id: Self.loadingIndicatorIdentifier, - type: .modal(progress: .indeterminate, interactiveDismissDisabled: false, allowsInteraction: true), - title: L10n.commonSending, - persistent: true) - ) + userIndicatorController.submitIndicator(UserIndicator(id: Self.loadingIndicatorIdentifier, + type: .modal(progress: .indeterminate, interactiveDismissDisabled: false, allowsInteraction: true), + title: L10n.commonSending, + persistent: true)) + + state.shouldDisableInteraction = true } private func stopLoading() { userIndicatorController.retractIndicatorWithId(Self.loadingIndicatorIdentifier) + state.shouldDisableInteraction = false requestHandle = nil } diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModelProtocol.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModelProtocol.swift index 71d9f1b70..32fe8b648 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModelProtocol.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModelProtocol.swift @@ -11,4 +11,7 @@ import Combine protocol MediaUploadPreviewScreenViewModelProtocol { var actions: AnyPublisher { get } var context: MediaUploadPreviewScreenViewModelType.Context { get } + + /// Stops any ongoing media processing tasks. + func stopProcessing() } diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift index 17ba7c9be..b9230cea4 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift @@ -6,6 +6,7 @@ // import Compound +import GameController import QuickLook import SwiftUI @@ -15,6 +16,7 @@ struct MediaUploadPreviewScreen: View { @ObservedObject var context: MediaUploadPreviewScreenViewModel.Context @State private var captionWarningFrame: CGRect = .zero + @FocusState private var isComposerFocussed private var title: String { ProcessInfo.processInfo.isiOSAppOnMac ? context.viewState.title ?? "" : "" } private var colorSchemeOverride: ColorScheme { ProcessInfo.processInfo.isiOSAppOnMac ? colorScheme : .dark } @@ -36,6 +38,7 @@ struct MediaUploadPreviewScreen: View { .interactiveDismissDisabled() .presentationBackground(.background) // Fix a bug introduced by the caption warning. .preferredColorScheme(colorSchemeOverride) + .onAppear(perform: focusComposerIfHardwareKeyboardConnected) } @ViewBuilder @@ -58,8 +61,8 @@ struct MediaUploadPreviewScreen: View { text: $context.caption, presendCallback: $context.presendCallback, maxHeight: ComposerConstant.maxHeight, - keyHandler: { _ in }, - pasteHandler: { _ in }) + keyHandler: handleKeyPress) { _ in } + .focused($isComposerFocussed) if context.viewState.shouldShowCaptionWarning { captionWarningButton @@ -125,6 +128,27 @@ struct MediaUploadPreviewScreen: View { .tint(.compound.textActionPrimary) } } + + private func handleKeyPress(_ key: UIKeyboardHIDUsage) { + switch key { + case .keyboardReturnOrEnter: + context.send(viewAction: .send) + case .keyboardEscape: + context.send(viewAction: .cancel) + default: + break + } + } + + private func focusComposerIfHardwareKeyboardConnected() { + // The simulator always detects the hardware keyboard as connected + #if !targetEnvironment(simulator) + if GCKeyboard.coalesced != nil { + MXLog.info("Hardware keyboard is connected") + isComposerFocussed = true + } + #endif + } } private struct PreviewView: UIViewControllerRepresentable { diff --git a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift index 81218b031..a0c174b07 100644 --- a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift +++ b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift @@ -138,8 +138,13 @@ class MediaUploadPreviewScreenViewModelTests: XCTestCase { } private func send() async throws { + XCTAssertFalse(context.viewState.shouldDisableInteraction, "Attempting to send when interaction is disabled.") + let deferred = deferFulfillment(viewModel.actions) { $0 == .dismiss } context.send(viewAction: .send) + + XCTAssertTrue(context.viewState.shouldDisableInteraction, "The interaction should be disabled while sending.") + try await deferred.fulfill() } }