Ensure roomId are not rendered in the UI.

Add preview to see the effect.
Use RoomAvatar data fallback everywhere, to not use roomId (`!` char) for the avatar initial, but rather `#`.
This commit is contained in:
Benoit Marty
2024-04-25 11:47:00 +02:00
parent 610caf6d90
commit f1a31e3b3c
23 changed files with 107 additions and 69 deletions

View File

@@ -214,6 +214,7 @@ internal fun ContentState.toInviteData(): InviteData? {
return when (this) {
is ContentState.Loaded -> InviteData(
roomId = roomId,
// Note: name should not be null at this point, but use Id just in case...
roomName = name ?: roomId.value,
isDirect = isDirect
)

View File

@@ -47,7 +47,6 @@ import io.element.android.libraries.designsystem.atomic.pages.HeaderFooterPage
import io.element.android.libraries.designsystem.background.LightGradientBackground
import io.element.android.libraries.designsystem.components.async.AsyncActionView
import io.element.android.libraries.designsystem.components.avatar.Avatar
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.designsystem.components.button.BackButton
import io.element.android.libraries.designsystem.components.button.SuperButton
@@ -196,19 +195,7 @@ private fun JoinRoomContent(
RoomPreviewOrganism(
modifier = modifier,
avatar = {
if (contentState.name == null && contentState.roomAvatarUrl == null) {
// Use a Dash Avatar
Avatar(
AvatarData(
id = contentState.roomId.value,
name = "#",
url = null,
size = AvatarSize.RoomHeader,
)
)
} else {
Avatar(contentState.avatarData(AvatarSize.RoomHeader))
}
Avatar(contentState.avatarData(AvatarSize.RoomHeader))
},
title = {
if (contentState.name != null) {

View File

@@ -42,16 +42,21 @@ private fun anEditDefaultNotificationSettingsState(
) = EditDefaultNotificationSettingState(
isOneToOne = isOneToOne,
mode = RoomNotificationMode.MENTIONS_AND_KEYWORDS_ONLY,
roomsWithUserDefinedMode = persistentListOf(aRoomSummary()),
roomsWithUserDefinedMode = persistentListOf(
aRoomSummary("Room"),
aRoomSummary(null),
),
changeNotificationSettingAction = changeNotificationSettingAction,
displayMentionsOnlyDisclaimer = displayMentionsOnlyDisclaimer,
eventSink = {}
)
private fun aRoomSummary() = RoomSummary.Filled(
private fun aRoomSummary(
name: String?,
) = RoomSummary.Filled(
aRoomSummaryDetails(
roomId = RoomId("!roomId:domain"),
name = "Room",
name = name,
avatarUrl = null,
isDirect = false,
lastMessage = null,

View File

@@ -21,6 +21,7 @@ import androidx.compose.foundation.selection.selectableGroup
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.font.FontStyle
import androidx.compose.ui.tooling.preview.PreviewParameter
import io.element.android.features.preferences.impl.R
import io.element.android.libraries.designsystem.components.async.AsyncActionView
@@ -100,7 +101,11 @@ fun EditDefaultNotificationSettingView(
)
ListItem(
headlineContent = {
Text(text = summary.details.name)
val roomName = summary.details.name
Text(
text = roomName ?: stringResource(id = CommonStrings.common_no_room_name),
fontStyle = FontStyle.Italic.takeIf { roomName == null }
)
},
supportingContent = {
Text(text = subtitle)

View File

@@ -24,6 +24,8 @@ import androidx.compose.material3.MaterialTheme
import androidx.compose.runtime.Composable
import androidx.compose.ui.Modifier
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.font.FontStyle
import androidx.compose.ui.tooling.preview.PreviewParameter
import io.element.android.compound.theme.ElementTheme
import io.element.android.compound.tokens.generated.CompoundIcons
import io.element.android.libraries.designsystem.components.list.ListItemContent
@@ -87,8 +89,9 @@ private fun RoomListModalBottomSheetContent(
ListItem(
headlineContent = {
Text(
text = contextMenu.roomName,
text = contextMenu.roomName ?: stringResource(id = CommonStrings.common_no_room_name),
style = ElementTheme.typography.fontBodyLgMedium,
fontStyle = FontStyle.Italic.takeIf { contextMenu.roomName == null }
)
}
)
@@ -192,22 +195,11 @@ private fun RoomListModalBottomSheetContent(
// Remove this preview when the issue is fixed.
@PreviewsDayNight
@Composable
internal fun RoomListModalBottomSheetContentPreview() = ElementPreview {
internal fun RoomListModalBottomSheetContentPreview(
@PreviewParameter(RoomListStateContextMenuShownProvider::class) contextMenu: RoomListState.ContextMenu.Shown
) = ElementPreview {
RoomListModalBottomSheetContent(
contextMenu = aContextMenuShown(hasNewContent = true),
onRoomMarkReadClicked = {},
onRoomMarkUnreadClicked = {},
onRoomSettingsClicked = {},
onLeaveRoomClicked = {},
onFavoriteChanged = {},
)
}
@PreviewsDayNight
@Composable
internal fun RoomListModalBottomSheetContentForDmPreview() = ElementPreview {
RoomListModalBottomSheetContent(
contextMenu = aContextMenuShown(isDm = true),
contextMenu = contextMenu,
onRoomMarkReadClicked = {},
onRoomMarkUnreadClicked = {},
onRoomSettingsClicked = {},

View File

@@ -298,6 +298,7 @@ class RoomListPresenter @Inject constructor(
@VisibleForTesting
internal fun RoomListRoomSummary.toInviteData() = InviteData(
roomId = roomId,
roomName = name,
// Note: `name` should not be null at this point, but just in case, fallback to the roomId
roomName = name ?: roomId.value,
isDirect = isDirect,
)

View File

@@ -48,7 +48,7 @@ data class RoomListState(
data object Hidden : ContextMenu
data class Shown(
val roomId: RoomId,
val roomName: String,
val roomName: String?,
val isDm: Boolean,
val isFavorite: Boolean,
val markAsUnreadFeatureFlagEnabled: Boolean,

View File

@@ -0,0 +1,43 @@
/*
* Copyright (c) 2023 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.features.roomlist.impl
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.libraries.matrix.api.core.RoomId
open class RoomListStateContextMenuShownProvider : PreviewParameterProvider<RoomListState.ContextMenu.Shown> {
override val values: Sequence<RoomListState.ContextMenu.Shown>
get() = sequenceOf(
aContextMenuShown(hasNewContent = true),
aContextMenuShown(isDm = true),
aContextMenuShown(roomName = null)
)
}
internal fun aContextMenuShown(
roomName: String? = "aRoom",
isDm: Boolean = false,
hasNewContent: Boolean = false,
isFavorite: Boolean = false,
) = RoomListState.ContextMenu.Shown(
roomId = RoomId("!aRoom:aDomain"),
roomName = roomName,
isDm = isDm,
markAsUnreadFeatureFlagEnabled = true,
hasNewContent = hasNewContent,
isFavorite = isFavorite,
)

View File

@@ -32,7 +32,6 @@ import io.element.android.features.roomlist.impl.search.aRoomListSearchState
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.designsystem.utils.snackbar.SnackbarMessage
import io.element.android.libraries.matrix.api.core.RoomId
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.user.MatrixUser
import io.element.android.libraries.ui.strings.CommonStrings
@@ -45,6 +44,7 @@ open class RoomListStateProvider : PreviewParameterProvider<RoomListState> {
aRoomListState(),
aRoomListState(snackbarMessage = SnackbarMessage(CommonStrings.common_verification_complete)),
aRoomListState(hasNetworkConnection = false),
aRoomListState(contextMenu = aContextMenuShown(roomName = null)),
aRoomListState(contextMenu = aContextMenuShown(roomName = "A nice room name")),
aRoomListState(contextMenu = aContextMenuShown(isFavorite = true)),
aRoomListState(contentState = aRoomsContentState(securityBannerState = SecurityBannerState.RecoveryKeyConfirmation)),
@@ -109,7 +109,7 @@ internal fun aRoomListRoomSummaryList(): ImmutableList<RoomListRoomSummary> {
),
aRoomListRoomSummary(
id = "!roomId3:domain",
displayType = RoomSummaryDisplayType.PLACEHOLDER,
displayType = RoomSummaryDisplayType.PLACEHOLDER,
),
aRoomListRoomSummary(
id = "!roomId4:domain",
@@ -117,17 +117,3 @@ internal fun aRoomListRoomSummaryList(): ImmutableList<RoomListRoomSummary> {
),
)
}
internal fun aContextMenuShown(
roomName: String = "aRoom",
isDm: Boolean = false,
hasNewContent: Boolean = false,
isFavorite: Boolean = false,
) = RoomListState.ContextMenu.Shown(
roomId = RoomId("!aRoom:aDomain"),
roomName = roomName,
isDm = isDm,
markAsUnreadFeatureFlagEnabled = true,
hasNewContent = hasNewContent,
isFavorite = isFavorite,
)

View File

@@ -41,6 +41,7 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.graphics.Color
import androidx.compose.ui.res.stringResource
import androidx.compose.ui.text.AnnotatedString
import androidx.compose.ui.text.font.FontStyle
import androidx.compose.ui.text.style.TextOverflow
import androidx.compose.ui.tooling.preview.PreviewParameter
import androidx.compose.ui.unit.dp
@@ -168,7 +169,7 @@ private fun RoomSummaryScaffoldRow(
@Composable
private fun NameAndTimestampRow(
name: String,
name: String?,
timestamp: String?,
isHighlighted: Boolean,
modifier: Modifier = Modifier
@@ -181,7 +182,8 @@ private fun NameAndTimestampRow(
Text(
modifier = Modifier.weight(1f),
style = ElementTheme.typography.fontBodyLgMedium,
text = name,
text = name ?: stringResource(id = CommonStrings.common_no_room_name),
fontStyle = FontStyle.Italic.takeIf { name == null },
color = MaterialTheme.roomListRoomName(),
maxLines = 1,
overflow = TextOverflow.Ellipsis
@@ -272,7 +274,7 @@ private fun LastMessageAndIndicatorRow(
@Composable
private fun InviteNameAndIndicatorRow(
name: String,
name: String?,
modifier: Modifier = Modifier,
) {
Row(
@@ -283,7 +285,8 @@ private fun InviteNameAndIndicatorRow(
Text(
modifier = Modifier.weight(1f),
style = ElementTheme.typography.fontBodyLgMedium,
text = name,
text = name ?: stringResource(id = CommonStrings.common_no_room_name),
fontStyle = FontStyle.Italic.takeIf { name == null },
color = MaterialTheme.roomListRoomName(),
maxLines = 1,
overflow = TextOverflow.Ellipsis

View File

@@ -28,7 +28,7 @@ data class RoomListRoomSummary(
val id: String,
val displayType: RoomSummaryDisplayType,
val roomId: RoomId,
val name: String,
val name: String?,
val canonicalAlias: RoomAlias?,
val numberOfUnreadMessages: Int,
val numberOfUnreadMentions: Int,

View File

@@ -31,6 +31,7 @@ open class RoomListRoomSummaryProvider : PreviewParameterProvider<RoomListRoomSu
listOf(
aRoomListRoomSummary(displayType = RoomSummaryDisplayType.PLACEHOLDER),
aRoomListRoomSummary(),
aRoomListRoomSummary(name = null),
aRoomListRoomSummary(lastMessage = null),
aRoomListRoomSummary(
name = "A very long room name that should be truncated",
@@ -100,7 +101,15 @@ open class RoomListRoomSummaryProvider : PreviewParameterProvider<RoomListRoomSu
displayName = "Bob",
),
isDirect = true,
)
),
aRoomListRoomSummary(
name = null,
displayType = RoomSummaryDisplayType.INVITE,
inviteSender = anInviteSender(
userId = UserId("@bob:matrix.org"),
displayName = "Bob",
),
),
),
).flatten()
}
@@ -117,7 +126,7 @@ internal fun anInviteSender(
internal fun aRoomListRoomSummary(
id: String = "!roomId:domain",
name: String = "Room name",
name: String? = "Room name",
numberOfUnreadMessages: Int = 0,
numberOfUnreadMentions: Int = 0,
numberOfUnreadNotifications: Int = 0,

View File

@@ -26,7 +26,8 @@ data class AvatarData(
val size: AvatarSize,
) {
val initial by lazy {
(name?.takeIf { it.isNotBlank() } ?: id)
// For roomIds, use "#" as initial
(name?.takeIf { it.isNotBlank() } ?: id.takeIf { !it.startsWith("!") } ?: "#")
.let { dn ->
var startIndex = 0
val initial = dn[startIndex]

View File

@@ -37,7 +37,7 @@ sealed interface RoomSummary {
data class RoomSummaryDetails(
val roomId: RoomId,
val name: String,
val name: String?,
val canonicalAlias: RoomAlias?,
val isDirect: Boolean,
val avatarUrl: String?,

View File

@@ -40,7 +40,7 @@ val RoomListFilter.predicate
(roomSummary.details.numUnreadNotifications > 0 || roomSummary.details.isMarkedUnread)
}
is RoomListFilter.NormalizedMatchRoomName -> { roomSummary: RoomSummary ->
roomSummary is RoomSummary.Filled && roomSummary.details.name.contains(pattern, ignoreCase = true)
roomSummary is RoomSummary.Filled && roomSummary.details.name.orEmpty().contains(pattern, ignoreCase = true)
}
RoomListFilter.Invite -> { roomSummary: RoomSummary ->
roomSummary.isInvited()

View File

@@ -33,7 +33,7 @@ class RoomSummaryDetailsFactory(private val roomMessageFactory: RoomMessageFacto
}
return RoomSummaryDetails(
roomId = RoomId(roomInfo.id),
name = roomInfo.name ?: roomInfo.id,
name = roomInfo.name,
canonicalAlias = roomInfo.canonicalAlias?.let(::RoomAlias),
isDirect = roomInfo.isDirect,
avatarUrl = roomInfo.avatarUrl,

View File

@@ -63,7 +63,7 @@ fun aRoomSummaryFilled(
fun aRoomSummaryDetails(
roomId: RoomId = A_ROOM_ID,
name: String = A_ROOM_NAME,
name: String? = A_ROOM_NAME,
isDirect: Boolean = false,
avatarUrl: String? = null,
lastMessage: RoomMessage? = aRoomMessage(),

View File

@@ -29,12 +29,13 @@ open class RoomSummaryDetailsProvider : PreviewParameterProvider<RoomSummaryDeta
override val values: Sequence<RoomSummaryDetails>
get() = sequenceOf(
aRoomSummaryDetails(),
aRoomSummaryDetails(name = null),
)
}
fun aRoomSummaryDetails(
roomId: RoomId = RoomId("!room:domain"),
name: String = "roomName",
name: String? = "roomName",
canonicalAlias: RoomAlias? = null,
isDirect: Boolean = true,
avatarUrl: String? = null,

View File

@@ -62,7 +62,8 @@ fun SelectedRoom(
) {
Avatar(AvatarData(roomSummary.roomId.value, roomSummary.name, roomSummary.avatarUrl, AvatarSize.SelectedRoom))
Text(
text = roomSummary.name,
// If name is null, we do not have space to render "No room name", so just use `#` here.
text = roomSummary.name ?: "#",
overflow = TextOverflow.Ellipsis,
maxLines = 1,
style = MaterialTheme.typography.bodyLarge,

View File

@@ -57,7 +57,7 @@ class RoomSelectPresenter @AssistedInject constructor(
LaunchedEffect(query, summaries) {
val filteredSummaries = summaries.filterIsInstance<RoomSummary.Filled>()
.map { it.details }
.filter { it.name.contains(query, ignoreCase = true) }
.filter { it.name.orEmpty().contains(query, ignoreCase = true) }
.distinctBy { it.roomId } // This should be removed once we're sure no duplicate Rooms can be received
.toPersistentList()
results = if (filteredSummaries.isNotEmpty()) {

View File

@@ -68,4 +68,7 @@ private fun aForwardMessagesRoomList() = persistentListOf(
name = "Room with alias",
canonicalAlias = RoomAlias("#alias:example.org"),
),
aRoomSummaryDetails(
name = null,
),
)

View File

@@ -235,7 +235,8 @@ private fun RoomSummaryView(
// Name
Text(
style = ElementTheme.typography.fontBodyLgRegular,
text = summary.name,
// If name is null, we do not have space to render "No room name", so just use `#` here.
text = summary.name ?: "#",
color = ElementTheme.colors.textPrimary,
maxLines = 1,
overflow = TextOverflow.Ellipsis

View File

@@ -101,7 +101,6 @@ class KonsistPreviewTest {
"PollContentViewEndedPreview",
"PollContentViewUndisclosedPreview",
"ReadReceiptBottomSheetPreview",
"RoomListModalBottomSheetContentForDmPreview",
"RoomMemberListViewBannedPreview",
"SasEmojisPreview",
"SecureBackupSetupViewChangePreview",