From 785952628027668147a8fe18ec9121775379c784 Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Thu, 22 Aug 2024 19:25:44 +0200 Subject: [PATCH] Improve the text for mentions and replies in notifications (#3328) --- .../DefaultNotifiableEventResolver.kt | 10 ++++------ .../factories/NotificationCreator.kt | 16 ++++++++++++++-- .../model/NotifiableMessageEvent.kt | 3 ++- .../DefaultNotifiableEventResolverTest.kt | 10 +++++++--- .../ui-strings/src/main/res/values/localazy.xml | 2 ++ 5 files changed, 29 insertions(+), 12 deletions(-) diff --git a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt index 1e186b38a8..953215feb2 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolver.kt +++ b/libraries/push/impl/src/main/kotlin/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/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt index c7a5eb40d6..b1b4068efe 100755 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/factories/NotificationCreator.kt +++ b/libraries/push/impl/src/main/kotlin/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/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt b/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt index b90af3df94..2862dda08b 100644 --- a/libraries/push/impl/src/main/kotlin/io/element/android/libraries/push/impl/notifications/model/NotifiableMessageEvent.kt +++ b/libraries/push/impl/src/main/kotlin/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/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt b/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt index 1f6c98e914..0fa3f730aa 100644 --- a/libraries/push/impl/src/test/kotlin/io/element/android/libraries/push/impl/notifications/DefaultNotifiableEventResolverTest.kt +++ b/libraries/push/impl/src/test/kotlin/io/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 index 730afa291e..501843bffb 100644 --- a/libraries/ui-strings/src/main/res/values/localazy.xml +++ b/libraries/ui-strings/src/main/res/values/localazy.xml @@ -112,6 +112,7 @@ "Tap for options" "Try again" "Unpin" + "View in timeline" "View source" "Yes" "About" @@ -177,6 +178,7 @@ Reason: %1$s." "People" "Permalink" "Permission" + "Pinned" "Please wait…" "Are you sure you want to end this poll?" "Poll: %1$s"