From a6a8f53b30577aa333f1e312ffd90c9928ecc8ca Mon Sep 17 00:00:00 2001 From: Doug <6060466+pixlwave@users.noreply.github.com> Date: Fri, 29 Nov 2024 16:23:20 +0000 Subject: [PATCH] Add a warning to the media caption composer. (#3574) We can now remove the feature flag. --- .../en.lproj/Localizable.strings | 5 +- .../Sources/Application/AppSettings.swift | 4 + .../RoomFlowCoordinator.swift | 2 +- ElementX/Sources/Generated/Strings.swift | 2 + .../MediaUploadPreviewScreenCoordinator.swift | 4 +- .../MediaUploadPreviewScreenModels.swift | 4 +- .../MediaUploadPreviewScreenViewModel.swift | 4 +- .../View/MediaUploadPreviewScreen.swift | 94 ++++++++++++++----- ..._mediaUploadPreviewScreen-iPad-en-GB.1.png | 4 +- ...eviewScreen-iPad-en-GB.Caption-warning.png | 3 + ...mediaUploadPreviewScreen-iPad-pseudo.1.png | 4 +- ...viewScreen-iPad-pseudo.Caption-warning.png | 3 + ...aUploadPreviewScreen-iPhone-16-en-GB.1.png | 4 +- ...Screen-iPhone-16-en-GB.Caption-warning.png | 3 + ...UploadPreviewScreen-iPhone-16-pseudo.1.png | 4 +- ...creen-iPhone-16-pseudo.Caption-warning.png | 3 + ...diaUploadPreviewScreenViewModelTests.swift | 2 +- 17 files changed, 106 insertions(+), 43 deletions(-) create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.Caption-warning.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.Caption-warning.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.Caption-warning.png create mode 100644 PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.Caption-warning.png diff --git a/ElementX/Resources/Localizations/en.lproj/Localizable.strings b/ElementX/Resources/Localizations/en.lproj/Localizable.strings index f81ac7c6d..050bd1032 100644 --- a/ElementX/Resources/Localizations/en.lproj/Localizable.strings +++ b/ElementX/Resources/Localizations/en.lproj/Localizable.strings @@ -387,6 +387,9 @@ "screen_knock_requests_list_empty_state_description" = "When somebody will ask to join the room, you’ll be able to see their request here."; "screen_knock_requests_list_empty_state_title" = "No pending request to join"; "screen_knock_requests_list_title" = "Requests to join"; +"screen_media_upload_preview_caption_warning" = "Captions might not be visible to people using older apps."; +"screen_media_upload_preview_error_failed_processing" = "Failed processing media to upload, please try again."; +"screen_media_upload_preview_error_failed_sending" = "Failed uploading media, please try again."; "screen_pinned_timeline_empty_state_description" = "Press on a message and choose “%1$@” to include here."; "screen_pinned_timeline_empty_state_headline" = "Pin important messages so that they can be easily discovered"; "screen_reset_encryption_password_error" = "An unknown error happened. Please check your account password is correct and try again."; @@ -584,8 +587,6 @@ "screen_login_title" = "Welcome back!"; "screen_login_title_with_homeserver" = "Sign in to %1$@"; "screen_media_picker_error_failed_selection" = "Failed selecting media, please try again."; -"screen_media_upload_preview_error_failed_processing" = "Failed processing media to upload, please try again."; -"screen_media_upload_preview_error_failed_sending" = "Failed uploading media, please try again."; "screen_migration_message" = "This is a one time process, thanks for waiting."; "screen_migration_title" = "Setting up your account."; "screen_notification_optin_subtitle" = "You can change your settings later."; diff --git a/ElementX/Sources/Application/AppSettings.swift b/ElementX/Sources/Application/AppSettings.swift index eff6e295a..8da94ab93 100644 --- a/ElementX/Sources/Application/AppSettings.swift +++ b/ElementX/Sources/Application/AppSettings.swift @@ -240,6 +240,10 @@ final class AppSettings { @UserPreference(key: UserDefaultsKeys.optimizeMediaUploads, defaultValue: true, storageType: .userDefaults(store)) var optimizeMediaUploads + + /// Whether or not to show a warning on the media caption composer so the user knows + /// that captions might not be visible to users who are using other Matrix clients. + let shouldShowMediaCaptionWarning = true // MARK: - Element Call diff --git a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift index 3a58bdfac..fddb7fea1 100644 --- a/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift +++ b/ElementX/Sources/FlowCoordinators/RoomFlowCoordinator.swift @@ -997,7 +997,7 @@ class RoomFlowCoordinator: FlowCoordinatorProtocol { mediaUploadingPreprocessor: MediaUploadingPreprocessor(appSettings: appSettings), title: url.lastPathComponent, url: url, - createMediaCaptionsEnabled: appSettings.createMediaCaptionsEnabled) + shouldShowCaptionWarning: appSettings.shouldShowMediaCaptionWarning) let mediaUploadPreviewScreenCoordinator = MediaUploadPreviewScreenCoordinator(parameters: parameters) diff --git a/ElementX/Sources/Generated/Strings.swift b/ElementX/Sources/Generated/Strings.swift index 36ae30edd..4e3258ecc 100644 --- a/ElementX/Sources/Generated/Strings.swift +++ b/ElementX/Sources/Generated/Strings.swift @@ -1358,6 +1358,8 @@ internal enum L10n { } /// Failed selecting media, please try again. internal static var screenMediaPickerErrorFailedSelection: String { return L10n.tr("Localizable", "screen_media_picker_error_failed_selection") } + /// Captions might not be visible to people using older apps. + internal static var screenMediaUploadPreviewCaptionWarning: String { return L10n.tr("Localizable", "screen_media_upload_preview_caption_warning") } /// Failed processing media to upload, please try again. internal static var screenMediaUploadPreviewErrorFailedProcessing: String { return L10n.tr("Localizable", "screen_media_upload_preview_error_failed_processing") } /// Failed uploading media, please try again. diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift index 78bf65104..4d2610314 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenCoordinator.swift @@ -14,7 +14,7 @@ struct MediaUploadPreviewScreenCoordinatorParameters { let mediaUploadingPreprocessor: MediaUploadingPreprocessor let title: String? let url: URL - let createMediaCaptionsEnabled: Bool + let shouldShowCaptionWarning: Bool } enum MediaUploadPreviewScreenCoordinatorAction { @@ -36,7 +36,7 @@ final class MediaUploadPreviewScreenCoordinator: CoordinatorProtocol { mediaUploadingPreprocessor: parameters.mediaUploadingPreprocessor, title: parameters.title, url: parameters.url, - createMediaCaptionsEnabled: parameters.createMediaCaptionsEnabled) + shouldShowCaptionWarning: parameters.shouldShowCaptionWarning) } func start() { diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenModels.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenModels.swift index ecd05d345..b3568c35c 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenModels.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenModels.swift @@ -14,7 +14,7 @@ enum MediaUploadPreviewScreenViewModelAction { struct MediaUploadPreviewScreenViewState: BindableState { let url: URL let title: String? - let showMediaCaptionComposer: Bool + let shouldShowCaptionWarning: Bool var shouldDisableInteraction = false var bindings = MediaUploadPreviewScreenBindings() @@ -23,6 +23,8 @@ struct MediaUploadPreviewScreenViewState: BindableState { struct MediaUploadPreviewScreenBindings: BindableState { var caption = NSAttributedString() var presendCallback: (() -> Void)? + + var isPresentingMediaCaptionWarning = false } enum MediaUploadPreviewScreenViewAction { diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift index e4d8decd4..f76a025c8 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/MediaUploadPreviewScreenViewModel.swift @@ -33,13 +33,13 @@ class MediaUploadPreviewScreenViewModel: MediaUploadPreviewScreenViewModelType, mediaUploadingPreprocessor: MediaUploadingPreprocessor, title: String?, url: URL, - createMediaCaptionsEnabled: Bool) { + shouldShowCaptionWarning: Bool) { self.userIndicatorController = userIndicatorController self.roomProxy = roomProxy self.mediaUploadingPreprocessor = mediaUploadingPreprocessor self.url = url - super.init(initialViewState: MediaUploadPreviewScreenViewState(url: url, title: title, showMediaCaptionComposer: createMediaCaptionsEnabled)) + super.init(initialViewState: MediaUploadPreviewScreenViewState(url: url, title: title, shouldShowCaptionWarning: shouldShowCaptionWarning)) } override func process(viewAction: MediaUploadPreviewScreenViewAction) { diff --git a/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift b/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift index 2b4e722ab..17ba7c9be 100644 --- a/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift +++ b/ElementX/Sources/Screens/MediaUploadPreviewScreen/View/MediaUploadPreviewScreen.swift @@ -14,26 +14,27 @@ struct MediaUploadPreviewScreen: View { @ObservedObject var context: MediaUploadPreviewScreenViewModel.Context - var title: String { ProcessInfo.processInfo.isiOSAppOnMac ? context.viewState.title ?? "" : "" } - var colorSchemeOverride: ColorScheme { ProcessInfo.processInfo.isiOSAppOnMac ? colorScheme : .dark } + @State private var captionWarningFrame: CGRect = .zero + + private var title: String { ProcessInfo.processInfo.isiOSAppOnMac ? context.viewState.title ?? "" : "" } + private var colorSchemeOverride: ColorScheme { ProcessInfo.processInfo.isiOSAppOnMac ? colorScheme : .dark } var body: some View { mainContent .id(context.viewState.url) .ignoresSafeArea(edges: [.horizontal]) .safeAreaInset(edge: .bottom, spacing: 0) { - if context.viewState.showMediaCaptionComposer { - composer - .padding(.horizontal, 12) - .padding(.vertical, 16) - .background() // Don't use compound so we match the QLPreviewController. - } + composer + .padding(.horizontal, 12) + .padding(.vertical, 16) + .background() // Don't use compound so we match the QLPreviewController. } .navigationTitle(title) .navigationBarTitleDisplayMode(.inline) .toolbar { toolbar } .disabled(context.viewState.shouldDisableInteraction) .interactiveDismissDisabled() + .presentationBackground(.background) // Fix a bug introduced by the caption warning. .preferredColorScheme(colorSchemeOverride) } @@ -52,13 +53,19 @@ struct MediaUploadPreviewScreen: View { private var composer: some View { HStack(spacing: 12) { - MessageComposerTextField(placeholder: L10n.richTextEditorComposerCaptionPlaceholder, - text: $context.caption, - presendCallback: $context.presendCallback, - maxHeight: ComposerConstant.maxHeight, - keyHandler: { _ in }, - pasteHandler: { _ in }) - .messageComposerStyle() + HStack(spacing: 6) { + MessageComposerTextField(placeholder: L10n.richTextEditorComposerCaptionPlaceholder, + text: $context.caption, + presendCallback: $context.presendCallback, + maxHeight: ComposerConstant.maxHeight, + keyHandler: { _ in }, + pasteHandler: { _ in }) + + if context.viewState.shouldShowCaptionWarning { + captionWarningButton + } + } + .messageComposerStyle() SendButton { context.send(viewAction: .send) @@ -66,6 +73,47 @@ struct MediaUploadPreviewScreen: View { } } + private var captionWarningButton: some View { + Button { + context.isPresentingMediaCaptionWarning = true + } label: { + CompoundIcon(\.infoSolid, size: .xSmall, relativeTo: .compound.bodyLG) + } + .tint(.compound.iconCriticalPrimary) + .popover(isPresented: $context.isPresentingMediaCaptionWarning, arrowEdge: .bottom) { + captionWarningContent + .presentationDetents([.height(captionWarningFrame.height)]) + .presentationDragIndicator(.visible) + .padding(.top, 19) // For the drag indicator + .presentationBackground(.compound.bgCanvasDefault) + .preferredColorScheme(colorSchemeOverride) + } + } + + var captionWarningContent: some View { + VStack(spacing: 0) { + VStack(spacing: 16) { + BigIcon(icon: \.infoSolid, style: .alertSolid) + + Text(L10n.screenMediaUploadPreviewCaptionWarning) + .font(.compound.bodyMD) + .foregroundStyle(.compound.textPrimary) + .multilineTextAlignment(.center) + .fixedSize(horizontal: false, vertical: true) + } + .padding(24) + .padding(.bottom, 8) + + Button(L10n.actionOk) { + context.isPresentingMediaCaptionWarning = false + } + .buttonStyle(.compound(.secondary)) + .padding(.horizontal, 16) + .padding(.bottom, 16) + } + .readFrame($captionWarningFrame) + } + @ToolbarContentBuilder private var toolbar: some ToolbarContent { ToolbarItem(placement: .cancellationAction) { @@ -76,16 +124,6 @@ struct MediaUploadPreviewScreen: View { // follow the dark colour scheme on devices running with dark mode disabled. .tint(.compound.textActionPrimary) } - - if !context.viewState.showMediaCaptionComposer { - ToolbarItem(placement: .confirmationAction) { - Button { context.send(viewAction: .send) } label: { - Text(L10n.actionSend) - } - // Same fix as above (this button is temporary anyway). - .tint(.compound.textActionPrimary) - } - } } } @@ -169,10 +207,14 @@ struct MediaUploadPreviewScreen_Previews: PreviewProvider, TestablePreview { mediaUploadingPreprocessor: MediaUploadingPreprocessor(appSettings: ServiceLocator.shared.settings), title: "App Icon.png", url: snapshotURL, - createMediaCaptionsEnabled: true) + shouldShowCaptionWarning: true) static var previews: some View { NavigationStack { MediaUploadPreviewScreen(context: viewModel.context) } + + MediaUploadPreviewScreen(context: viewModel.context) + .captionWarningContent + .previewDisplayName("Caption warning") } } diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.1.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.1.png index a218dc80e..d9424a300 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.1.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:885fb08cfe301dd996fa6f1b276afa7a74f6534d692ab313ec7d64bfcb970981 -size 91980 +oid sha256:79c2656c58dde4df7af57a9ce1561d33c83772a8f15e88ea57fe80f8c14b61b5 +size 92652 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.Caption-warning.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.Caption-warning.png new file mode 100644 index 000000000..3881a9edb --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-en-GB.Caption-warning.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:4443b88f50798db3ddf789a3a1daf69b45158e799801e51e0a6604840af9b811 +size 84527 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.1.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.1.png index 687ed4884..377fe1b2b 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.1.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:ab99e9ac3233f55705f96cc0c8f1480351f0765b7b36d1e90e3262758b2ede9a -size 93223 +oid sha256:f53cf59f991c8958e8ae06759f05cf41b39ec6d1b402fcc6123edbd54e863409 +size 93837 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.Caption-warning.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.Caption-warning.png new file mode 100644 index 000000000..de24f7d43 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPad-pseudo.Caption-warning.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:babfab486bcb62122404c32f1268e3f6ce486c7ac1942de40e3fd754260c2e9a +size 89371 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.1.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.1.png index 748902c2b..b77d39bca 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.1.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:3cd02932f70d0c1336b295d94ca31694f569fea56b36b8cc29c448284406e8dd -size 50622 +oid sha256:1212f7afafd62561d7a90852e077667a042d4e86bd9b572904e0c89146e8d8b9 +size 51188 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.Caption-warning.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.Caption-warning.png new file mode 100644 index 000000000..1b0c00337 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-en-GB.Caption-warning.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:74e2863ec160d47570f9d8c512d5dc4c6afa50ce0f2fb6cd7520a4bfb287f4dc +size 43826 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.1.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.1.png index 960476010..3bf2bb188 100644 --- a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.1.png +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.1.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:26109f9334be8c7452fd98c15158bea787df6b87f560f8ee6b6aea46dc6a1647 -size 51627 +oid sha256:f6ac82c8457a7771ae7c30049ea4250f434285e4ae15ad52eefc935bd38e37eb +size 51942 diff --git a/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.Caption-warning.png b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.Caption-warning.png new file mode 100644 index 000000000..8d73d82d7 --- /dev/null +++ b/PreviewTests/Sources/__Snapshots__/PreviewTests/test_mediaUploadPreviewScreen-iPhone-16-pseudo.Caption-warning.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:d1ea4d6009691a8079ae6d788dda3d9ea3c28d45451ad31278dbe8dfc399e2f7 +size 54743 diff --git a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift index 9a47185ae..7fe56df9d 100644 --- a/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift +++ b/UnitTests/Sources/MediaUploadPreviewScreenViewModelTests.swift @@ -126,7 +126,7 @@ class MediaUploadPreviewScreenViewModelTests: XCTestCase { mediaUploadingPreprocessor: MediaUploadingPreprocessor(appSettings: ServiceLocator.shared.settings), title: "Some File", url: url, - createMediaCaptionsEnabled: true) + shouldShowCaptionWarning: true) } private func verifyCaption(_ caption: String?, expectedCaption: String?) -> Result {