From 14d5274d22eb8bd925663295c5c785ca0b44b853 Mon Sep 17 00:00:00 2001 From: Benoit Marty Date: Fri, 12 Jan 2024 15:56:47 +0100 Subject: [PATCH] Display name disambiguation #2215. Applied to: - timeline message - detail of timeline message - reply preview of timeline message - rendering of state Event Not applied to: - room last message - room member list (we display the MatrixId here) - room member detail page --- changelog.d/2215.misc | 1 + .../event/TimelineItemContentFactory.kt | 4 +- .../event/TimelineItemEventFactory.kt | 3 +- .../impl/DefaultRoomLastMessageFormatter.kt | 2 + .../impl/DefaultTimelineEventFormatter.kt | 4 +- .../item/event/ProfileTimelineDetails.kt | 18 +++++ .../item/event/ProfileTimelineDetailsTest.kt | 74 +++++++++++++++++++ 7 files changed, 101 insertions(+), 5 deletions(-) create mode 100644 changelog.d/2215.misc create mode 100644 libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetailsTest.kt diff --git a/changelog.d/2215.misc b/changelog.d/2215.misc new file mode 100644 index 0000000000..1b343d2eb5 --- /dev/null +++ b/changelog.d/2215.misc @@ -0,0 +1 @@ +Disambiguate display name in the timeline. diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentFactory.kt index abd4dce536..4a15a47f95 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemContentFactory.kt @@ -24,13 +24,13 @@ import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParse import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent import io.element.android.libraries.matrix.api.timeline.item.event.PollContent import io.element.android.libraries.matrix.api.timeline.item.event.ProfileChangeContent -import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails import io.element.android.libraries.matrix.api.timeline.item.event.RedactedContent import io.element.android.libraries.matrix.api.timeline.item.event.RoomMembershipContent import io.element.android.libraries.matrix.api.timeline.item.event.StateContent import io.element.android.libraries.matrix.api.timeline.item.event.StickerContent import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecryptContent import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent +import io.element.android.libraries.matrix.api.timeline.item.event.getDisambiguatedDisplayName import javax.inject.Inject class TimelineItemContentFactory @Inject constructor( @@ -50,7 +50,7 @@ class TimelineItemContentFactory @Inject constructor( is FailedToParseMessageLikeContent -> failedToParseMessageFactory.create(itemContent) is FailedToParseStateContent -> failedToParseStateFactory.create(itemContent) is MessageContent -> { - val senderDisplayName = (eventTimelineItem.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: eventTimelineItem.sender.value + val senderDisplayName = eventTimelineItem.senderProfile.getDisambiguatedDisplayName(eventTimelineItem.sender) messageFactory.create(itemContent, senderDisplayName, eventTimelineItem.eventId) } is ProfileChangeContent -> profileChangeFactory.create(eventTimelineItem) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt index 0f204921d2..29ea9df6c2 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/timeline/factories/event/TimelineItemEventFactory.kt @@ -33,6 +33,7 @@ import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.room.RoomMember import io.element.android.libraries.matrix.api.timeline.MatrixTimelineItem import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails +import io.element.android.libraries.matrix.api.timeline.item.event.getDisambiguatedDisplayName import kotlinx.collections.immutable.toImmutableList import java.text.DateFormat import java.util.Date @@ -63,7 +64,7 @@ class TimelineItemEventFactory @Inject constructor( senderAvatarUrl = null } is ProfileTimelineDetails.Ready -> { - senderDisplayName = senderProfile.displayName + senderDisplayName = senderProfile.getDisambiguatedDisplayName(currentSender) senderAvatarUrl = senderProfile.avatarUrl } } diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt index 131250fac7..e9066681c5 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultRoomLastMessageFormatter.kt @@ -69,6 +69,8 @@ class DefaultRoomLastMessageFormatter @Inject constructor( override fun format(event: EventTimelineItem, isDmRoom: Boolean): CharSequence? { val isOutgoing = event.isOwn + // Note: we do not use disambiguated display name here, see + // https://github.com/element-hq/element-x-ios/issues/1845#issuecomment-1888707428 val senderDisplayName = (event.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: event.sender.value return when (val content = event.content) { is MessageContent -> processMessageContents(content, senderDisplayName, isDmRoom) diff --git a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultTimelineEventFormatter.kt b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultTimelineEventFormatter.kt index 63c8f939a0..e147f25e16 100644 --- a/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultTimelineEventFormatter.kt +++ b/libraries/eventformatter/impl/src/main/kotlin/io/element/android/libraries/eventformatter/impl/DefaultTimelineEventFormatter.kt @@ -27,13 +27,13 @@ import io.element.android.libraries.matrix.api.timeline.item.event.FailedToParse import io.element.android.libraries.matrix.api.timeline.item.event.MessageContent import io.element.android.libraries.matrix.api.timeline.item.event.PollContent import io.element.android.libraries.matrix.api.timeline.item.event.ProfileChangeContent -import io.element.android.libraries.matrix.api.timeline.item.event.ProfileTimelineDetails import io.element.android.libraries.matrix.api.timeline.item.event.RedactedContent import io.element.android.libraries.matrix.api.timeline.item.event.RoomMembershipContent import io.element.android.libraries.matrix.api.timeline.item.event.StateContent import io.element.android.libraries.matrix.api.timeline.item.event.StickerContent import io.element.android.libraries.matrix.api.timeline.item.event.UnableToDecryptContent import io.element.android.libraries.matrix.api.timeline.item.event.UnknownContent +import io.element.android.libraries.matrix.api.timeline.item.event.getDisambiguatedDisplayName import io.element.android.libraries.ui.strings.CommonStrings import io.element.android.services.toolbox.api.strings.StringProvider import javax.inject.Inject @@ -48,7 +48,7 @@ class DefaultTimelineEventFormatter @Inject constructor( ) : TimelineEventFormatter { override fun format(event: EventTimelineItem): CharSequence? { val isOutgoing = event.isOwn - val senderDisplayName = (event.senderProfile as? ProfileTimelineDetails.Ready)?.displayName ?: event.sender.value + val senderDisplayName = event.senderProfile.getDisambiguatedDisplayName(event.sender) return when (val content = event.content) { is RoomMembershipContent -> { roomMembershipContentFormatter.format(content, senderDisplayName, isOutgoing) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetails.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetails.kt index 44664a256a..efbc2564a9 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetails.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetails.kt @@ -17,6 +17,7 @@ package io.element.android.libraries.matrix.api.timeline.item.event import androidx.compose.runtime.Immutable +import io.element.android.libraries.matrix.api.core.UserId @Immutable sealed interface ProfileTimelineDetails { @@ -34,3 +35,20 @@ sealed interface ProfileTimelineDetails { val message: String ) : ProfileTimelineDetails } + +/** + * Returns a disambiguated display name for the user. + * If the display name is null, or profile is not Ready, the user ID is returned. + * If the display name is ambiguous, the user ID is appended in parentheses. + * Otherwise, the display name is returned. + */ +fun ProfileTimelineDetails.getDisambiguatedDisplayName(userId: UserId): String { + return when (this) { + is ProfileTimelineDetails.Ready -> when { + displayName == null -> userId.value + displayNameAmbiguous -> "$displayName ($userId)" + else -> displayName + } + else -> userId.value + } +} diff --git a/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetailsTest.kt b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetailsTest.kt new file mode 100644 index 0000000000..e760cfa210 --- /dev/null +++ b/libraries/matrix/api/src/test/kotlin/io/element/android/libraries/matrix/api/timeline/item/event/ProfileTimelineDetailsTest.kt @@ -0,0 +1,74 @@ +/* + * Copyright (c) 2024 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package io.element.android.libraries.matrix.api.timeline.item.event + +import com.google.common.truth.Truth.assertThat +import io.element.android.libraries.matrix.api.core.UserId +import org.junit.Test + +private const val A_USER_ID = "@foo:example.org" +private val aUserId = UserId(A_USER_ID) + +class ProfileTimelineDetailsTest { + @Test + fun `getDisambiguatedDisplayName of Unavailable should be equal to userId`() { + assertThat(ProfileTimelineDetails.Unavailable.getDisambiguatedDisplayName(aUserId)).isEqualTo(A_USER_ID) + } + + @Test + fun `getDisambiguatedDisplayName of Error should be equal to userId`() { + assertThat(ProfileTimelineDetails.Error("An error").getDisambiguatedDisplayName(aUserId)).isEqualTo(A_USER_ID) + } + + @Test + fun `getDisambiguatedDisplayName of Pending should be equal to userId`() { + assertThat(ProfileTimelineDetails.Pending.getDisambiguatedDisplayName(aUserId)).isEqualTo(A_USER_ID) + } + + @Test + fun `getDisambiguatedDisplayName of Ready without display name should be equal to userId`() { + assertThat( + ProfileTimelineDetails.Ready( + displayName = null, + displayNameAmbiguous = false, + avatarUrl = null, + ).getDisambiguatedDisplayName(aUserId) + ).isEqualTo(A_USER_ID) + } + + @Test + fun `getDisambiguatedDisplayName of Ready with display name should be equal to display name`() { + assertThat( + ProfileTimelineDetails.Ready( + displayName = "Alice", + displayNameAmbiguous = false, + avatarUrl = null, + ).getDisambiguatedDisplayName(aUserId) + ).isEqualTo("Alice") + } + + @Test + fun `getDisambiguatedDisplayName of Ready with display name and ambiguous should be equal to display name with user id`() { + assertThat( + ProfileTimelineDetails.Ready( + displayName = "Alice", + displayNameAmbiguous = true, + avatarUrl = null, + ).getDisambiguatedDisplayName(aUserId) + ).isEqualTo("Alice ($A_USER_ID)") + } +}