Skip to content

Commit

Permalink
Add support for adding/editing/removing media captions. (#3547)
Browse files Browse the repository at this point in the history
* Add support for editing media captions.

* Add composer snapshots.

* PR comment.
  • Loading branch information
pixlwave authored Nov 21, 2024
1 parent c081e53 commit 7e1476d
Show file tree
Hide file tree
Showing 21 changed files with 178 additions and 61 deletions.
5 changes: 5 additions & 0 deletions ElementX/Resources/Localizations/en.lproj/Localizable.strings
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
"a11y_voice_message_record" = "Record voice message.";
"a11y_voice_message_stop_recording" = "Stop recording";
"action_accept" = "Accept";
"action_add_caption" = "Add caption";
"action_add_to_timeline" = "Add to timeline";
"action_back" = "Back";
"action_call" = "Call";
Expand All @@ -47,6 +48,7 @@
"action_discard" = "Discard";
"action_done" = "Done";
"action_edit" = "Edit";
"action_edit_caption" = "Edit caption";
"action_edit_poll" = "Edit poll";
"action_enable" = "Enable";
"action_end_poll" = "End poll";
Expand Down Expand Up @@ -81,6 +83,7 @@
"action_react" = "React";
"action_reject" = "Reject";
"action_remove" = "Remove";
"action_remove_caption" = "Remove caption";
"action_reply" = "Reply";
"action_reply_in_thread" = "Reply in thread";
"action_report_bug" = "Report bug";
Expand Down Expand Up @@ -119,6 +122,7 @@
"banner_set_up_recovery_title" = "Set up recovery to protect your account";
"common_about" = "About";
"common_acceptable_use_policy" = "Acceptable use policy";
"common_adding_caption" = "Adding caption";
"common_advanced_settings" = "Advanced settings";
"common_analytics" = "Analytics";
"common_appearance" = "Appearance";
Expand All @@ -137,6 +141,7 @@
"common_direct_chat" = "Direct chat";
"common_edited_suffix" = "(edited)";
"common_editing" = "Editing";
"common_editing_caption" = "Editing caption";
"common_emote" = "* %1$@ %2$@";
"common_encryption" = "Encryption";
"common_encryption_enabled" = "Encryption enabled";
Expand Down
10 changes: 10 additions & 0 deletions ElementX/Sources/Generated/Strings.swift
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,8 @@ internal enum L10n {
internal static var a11yVoiceMessageStopRecording: String { return L10n.tr("Localizable", "a11y_voice_message_stop_recording") }
/// Accept
internal static var actionAccept: String { return L10n.tr("Localizable", "action_accept") }
/// Add caption
internal static var actionAddCaption: String { return L10n.tr("Localizable", "action_add_caption") }
/// Add to timeline
internal static var actionAddToTimeline: String { return L10n.tr("Localizable", "action_add_to_timeline") }
/// Back
Expand Down Expand Up @@ -126,6 +128,8 @@ internal enum L10n {
internal static var actionDone: String { return L10n.tr("Localizable", "action_done") }
/// Edit
internal static var actionEdit: String { return L10n.tr("Localizable", "action_edit") }
/// Edit caption
internal static var actionEditCaption: String { return L10n.tr("Localizable", "action_edit_caption") }
/// Edit poll
internal static var actionEditPoll: String { return L10n.tr("Localizable", "action_edit_poll") }
/// Enable
Expand Down Expand Up @@ -198,6 +202,8 @@ internal enum L10n {
internal static var actionReject: String { return L10n.tr("Localizable", "action_reject") }
/// Remove
internal static var actionRemove: String { return L10n.tr("Localizable", "action_remove") }
/// Remove caption
internal static var actionRemoveCaption: String { return L10n.tr("Localizable", "action_remove_caption") }
/// Reply
internal static var actionReply: String { return L10n.tr("Localizable", "action_reply") }
/// Reply in thread
Expand Down Expand Up @@ -276,6 +282,8 @@ internal enum L10n {
internal static var commonAbout: String { return L10n.tr("Localizable", "common_about") }
/// Acceptable use policy
internal static var commonAcceptableUsePolicy: String { return L10n.tr("Localizable", "common_acceptable_use_policy") }
/// Adding caption
internal static var commonAddingCaption: String { return L10n.tr("Localizable", "common_adding_caption") }
/// Advanced settings
internal static var commonAdvancedSettings: String { return L10n.tr("Localizable", "common_advanced_settings") }
/// Analytics
Expand Down Expand Up @@ -312,6 +320,8 @@ internal enum L10n {
internal static var commonEditedSuffix: String { return L10n.tr("Localizable", "common_edited_suffix") }
/// Editing
internal static var commonEditing: String { return L10n.tr("Localizable", "common_editing") }
/// Editing caption
internal static var commonEditingCaption: String { return L10n.tr("Localizable", "common_editing_caption") }
/// * %1$@ %2$@
internal static func commonEmote(_ p1: Any, _ p2: Any) -> String {
return L10n.tr("Localizable", "common_emote", String(describing: p1), String(describing: p2))
Expand Down
8 changes: 4 additions & 4 deletions ElementX/Sources/Mocks/Generated/GeneratedMocks.swift
Original file line number Diff line number Diff line change
Expand Up @@ -14133,8 +14133,8 @@ class TimelineProxyMock: TimelineProxyProtocol {
var editNewContentCalled: Bool {
return editNewContentCallsCount > 0
}
var editNewContentReceivedArguments: (eventOrTransactionID: EventOrTransactionId, newContent: RoomMessageEventContentWithoutRelation)?
var editNewContentReceivedInvocations: [(eventOrTransactionID: EventOrTransactionId, newContent: RoomMessageEventContentWithoutRelation)] = []
var editNewContentReceivedArguments: (eventOrTransactionID: EventOrTransactionId, newContent: EditedContent)?
var editNewContentReceivedInvocations: [(eventOrTransactionID: EventOrTransactionId, newContent: EditedContent)] = []

var editNewContentUnderlyingReturnValue: Result<Void, TimelineProxyError>!
var editNewContentReturnValue: Result<Void, TimelineProxyError>! {
Expand All @@ -14160,9 +14160,9 @@ class TimelineProxyMock: TimelineProxyProtocol {
}
}
}
var editNewContentClosure: ((EventOrTransactionId, RoomMessageEventContentWithoutRelation) async -> Result<Void, TimelineProxyError>)?
var editNewContentClosure: ((EventOrTransactionId, EditedContent) async -> Result<Void, TimelineProxyError>)?

func edit(_ eventOrTransactionID: EventOrTransactionId, newContent: RoomMessageEventContentWithoutRelation) async -> Result<Void, TimelineProxyError> {
func edit(_ eventOrTransactionID: EventOrTransactionId, newContent: EditedContent) async -> Result<Void, TimelineProxyError> {
editNewContentCallsCount += 1
editNewContentReceivedArguments = (eventOrTransactionID: eventOrTransactionID, newContent: newContent)
DispatchQueue.main.async {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -284,9 +284,11 @@ extension FormatType {
}

enum ComposerMode: Equatable {
enum EditType { case `default`, addCaption, editCaption }

case `default`
case reply(eventID: String, replyDetails: TimelineItemReplyDetails, isThread: Bool)
case edit(originalEventOrTransactionID: EventOrTransactionId)
case edit(originalEventOrTransactionID: EventOrTransactionId, type: EditType)
case recordVoiceMessage(state: AudioRecorderState)
case previewVoiceMessage(state: AudioPlayerState, waveform: WaveformSource, isUploading: Bool)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
case .newMessage:
set(mode: .default)
case .edit(let eventID):
set(mode: .edit(originalEventOrTransactionID: .eventId(eventId: eventID)))
set(mode: .edit(originalEventOrTransactionID: .eventId(eventId: eventID), type: .default))
case .reply(let eventID):
set(mode: .reply(eventID: eventID, replyDetails: .loading(eventID: eventID), isThread: false))
replyLoadingTask = Task {
Expand Down Expand Up @@ -323,7 +323,7 @@ final class ComposerToolbarViewModel: ComposerToolbarViewModelType, ComposerTool
switch state.composerMode {
case .default:
type = .newMessage
case .edit(.eventId(let originalEventID)):
case .edit(.eventId(let originalEventID), .default):
type = .edit(eventID: originalEventID)
case .reply(let eventID, _, _):
type = .reply(eventID: eventID)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,8 @@ struct MessageComposer: View {
switch mode {
case .reply(_, let replyDetails, _):
MessageComposerReplyHeader(replyDetails: replyDetails, action: cancellationAction)
case .edit:
MessageComposerEditHeader(action: cancellationAction)
case .edit(_, let editType):
MessageComposerEditHeader(editType: editType, action: cancellationAction)
case .recordVoiceMessage, .previewVoiceMessage, .default:
EmptyView()
}
Expand Down Expand Up @@ -152,14 +152,20 @@ private struct MessageComposerReplyHeader: View {
}

private struct MessageComposerEditHeader: View {
let editType: ComposerMode.EditType
let action: () -> Void

private var title: String {
switch editType {
case .default: L10n.commonEditing
case .addCaption: L10n.commonAddingCaption
case .editCaption: L10n.commonEditingCaption
}
}

var body: some View {
HStack(alignment: .center, spacing: 8) {
Label(L10n.commonEditing,
icon: \.editSolid,
iconSize: .xSmall,
relativeTo: .compound.bodySMSemibold)
Label(title, icon: \.editSolid, iconSize: .xSmall, relativeTo: .compound.bodySMSemibold)
.labelStyle(MessageComposerHeaderLabelStyle())
Spacer()
Button(action: action) {
Expand Down Expand Up @@ -294,13 +300,20 @@ struct MessageComposer_Previews: PreviewProvider, TestablePreview {
messageComposer()

messageComposer(.init(string: "Some message"),
mode: .edit(originalEventOrTransactionID: .eventId(eventId: UUID().uuidString)))
mode: .edit(originalEventOrTransactionID: .eventId(eventId: UUID().uuidString), type: .default))

messageComposer(mode: .reply(eventID: UUID().uuidString,
replyDetails: .loaded(sender: .init(id: "Kirk"),
eventID: "123",
eventContent: .message(.text(.init(body: "Text: Where the wild things are")))),
isThread: false))

Color.clear.frame(height: 20)

messageComposer(.init(string: "Some new caption"),
mode: .edit(originalEventOrTransactionID: .eventId(eventId: UUID().uuidString), type: .addCaption))
messageComposer(.init(string: "Some updated caption"),
mode: .edit(originalEventOrTransactionID: .eventId(eventId: UUID().uuidString), type: .editCaption))
}
.padding(.horizontal)

Expand Down
63 changes: 38 additions & 25 deletions ElementX/Sources/Screens/Timeline/TimelineInteractionHandler.swift
Original file line number Diff line number Diff line change
Expand Up @@ -100,10 +100,7 @@ class TimelineInteractionHandler {

switch action {
case .copy:
guard let messageTimelineItem = timelineItem as? EventBasedMessageTimelineItemProtocol else {
return
}

guard let messageTimelineItem = timelineItem as? EventBasedMessageTimelineItemProtocol else { return }
UIPasteboard.general.string = messageTimelineItem.body
case .edit:
switch timelineItem {
Expand All @@ -118,6 +115,19 @@ class TimelineInteractionHandler {
default:
MXLog.error("Cannot edit item with id: \(timelineItem.id)")
}
case .addCaption, .editCaption:
switch timelineItem {
case let messageTimelineItem as EventBasedMessageTimelineItemProtocol:
processEditMessageEvent(messageTimelineItem)
default:
MXLog.error("Cannot add/edit caption on item with id: \(timelineItem.id)")
}
case .removeCaption:
guard case let .event(_, eventOrTransactionID) = timelineItem.id else {
MXLog.error("Failed removing caption, missing event ID")
return
}
Task { await timelineController.removeCaption(eventOrTransactionID) }
case .copyPermalink:
guard let eventID = eventTimelineItem.id.eventID else {
actionsSubject.send(.displayErrorToast(L10n.errorFailedCreatingThePermalink))
Expand All @@ -133,17 +143,10 @@ class TimelineInteractionHandler {
UIPasteboard.general.url = permalinkURL
}
case .redact:
guard case let .event(_, eventOrTransactionID) = itemID else {
fatalError()
}

Task {
await timelineController.redact(eventOrTransactionID)
}
guard case let .event(_, eventOrTransactionID) = itemID else { fatalError() }
Task { await timelineController.redact(eventOrTransactionID) }
case .reply:
guard let eventID = eventTimelineItem.id.eventID else {
return
}
guard let eventID = eventTimelineItem.id.eventID else { return }

let replyInfo = buildReplyInfo(for: eventTimelineItem)
let replyDetails = TimelineItemReplyDetails.loaded(sender: eventTimelineItem.sender, eventID: eventID, eventContent: replyInfo.type)
Expand All @@ -156,21 +159,14 @@ class TimelineInteractionHandler {
MXLog.info("Showing debug info for \(eventTimelineItem.id)")
actionsSubject.send(.showDebugInfo(debugInfo))
case .retryDecryption(let sessionID):
Task {
await timelineController.retryDecryption(for: sessionID)
}
Task { await timelineController.retryDecryption(for: sessionID) }
case .report:
actionsSubject.send(.displayReportContent(itemID: itemID, senderID: eventTimelineItem.sender.id))
case .react:
displayEmojiPicker(for: itemID)
case .toggleReaction(let key):
Task {
guard case let .event(_, eventOrTransactionID) = itemID else {
fatalError()
}

await timelineController.toggleReaction(key, to: eventOrTransactionID)
}
guard case let .event(_, eventOrTransactionID) = itemID else { fatalError() }
Task { await timelineController.toggleReaction(key, to: eventOrTransactionID) }
case .endPoll(let pollStartID):
endPoll(pollStartID: pollStartID)
case .pin:
Expand Down Expand Up @@ -202,18 +198,35 @@ class TimelineInteractionHandler {

let text: String
var htmlText: String?
var editType = ComposerMode.EditType.default
switch messageTimelineItem.contentType {
case .text(let content):
text = content.body
htmlText = content.formattedBodyHTMLString
case .emote(let content):
text = "/me " + content.body
case .audio(let content):
text = content.caption ?? ""
htmlText = content.formattedCaptionHTMLString
editType = text.isEmpty ? .addCaption : .editCaption
case .file(let content):
text = content.caption ?? ""
htmlText = content.formattedCaptionHTMLString
editType = text.isEmpty ? .addCaption : .editCaption
case .image(let content):
text = content.caption ?? ""
htmlText = content.formattedCaptionHTMLString
editType = text.isEmpty ? .addCaption : .editCaption
case .video(let content):
text = content.caption ?? ""
htmlText = content.formattedCaptionHTMLString
editType = text.isEmpty ? .addCaption : .editCaption
default:
text = messageTimelineItem.body
}

// Always update the mode first and then the text so that the composer has time to save the text draft
actionsSubject.send(.composer(action: .setMode(mode: .edit(originalEventOrTransactionID: eventOrTransactionID))))
actionsSubject.send(.composer(action: .setMode(mode: .edit(originalEventOrTransactionID: eventOrTransactionID, type: editType))))
actionsSubject.send(.composer(action: .setText(plainText: text, htmlText: htmlText)))
}

Expand Down
8 changes: 7 additions & 1 deletion ElementX/Sources/Screens/Timeline/TimelineViewModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -606,11 +606,17 @@ class TimelineViewModel: TimelineViewModelType, TimelineViewModelProtocol {
html: html,
inReplyToEventID: eventID,
intentionalMentions: intentionalMentions)
case .edit(let originalEventOrTransactionID):
case .edit(let originalEventOrTransactionID, .default):
await timelineController.edit(originalEventOrTransactionID,
message: message,
html: html,
intentionalMentions: intentionalMentions)
case .edit(let originalEventOrTransactionID, .addCaption),
.edit(let originalEventOrTransactionID, .editCaption):
await timelineController.editCaption(originalEventOrTransactionID,
message: message,
html: html,
intentionalMentions: intentionalMentions)
case .default:
switch slashCommand(message: message) {
case .join:
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,9 @@ struct TimelineItemMenuReaction: Hashable {
enum TimelineItemMenuAction: Identifiable, Hashable {
case copy
case edit
case addCaption
case editCaption
case removeCaption
case copyPermalink
case redact
case reply(isThread: Bool)
Expand All @@ -76,7 +79,7 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
/// Whether the item should cancel a reply/edit occurring in the composer.
var switchToDefaultComposer: Bool {
switch self {
case .reply, .edit:
case .reply, .edit, .addCaption, .editCaption:
return false
default:
return true
Expand Down Expand Up @@ -106,7 +109,7 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
/// Whether or not the action is destructive.
var isDestructive: Bool {
switch self {
case .redact, .report:
case .redact, .report, .removeCaption:
return true
default:
return false
Expand All @@ -130,6 +133,12 @@ enum TimelineItemMenuAction: Identifiable, Hashable {
Label(L10n.actionCopy, icon: \.copy)
case .edit:
Label(L10n.actionEdit, icon: \.edit)
case .addCaption:
Label(L10n.actionAddCaption, icon: \.edit)
case .editCaption:
Label(L10n.actionEditCaption, icon: \.edit)
case .removeCaption:
Label(L10n.actionRemoveCaption, icon: \.delete)
case .copyPermalink:
Label(L10n.actionCopyLinkToMessage, icon: \.link)
case .reply(let isThread):
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,15 @@ struct TimelineItemMenuActionProvider {
}

if item.isEditable {
actions.append(.edit)
if let messageItem = item as? EventBasedMessageTimelineItemProtocol, messageItem.supportsMediaCaption {
if messageItem.hasMediaCaption {
actions.append(contentsOf: [.editCaption, .removeCaption])
} else {
actions.append(.addCaption)
}
} else if !(item is VoiceMessageRoomTimelineItem) {
actions.append(.edit)
}
}

if canCurrentUserPin, let eventID = item.id.eventID {
Expand Down
Loading

0 comments on commit 7e1476d

Please sign in to comment.