Skip to content

Commit

Permalink
Merge pull request #2221 from element-hq/feature/bma/displayNameAmbig…
Browse files Browse the repository at this point in the history
…uous

Display name disambiguation
  • Loading branch information
bmarty authored Jan 19, 2024
2 parents b4773d5 + 14d5274 commit 5e359a4
Show file tree
Hide file tree
Showing 7 changed files with 101 additions and 5 deletions.
1 change: 1 addition & 0 deletions changelog.d/2215.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Disambiguate display name in the timeline.
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -63,7 +64,7 @@ class TimelineItemEventFactory @Inject constructor(
senderAvatarUrl = null
}
is ProfileTimelineDetails.Ready -> {
senderDisplayName = senderProfile.displayName
senderDisplayName = senderProfile.getDisambiguatedDisplayName(currentSender)
senderAvatarUrl = senderProfile.avatarUrl
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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
}
}
Original file line number Diff line number Diff line change
@@ -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)")
}
}

0 comments on commit 5e359a4

Please sign in to comment.