Skip to content

Commit

Permalink
Add a warning to the media caption composer. (#3574)
Browse files Browse the repository at this point in the history
We can now remove the feature flag.
  • Loading branch information
pixlwave authored Nov 29, 2024
1 parent 8d69099 commit 34f1c61
Show file tree
Hide file tree
Showing 17 changed files with 106 additions and 43 deletions.
5 changes: 3 additions & 2 deletions ElementX/Resources/Localizations/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -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.";
Expand Down Expand Up @@ -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.";
Expand Down
4 changes: 4 additions & 0 deletions ElementX/Sources/Application/AppSettings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
2 changes: 2 additions & 0 deletions ElementX/Sources/Generated/Strings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ struct MediaUploadPreviewScreenCoordinatorParameters {
let mediaUploadingPreprocessor: MediaUploadingPreprocessor
let title: String?
let url: URL
let createMediaCaptionsEnabled: Bool
let shouldShowCaptionWarning: Bool
}

enum MediaUploadPreviewScreenCoordinatorAction {
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand All @@ -23,6 +23,8 @@ struct MediaUploadPreviewScreenViewState: BindableState {
struct MediaUploadPreviewScreenBindings: BindableState {
var caption = NSAttributedString()
var presendCallback: (() -> Void)?

var isPresentingMediaCaptionWarning = false
}

enum MediaUploadPreviewScreenViewAction {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
}

Expand All @@ -52,20 +53,67 @@ 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)
}
}
}

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) {
Expand All @@ -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)
}
}
}
}

Expand Down Expand Up @@ -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")
}
}
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Original file line number Diff line number Diff line change
Expand Up @@ -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<Void, TimelineProxyError> {
Expand Down

0 comments on commit 34f1c61

Please sign in to comment.