Merge pull request #3816 from element-hq/feature/bma/moreCaptionWork

Iteration on caption

features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/components/event/TimelineItemVideoView.kt

@@ -70,14 +70,14 @@ fun TimelineItemVideoView(
     onContentLayoutChange: (ContentAvoidingLayoutData) -> Unit,
     modifier: Modifier = Modifier,
 ) {
-    val description = stringResource(CommonStrings.common_image)
+    val description = stringResource(CommonStrings.common_video)
     Column(
         modifier = modifier.semantics { contentDescription = description }
     ) {
         val containerModifier = if (content.showCaption) {
             Modifier
-                    .padding(top = 6.dp)
-                    .clip(RoundedCornerShape(6.dp))
+                .padding(top = 6.dp)
+                .clip(RoundedCornerShape(6.dp))
         } else {
             Modifier
         }
@@ -93,8 +93,8 @@ fun TimelineItemVideoView(
                 var isLoaded by remember { mutableStateOf(false) }
                 AsyncImage(
                     modifier = Modifier
-                            .fillMaxWidth()
-                            .then(if (isLoaded) Modifier.background(Color.White) else Modifier),
+                        .fillMaxWidth()
+                        .then(if (isLoaded) Modifier.background(Color.White) else Modifier),
                     model = MediaRequestData(
                         source = content.thumbnailSource,
                         kind = MediaRequestData.Kind.File(

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultPinnedMessagesBannerFormatter.kt

@@ -95,7 +95,9 @@ class DefaultPinnedMessagesBannerFormatter @Inject constructor(
                 messageType.bestDescription.prefixWith(CommonStrings.common_audio)
             }
             is VoiceMessageType -> {
-                messageType.bestDescription.prefixWith(CommonStrings.common_voice_message)
+                // In this case, do not use bestDescription, because the filename is useless, only use the caption if available.
+                messageType.caption?.prefixWith(sp.getString(CommonStrings.common_voice_message))
+                    ?: sp.getString(CommonStrings.common_voice_message)
             }
             is OtherMessageType -> {
                 messageType.body

libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt

@@ -110,25 +110,27 @@ class DefaultRoomLastMessageFormatter @Inject constructor(
                 messageType.toPlainText(permalinkParser)
             }
             is VideoMessageType -> {
-                sp.getString(CommonStrings.common_video)
+                messageType.bestDescription.prefixWith(sp.getString(CommonStrings.common_video))
             }
             is ImageMessageType -> {
-                sp.getString(CommonStrings.common_image)
+                messageType.bestDescription.prefixWith(sp.getString(CommonStrings.common_image))
             }
             is StickerMessageType -> {
-                sp.getString(CommonStrings.common_sticker)
+                messageType.bestDescription.prefixWith(sp.getString(CommonStrings.common_sticker))
             }
             is LocationMessageType -> {
                 sp.getString(CommonStrings.common_shared_location)
             }
             is FileMessageType -> {
-                sp.getString(CommonStrings.common_file)
+                messageType.bestDescription.prefixWith(sp.getString(CommonStrings.common_file))
             }
             is AudioMessageType -> {
-                sp.getString(CommonStrings.common_audio)
+                messageType.bestDescription.prefixWith(sp.getString(CommonStrings.common_audio))
             }
             is VoiceMessageType -> {
-                sp.getString(CommonStrings.common_voice_message)
+                // In this case, do not use bestDescription, because the filename is useless, only use the caption if available.
+                messageType.caption?.prefixWith(sp.getString(CommonStrings.common_voice_message))
+                    ?: sp.getString(CommonStrings.common_voice_message)
             }
             is OtherMessageType -> {
                 messageType.body
@@ -140,7 +142,7 @@ class DefaultRoomLastMessageFormatter @Inject constructor(
         return message.prefixIfNeeded(senderDisambiguatedDisplayName, isDmRoom, isOutgoing)
     }
 
-    private fun String.prefixIfNeeded(
+    private fun CharSequence.prefixIfNeeded(
         senderDisambiguatedDisplayName: String,
         isDmRoom: Boolean,
         isOutgoing: Boolean,

libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultPinnedMessagesBannerFormatterTest.kt

@@ -159,11 +159,11 @@ class DefaultPinnedMessagesBannerFormatterTest {
             val expectedResult = when (type) {
                 is VideoMessageType,
                 is AudioMessageType,
-                is VoiceMessageType,
                 is ImageMessageType,
                 is StickerMessageType,
                 is FileMessageType,
                 is LocationMessageType -> AnnotatedString::class.java
+                is VoiceMessageType,
                 is EmoteMessageType,
                 is TextMessageType,
                 is NoticeMessageType,
@@ -176,7 +176,7 @@ class DefaultPinnedMessagesBannerFormatterTest {
             val expectedResult = when (type) {
                 is VideoMessageType -> "Video: Shared body"
                 is AudioMessageType -> "Audio: Shared body"
-                is VoiceMessageType -> "Voice message: Shared body"
+                is VoiceMessageType -> "Voice message"
                 is ImageMessageType -> "Image: Shared body"
                 is StickerMessageType -> "Sticker: Shared body"
                 is FileMessageType -> "File: Shared body"

libraries/eventformatter/impl/src/test/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatterTest.kt

@@ -208,32 +208,51 @@ class DefaultRoomLastMessageFormatterTest {
 
         // Verify results of DM mode
         for ((type, result) in resultsInDm) {
+            val string = result.toString()
             val expectedResult = when (type) {
-                is VideoMessageType -> "Video"
-                is AudioMessageType -> "Audio"
+                is VideoMessageType -> "Video: Shared body"
+                is AudioMessageType -> "Audio: Shared body"
                 is VoiceMessageType -> "Voice message"
-                is ImageMessageType -> "Image"
-                is StickerMessageType -> "Sticker"
-                is FileMessageType -> "File"
+                is ImageMessageType -> "Image: Shared body"
+                is StickerMessageType -> "Sticker: Shared body"
+                is FileMessageType -> "File: Shared body"
                 is LocationMessageType -> "Shared location"
                 is EmoteMessageType -> "* $senderName ${type.body}"
                 is TextMessageType,
                 is NoticeMessageType,
                 is OtherMessageType -> body
             }
-            assertWithMessage("$type was not properly handled for DM").that(result).isEqualTo(expectedResult)
+            val shouldCreateAnnotatedString = when (type) {
+                is VideoMessageType -> true
+                is AudioMessageType -> true
+                is VoiceMessageType -> false
+                is ImageMessageType -> true
+                is StickerMessageType -> true
+                is FileMessageType -> true
+                is LocationMessageType -> false
+                is EmoteMessageType -> false
+                is TextMessageType -> false
+                is NoticeMessageType -> false
+                is OtherMessageType -> false
+            }
+            if (shouldCreateAnnotatedString) {
+                assertWithMessage("$type doesn't produce an AnnotatedString")
+                    .that(result)
+                    .isInstanceOf(AnnotatedString::class.java)
+            }
+            assertWithMessage("$type was not properly handled for DM").that(string).isEqualTo(expectedResult)
         }
 
         // Verify results of Room mode
         for ((type, result) in resultsInRoom) {
             val string = result.toString()
             val expectedResult = when (type) {
-                is VideoMessageType -> "$expectedPrefix: Video"
-                is AudioMessageType -> "$expectedPrefix: Audio"
+                is VideoMessageType -> "$expectedPrefix: Video: Shared body"
+                is AudioMessageType -> "$expectedPrefix: Audio: Shared body"
                 is VoiceMessageType -> "$expectedPrefix: Voice message"
-                is ImageMessageType -> "$expectedPrefix: Image"
-                is StickerMessageType -> "$expectedPrefix: Sticker"
-                is FileMessageType -> "$expectedPrefix: File"
+                is ImageMessageType -> "$expectedPrefix: Image: Shared body"
+                is StickerMessageType -> "$expectedPrefix: Sticker: Shared body"
+                is FileMessageType -> "$expectedPrefix: File: Shared body"
                 is LocationMessageType -> "$expectedPrefix: Shared location"
                 is TextMessageType,
                 is NoticeMessageType,
@@ -249,7 +268,8 @@ class DefaultRoomLastMessageFormatterTest {
                 is FileMessageType -> true
                 is LocationMessageType -> false
                 is EmoteMessageType -> false
-                is TextMessageType, is NoticeMessageType -> true
+                is TextMessageType -> true
+                is NoticeMessageType -> true
                 is OtherMessageType -> true
             }
             if (shouldCreateAnnotatedString) {

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt

@@ -107,6 +107,7 @@ class DefaultNotifiableEventResolver @Inject constructor(
                     senderDisambiguatedDisplayName = senderDisambiguatedDisplayName,
                     body = messageBody,
                     imageUriString = content.fetchImageIfPresent(client)?.toString(),
+                    imageMimeType = content.getImageMimetype(),
                     roomName = roomDisplayName,
                     roomIsDm = isDm,
                     roomAvatarPath = roomAvatarUrl,
@@ -316,6 +317,17 @@ class DefaultNotifiableEventResolver @Inject constructor(
             }
             .getOrNull()
     }
+
+    private suspend fun NotificationContent.MessageLike.RoomMessage.getImageMimetype(): String? {
+        if (appPreferencesStore.doesHideImagesAndVideosFlow().first()) {
+            return null
+        }
+        return when (val messageType = messageType) {
+            is ImageMessageType -> messageType.info?.mimetype
+            is VideoMessageType -> null // Use the thumbnail here?
+            else -> null
+        }
+    }
 }
 
 @Suppress("LongParameterList")
@@ -333,6 +345,7 @@ internal fun buildNotifiableMessageEvent(
     // We cannot use Uri? type here, as that could trigger a
     // NotSerializableException when persisting this to storage
     imageUriString: String? = null,
+    imageMimeType: String? = null,
     threadId: ThreadId? = null,
     roomName: String? = null,
     roomIsDm: Boolean = false,
@@ -358,6 +371,7 @@ internal fun buildNotifiableMessageEvent(
     senderDisambiguatedDisplayName = senderDisambiguatedDisplayName,
     body = body,
     imageUriString = imageUriString,
+    imageMimeType = imageMimeType,
     threadId = threadId,
     roomName = roomName,
     roomIsDm = roomIsDm,

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/NotificationBroadcastReceiverHandler.kt

@@ -150,6 +150,7 @@ class NotificationBroadcastReceiverHandler @Inject constructor(
                 ?: stringProvider.getString(R.string.notification_sender_me),
             body = message,
             imageUriString = null,
+            imageMimeType = null,
             threadId = threadId,
             roomName = room.displayName,
             roomIsDm = room.isDm,

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt

@@ -419,11 +419,22 @@ class DefaultNotificationCreator @Inject constructor(
                         senderPerson
                     ).also { message ->
                         event.imageUri?.let {
-                            message.setData("image/", it)
+                            message.setData(event.imageMimeType ?: "image/", it)
                         }
                         message.extras.putString(MESSAGE_EVENT_ID, event.eventId.value)
                     }
                     addMessage(message)
+
+                    // Add additional message for captions
+                    if (event.imageUri != null && event.body != null) {
+                        addMessage(
+                            MessagingStyle.Message(
+                                event.body,
+                                event.timestamp,
+                                senderPerson,
+                            )
+                        )
+                    }
                 }
             }
         }

libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt

@@ -31,7 +31,8 @@ data class NotifiableMessageEvent(
     val body: String?,
     // We cannot use Uri? type here, as that could trigger a
     // NotSerializableException when persisting this to storage
-    val imageUriString: String?,
+    private val imageUriString: String?,
+    val imageMimeType: String?,
     val threadId: ThreadId?,
     val roomName: String?,
     val roomIsDm: Boolean = false,

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt

@@ -590,6 +590,7 @@ class DefaultNotifiableEventResolverTest {
                 senderDisambiguatedDisplayName = A_USER_NAME_2,
                 body = "Call in progress (unsupported)",
                 imageUriString = null,
+                imageMimeType = null,
                 threadId = null,
                 roomName = A_ROOM_NAME,
                 roomAvatarPath = null,
@@ -669,6 +670,7 @@ class DefaultNotifiableEventResolverTest {
                 canBeReplaced = false,
                 isRedacted = false,
                 imageUriString = null,
+                imageMimeType = null,
                 type = EventType.CALL_NOTIFY,
             )
         )
@@ -704,6 +706,7 @@ class DefaultNotifiableEventResolverTest {
                 canBeReplaced = false,
                 isRedacted = false,
                 imageUriString = null,
+                imageMimeType = null,
                 type = EventType.CALL_NOTIFY,
             )
         )

libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/fixtures/NotifiableEventFixture.kt

@@ -100,6 +100,7 @@ fun aNotifiableMessageEvent(
     canBeReplaced = false,
     isRedacted = isRedacted,
     imageUriString = null,
+    imageMimeType = null,
     roomAvatarPath = null,
     senderAvatarPath = null,
     soundName = null,