Improve the text for mentions and replies in notifications (#3328)

element-hq · Aug 22, 2024 · 7859526 · 7859526
1 parent eeeb8fc
commit 7859526
Show file tree

Hide file tree

Showing 5 changed files with 29 additions and 12 deletions.
diff --git a/...in/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/...in/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt
@@ -104,11 +104,6 @@ class DefaultNotifiableEventResolver @Inject constructor(
             is NotificationContent.MessageLike.RoomMessage -> {
                 val senderDisambiguatedDisplayName = getDisambiguatedDisplayName(content.senderId)
                 val messageBody = descriptionFromMessageContent(content, senderDisambiguatedDisplayName)
-                val notificationBody = if (hasMention) {
-                    stringProvider.getString(R.string.notification_mentioned_you_body, messageBody)
-                } else {
-                    messageBody
-                }
                 buildNotifiableMessageEvent(
                     sessionId = userId,
                     senderId = content.senderId,
@@ -117,12 +112,13 @@ class DefaultNotifiableEventResolver @Inject constructor(
                     noisy = isNoisy,
                     timestamp = this.timestamp,
                     senderDisambiguatedDisplayName = senderDisambiguatedDisplayName,
-                    body = notificationBody,
+                    body = messageBody,
                     imageUriString = fetchImageIfPresent(client)?.toString(),
                     roomName = roomDisplayName,
                     roomIsDm = isDm,
                     roomAvatarPath = roomAvatarUrl,
                     senderAvatarPath = senderAvatarUrl,
+                    hasMentionOrReply = hasMention,
                 )
             }
             is NotificationContent.StateEvent.RoomMemberContent -> {
@@ -340,6 +336,7 @@ internal fun buildNotifiableMessageEvent(
     isRedacted: Boolean = false,
     isUpdated: Boolean = false,
     type: String = EventType.MESSAGE,
+    hasMentionOrReply: Boolean = false,
 ) = NotifiableMessageEvent(
     sessionId = sessionId,
     senderId = senderId,
@@ -363,4 +360,5 @@ internal fun buildNotifiableMessageEvent(
     isRedacted = isRedacted,
     isUpdated = isUpdated,
     type = type,
+    hasMentionOrReply = hasMentionOrReply,
 )
diff --git a/...lin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt b/...lin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt
@@ -397,10 +397,22 @@ class DefaultNotificationCreator @Inject constructor(
             val senderPerson = if (event.outGoingMessage) {
                 null
             } else {
+                val senderName = event.senderDisambiguatedDisplayName.orEmpty()
+                // If the notification is for a mention or reply, we create a fake `Person` with a custom name and key
+                val displayName = if (event.hasMentionOrReply) {
+                    stringProvider.getString(R.string.notification_sender_mention_reply, senderName)
+                } else {
+                    senderName
+                }
+                val key = if (event.hasMentionOrReply) {
+                    "mention-or-reply:${event.eventId.value}"
+                } else {
+                    event.senderId.value
+                }
                 Person.Builder()
-                    .setName(event.senderDisambiguatedDisplayName?.annotateForDebug(70))
+                    .setName(displayName.annotateForDebug(70))
                     .setIcon(bitmapLoader.getUserIcon(event.senderAvatarPath, imageLoader))
-                    .setKey(event.senderId.value)
+                    .setKey(key)
                     .build()
             }
             when {

diff --git a/...tlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt b/...tlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt
@@ -52,7 +52,8 @@ data class NotifiableMessageEvent(
     val outGoingMessageFailed: Boolean = false,
     override val isRedacted: Boolean = false,
     override val isUpdated: Boolean = false,
-    val type: String = EventType.MESSAGE
+    val type: String = EventType.MESSAGE,
+    val hasMentionOrReply: Boolean = false,
 ) : NotifiableEvent {
     override val description: String = body ?: ""
 

diff --git a/...o/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt b/...o/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt
@@ -123,7 +123,7 @@ class DefaultNotifiableEventResolverTest {
             )
         )
         val result = sut.resolveEvent(A_SESSION_ID, A_ROOM_ID, AN_EVENT_ID)
-        val expectedResult = createNotifiableMessageEvent(body = "Mentioned you: Hello world")
+        val expectedResult = createNotifiableMessageEvent(body = "Hello world", hasMentionOrReply = true)
         assertThat(result).isEqualTo(expectedResult)
     }
 
@@ -687,7 +687,10 @@ class DefaultNotifiableEventResolverTest {
         )
     }
 
-    private fun createNotifiableMessageEvent(body: String): NotifiableMessageEvent {
+    private fun createNotifiableMessageEvent(
+        body: String,
+        hasMentionOrReply: Boolean = false,
+    ): NotifiableMessageEvent {
         return NotifiableMessageEvent(
             sessionId = A_SESSION_ID,
             roomId = A_ROOM_ID,
@@ -708,7 +711,8 @@ class DefaultNotifiableEventResolverTest {
             outGoingMessage = false,
             outGoingMessageFailed = false,
             isRedacted = false,
-            isUpdated = false
+            isUpdated = false,
+            hasMentionOrReply = hasMentionOrReply,
         )
     }
 }

diff --git a/libraries/ui-strings/src/main/res/values/localazy.xml b/libraries/ui-strings/src/main/res/values/localazy.xml
@@ -112,6 +112,7 @@
   <string name="action_tap_for_options">"Tap for options"</string>
   <string name="action_try_again">"Try again"</string>
   <string name="action_unpin">"Unpin"</string>
+  <string name="action_view_in_timeline">"View in timeline"</string>
   <string name="action_view_source">"View source"</string>
   <string name="action_yes">"Yes"</string>
   <string name="common_about">"About"</string>
@@ -177,6 +178,7 @@ Reason: %1$s."</string>
   <string name="common_people">"People"</string>
   <string name="common_permalink">"Permalink"</string>
   <string name="common_permission">"Permission"</string>
+  <string name="common_pinned">"Pinned"</string>
   <string name="common_please_wait">"Please wait…"</string>
   <string name="common_poll_end_confirmation">"Are you sure you want to end this poll?"</string>
   <string name="common_poll_summary">"Poll: %1$s"</string>