Create data classes TypingRoomMember and IdentityRoomMember to avoid the risk of useless recomposition.

Also remove TypingNotificationStateForMessagesProvider which was not used anymore.
This commit is contained in:
Benoit Marty 2024-10-08 14:28:25 +02:00
parent 8aa34d8cd4
commit 41749ed5b1
13 changed files with 144 additions and 125 deletions

View file

@ -8,7 +8,6 @@
package io.element.android.features.messages.impl.crypto.identity
import io.element.android.libraries.matrix.api.encryption.identity.IdentityState
import io.element.android.libraries.matrix.api.room.RoomMember
import kotlinx.collections.immutable.ImmutableList
data class IdentityChangeState(
@ -17,6 +16,6 @@ data class IdentityChangeState(
)
data class RoomMemberIdentityStateChange(
val roomMember: RoomMember,
val identityRoomMember: IdentityRoomMember,
val identityState: IdentityState,
)

View file

@ -13,12 +13,14 @@ import androidx.compose.runtime.getValue
import androidx.compose.runtime.produceState
import androidx.compose.runtime.rememberCoroutineScope
import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.RoomMembershipState
import io.element.android.libraries.matrix.api.room.roomMembers
import io.element.android.libraries.matrix.ui.model.getAvatarData
import kotlinx.collections.immutable.PersistentList
import kotlinx.collections.immutable.persistentListOf
import kotlinx.collections.immutable.toPersistentList
@ -59,9 +61,10 @@ class IdentityChangeStatePresenter @Inject constructor(
identityStateChanges.map { identityStateChange ->
val member = membersState.roomMembers()
?.firstOrNull { roomMember -> roomMember.userId == identityStateChange.userId }
?.toIdentityRoomMember()
?: createDefaultRoomMemberForIdentityChange(identityStateChange.userId)
RoomMemberIdentityStateChange(
roomMember = member,
identityRoomMember = member,
identityState = identityStateChange.identityState,
)
}
@ -81,21 +84,19 @@ class IdentityChangeStatePresenter @Inject constructor(
}
}
/**
* Create a default [RoomMember] for identity change events.
* In this case, only the userId will be used for rendering, other fields are not used, but keep them
* as close as possible to the actual data.
*/
private fun createDefaultRoomMemberForIdentityChange(userId: UserId): RoomMember {
return RoomMember(
userId = userId,
displayName = null,
avatarUrl = null,
membership = RoomMembershipState.JOIN,
isNameAmbiguous = false,
powerLevel = 0,
normalizedPowerLevel = 0,
isIgnored = false,
role = RoomMember.Role.USER,
)
}
private fun RoomMember.toIdentityRoomMember() = IdentityRoomMember(
userId = userId,
disambiguatedDisplayName = disambiguatedDisplayName,
avatarData = getAvatarData(AvatarSize.ComposerAlert),
)
private fun createDefaultRoomMemberForIdentityChange(userId: UserId) = IdentityRoomMember(
userId = userId,
disambiguatedDisplayName = userId.value,
avatarData = AvatarData(
id = userId.value,
name = null,
url = null,
size = AvatarSize.ComposerAlert,
),
)

View file

@ -8,7 +8,9 @@
package io.element.android.features.messages.impl.crypto.identity
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.features.messages.impl.typing.aTypingRoomMember
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.encryption.identity.IdentityState
import kotlinx.collections.immutable.toImmutableList
@ -19,7 +21,7 @@ class IdentityChangeStateProvider : PreviewParameterProvider<IdentityChangeState
anIdentityChangeState(
roomMemberIdentityStateChanges = listOf(
RoomMemberIdentityStateChange(
roomMember = aTypingRoomMember(displayName = "Alice"),
identityRoomMember = anIdentityRoomMember(disambiguatedDisplayName = "Alice"),
identityState = IdentityState.PinViolation,
),
),
@ -33,3 +35,18 @@ internal fun anIdentityChangeState(
roomMemberIdentityStateChanges = roomMemberIdentityStateChanges.toImmutableList(),
eventSink = {},
)
internal fun anIdentityRoomMember(
userId: UserId = UserId("@alice:example.com"),
disambiguatedDisplayName: String = userId.value,
avatarData: AvatarData = AvatarData(
id = userId.value,
name = null,
url = null,
size = AvatarSize.ComposerAlert,
),
) = IdentityRoomMember(
userId = userId,
disambiguatedDisplayName = disambiguatedDisplayName,
avatarData = avatarData,
)

View file

@ -39,12 +39,12 @@ fun IdentityChangeStateView(
if (pinViolationIdentityChange != null) {
ComposerAlertMolecule(
modifier = modifier,
avatar = pinViolationIdentityChange.roomMember.getAvatarData(AvatarSize.ComposerAlert),
avatar = pinViolationIdentityChange.identityRoomMember.avatarData,
content = buildAnnotatedString {
val learnMoreStr = stringResource(CommonStrings.action_learn_more)
val fullText = stringResource(
id = CommonStrings.crypto_identity_change_pin_violation,
pinViolationIdentityChange.roomMember.disambiguatedDisplayName,
pinViolationIdentityChange.identityRoomMember.disambiguatedDisplayName,
learnMoreStr,
)
val learnMoreStartIndex = fullText.indexOf(learnMoreStr)
@ -68,7 +68,7 @@ fun IdentityChangeStateView(
end = learnMoreStartIndex + learnMoreStr.length,
)
},
onSubmitClick = { state.eventSink(IdentityChangeEvent.Submit(pinViolationIdentityChange.roomMember.userId)) },
onSubmitClick = { state.eventSink(IdentityChangeEvent.Submit(pinViolationIdentityChange.identityRoomMember.userId)) },
isCritical = pinViolationIdentityChange.identityState == IdentityState.VerificationViolation,
)
}

View file

@ -0,0 +1,17 @@
/*
* Copyright 2024 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only
* Please see LICENSE in the repository root for full details.
*/
package io.element.android.features.messages.impl.crypto.identity
import io.element.android.libraries.designsystem.components.avatar.AvatarData
import io.element.android.libraries.matrix.api.core.UserId
data class IdentityRoomMember(
val userId: UserId,
val disambiguatedDisplayName: String,
val avatarData: AvatarData,
)

View file

@ -20,7 +20,6 @@ import io.element.android.libraries.architecture.Presenter
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.MatrixRoom
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.RoomMembershipState
import io.element.android.libraries.matrix.api.room.roomMembers
import io.element.android.libraries.preferences.api.store.SessionPreferencesStore
import kotlinx.collections.immutable.ImmutableList
@ -60,12 +59,13 @@ class TypingNotificationPresenter @Inject constructor(
)
}
private fun ProduceStateScope<ImmutableList<RoomMember>>.observeRoomTypingMembers() {
private fun ProduceStateScope<ImmutableList<TypingRoomMember>>.observeRoomTypingMembers() {
combine(room.roomTypingMembersFlow, room.membersStateFlow) { typingMembers, membersState ->
typingMembers
.map { userId ->
membersState.roomMembers()
?.firstOrNull { roomMember -> roomMember.userId == userId }
?.toTypingRoomMember()
?: createDefaultRoomMemberForTyping(userId)
}
}
@ -77,21 +77,14 @@ class TypingNotificationPresenter @Inject constructor(
}
}
/**
* Create a default [RoomMember] for typing events.
* In this case, only the userId will be used for rendering, other fields are not used, but keep them
* as close as possible to the actual data.
*/
private fun createDefaultRoomMemberForTyping(userId: UserId): RoomMember {
return RoomMember(
userId = userId,
displayName = null,
avatarUrl = null,
membership = RoomMembershipState.JOIN,
isNameAmbiguous = false,
powerLevel = 0,
normalizedPowerLevel = 0,
isIgnored = false,
role = RoomMember.Role.USER,
private fun RoomMember.toTypingRoomMember(): TypingRoomMember {
return TypingRoomMember(
disambiguatedDisplayName = disambiguatedDisplayName,
)
}
private fun createDefaultRoomMemberForTyping(userId: UserId): TypingRoomMember {
return TypingRoomMember(
disambiguatedDisplayName = userId.value,
)
}

View file

@ -7,7 +7,6 @@
package io.element.android.features.messages.impl.typing
import io.element.android.libraries.matrix.api.room.RoomMember
import kotlinx.collections.immutable.ImmutableList
/**
@ -17,7 +16,7 @@ data class TypingNotificationState(
/** Whether to render the typing notifications based on the user's preferences. */
val renderTypingNotifications: Boolean,
/** The room members currently typing. */
val typingMembers: ImmutableList<RoomMember>,
val typingMembers: ImmutableList<TypingRoomMember>,
/** Whether to reserve space for the typing notifications at the bottom of the timeline. */
val reserveSpace: Boolean,
)

View file

@ -1,27 +0,0 @@
/*
* Copyright 2024 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only
* Please see LICENSE in the repository root for full details.
*/
package io.element.android.features.messages.impl.typing
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
class TypingNotificationStateForMessagesProvider : PreviewParameterProvider<TypingNotificationState> {
override val values: Sequence<TypingNotificationState>
get() = sequenceOf(
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice"),
aTypingRoomMember(displayName = "Bob"),
),
),
aTypingNotificationState(
typingMembers = listOf(aTypingRoomMember()),
reserveSpace = true
),
aTypingNotificationState(reserveSpace = true),
)
}

View file

@ -8,9 +8,6 @@
package io.element.android.features.messages.impl.typing
import androidx.compose.ui.tooling.preview.PreviewParameterProvider
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.room.RoomMember
import io.element.android.libraries.matrix.api.room.RoomMembershipState
import kotlinx.collections.immutable.toImmutableList
class TypingNotificationStateProvider : PreviewParameterProvider<TypingNotificationState> {
@ -24,39 +21,39 @@ class TypingNotificationStateProvider : PreviewParameterProvider<TypingNotificat
),
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice"),
aTypingRoomMember(disambiguatedDisplayName = "Alice"),
),
),
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice", isNameAmbiguous = true),
aTypingRoomMember(disambiguatedDisplayName = "Alice (@alice:example.com)"),
),
),
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice"),
aTypingRoomMember(displayName = "Bob"),
aTypingRoomMember(disambiguatedDisplayName = "Alice"),
aTypingRoomMember(disambiguatedDisplayName = "Bob"),
),
),
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice"),
aTypingRoomMember(displayName = "Bob"),
aTypingRoomMember(displayName = "Charlie"),
aTypingRoomMember(disambiguatedDisplayName = "Alice"),
aTypingRoomMember(disambiguatedDisplayName = "Bob"),
aTypingRoomMember(disambiguatedDisplayName = "Charlie"),
),
),
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice"),
aTypingRoomMember(displayName = "Bob"),
aTypingRoomMember(displayName = "Charlie"),
aTypingRoomMember(displayName = "Dan"),
aTypingRoomMember(displayName = "Eve"),
aTypingRoomMember(disambiguatedDisplayName = "Alice"),
aTypingRoomMember(disambiguatedDisplayName = "Bob"),
aTypingRoomMember(disambiguatedDisplayName = "Charlie"),
aTypingRoomMember(disambiguatedDisplayName = "Dan"),
aTypingRoomMember(disambiguatedDisplayName = "Eve"),
),
),
aTypingNotificationState(
typingMembers = listOf(
aTypingRoomMember(displayName = "Alice with a very long display name which means that it will be truncated"),
aTypingRoomMember(disambiguatedDisplayName = "Alice with a very long display name which means that it will be truncated"),
),
),
aTypingNotificationState(
@ -67,7 +64,7 @@ class TypingNotificationStateProvider : PreviewParameterProvider<TypingNotificat
}
internal fun aTypingNotificationState(
typingMembers: List<RoomMember> = emptyList(),
typingMembers: List<TypingRoomMember> = emptyList(),
reserveSpace: Boolean = false,
) = TypingNotificationState(
renderTypingNotifications = true,
@ -76,19 +73,7 @@ internal fun aTypingNotificationState(
)
internal fun aTypingRoomMember(
userId: UserId = UserId("@alice:example.com"),
displayName: String? = null,
isNameAmbiguous: Boolean = false,
): RoomMember {
return RoomMember(
userId = userId,
displayName = displayName,
avatarUrl = null,
membership = RoomMembershipState.JOIN,
isNameAmbiguous = isNameAmbiguous,
powerLevel = 0,
normalizedPowerLevel = 0,
isIgnored = false,
role = RoomMember.Role.USER,
)
}
disambiguatedDisplayName: String = "@alice:example.com",
) = TypingRoomMember(
disambiguatedDisplayName = disambiguatedDisplayName,
)

View file

@ -41,7 +41,6 @@ import io.element.android.features.messages.impl.R
import io.element.android.libraries.designsystem.preview.ElementPreview
import io.element.android.libraries.designsystem.preview.PreviewsDayNight
import io.element.android.libraries.designsystem.theme.components.Text
import io.element.android.libraries.matrix.api.room.RoomMember
import kotlinx.collections.immutable.ImmutableList
@Suppress("MultipleEmitters") // False positive
@ -53,7 +52,8 @@ fun TypingNotificationView(
val displayNotifications = state.typingMembers.isNotEmpty() && state.renderTypingNotifications
@Suppress("ModifierNaming")
@Composable fun TypingText(text: AnnotatedString, textModifier: Modifier = Modifier) {
@Composable
fun TypingText(text: AnnotatedString, textModifier: Modifier = Modifier) {
Text(
modifier = textModifier,
text = text,
@ -66,7 +66,9 @@ fun TypingNotificationView(
// Display the typing notification space when either a typing notification needs to be displayed or a previous one already was
AnimatedVisibility(
modifier = modifier.fillMaxWidth().padding(vertical = 2.dp),
modifier = modifier
.fillMaxWidth()
.padding(vertical = 2.dp),
visible = displayNotifications || state.reserveSpace,
enter = fadeIn() + expandVertically(),
exit = fadeOut() + shrinkVertically(),
@ -95,7 +97,7 @@ fun TypingNotificationView(
}
@Composable
private fun computeTypingNotificationText(typingMembers: ImmutableList<RoomMember>): AnnotatedString {
private fun computeTypingNotificationText(typingMembers: ImmutableList<TypingRoomMember>): AnnotatedString {
// Remember the last value to avoid empty typing messages while animating
var result by remember { mutableStateOf(AnnotatedString("")) }
if (typingMembers.isNotEmpty()) {

View file

@ -0,0 +1,12 @@
/*
* Copyright 2024 New Vector Ltd.
*
* SPDX-License-Identifier: AGPL-3.0-only
* Please see LICENSE in the repository root for full details.
*/
package io.element.android.features.messages.impl.typing
data class TypingRoomMember(
val disambiguatedDisplayName: String,
)

View file

@ -8,7 +8,7 @@
package io.element.android.features.messages.impl.crypto.identity
import com.google.common.truth.Truth.assertThat
import io.element.android.features.messages.impl.typing.aTypingRoomMember
import io.element.android.libraries.designsystem.components.avatar.AvatarSize
import io.element.android.libraries.matrix.api.core.UserId
import io.element.android.libraries.matrix.api.encryption.EncryptionService
import io.element.android.libraries.matrix.api.encryption.identity.IdentityState
@ -19,6 +19,7 @@ import io.element.android.libraries.matrix.test.A_USER_ID
import io.element.android.libraries.matrix.test.A_USER_ID_2
import io.element.android.libraries.matrix.test.encryption.FakeEncryptionService
import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
import io.element.android.libraries.matrix.test.room.aRoomMember
import io.element.android.tests.testutils.WarmUpRule
import io.element.android.tests.testutils.lambda.lambdaRecorder
import io.element.android.tests.testutils.lambda.value
@ -59,7 +60,7 @@ class IdentityChangeStatePresenterTest {
val finalItem = awaitItem()
assertThat(finalItem.roomMemberIdentityStateChanges).hasSize(1)
val value = finalItem.roomMemberIdentityStateChanges.first()
assertThat(value.roomMember.userId).isEqualTo(A_USER_ID_2)
assertThat(value.identityRoomMember.userId).isEqualTo(A_USER_ID_2)
assertThat(value.identityState).isEqualTo(IdentityState.PinViolation)
}
}
@ -71,7 +72,7 @@ class IdentityChangeStatePresenterTest {
givenRoomMembersState(
MatrixRoomMembersState.Ready(
listOf(
aTypingRoomMember(
aRoomMember(
A_USER_ID_2,
displayName = "Alice",
),
@ -94,8 +95,9 @@ class IdentityChangeStatePresenterTest {
val finalItem = awaitItem()
assertThat(finalItem.roomMemberIdentityStateChanges).hasSize(1)
val value = finalItem.roomMemberIdentityStateChanges.first()
assertThat(value.roomMember.userId).isEqualTo(A_USER_ID_2)
assertThat(value.roomMember.displayName).isEqualTo("Alice")
assertThat(value.identityRoomMember.userId).isEqualTo(A_USER_ID_2)
assertThat(value.identityRoomMember.disambiguatedDisplayName).isEqualTo("Alice")
assertThat(value.identityRoomMember.avatarData.size).isEqualTo(AvatarSize.ComposerAlert)
assertThat(value.identityState).isEqualTo(IdentityState.PinViolation)
}
}

View file

@ -21,6 +21,7 @@ import io.element.android.libraries.matrix.test.A_USER_ID_3
import io.element.android.libraries.matrix.test.A_USER_ID_4
import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
import io.element.android.libraries.matrix.test.room.aRoomInfo
import io.element.android.libraries.matrix.test.room.aRoomMember
import io.element.android.libraries.preferences.api.store.SessionPreferencesStore
import io.element.android.libraries.preferences.test.InMemorySessionPreferencesStore
import io.element.android.tests.testutils.WarmUpRule
@ -49,7 +50,6 @@ class TypingNotificationPresenterTest {
@Test
fun `present - typing notification disabled`() = runTest {
val aDefaultRoomMember = createDefaultRoomMember(A_USER_ID_2)
val room = FakeMatrixRoom()
val sessionPreferencesStore = InMemorySessionPreferencesStore(
isRenderTypingNotificationsEnabled = false
@ -73,7 +73,11 @@ class TypingNotificationPresenterTest {
val oneMemberTypingState = awaitItem()
assertThat(oneMemberTypingState.renderTypingNotifications).isTrue()
assertThat(oneMemberTypingState.typingMembers.size).isEqualTo(1)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(aDefaultRoomMember)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(
TypingRoomMember(
disambiguatedDisplayName = A_USER_ID_2.value,
)
)
// Preferences changes again
sessionPreferencesStore.setRenderTypingNotifications(false)
skipItems(2)
@ -85,7 +89,6 @@ class TypingNotificationPresenterTest {
@Test
fun `present - state is updated when a member is typing, member is not known`() = runTest {
val aDefaultRoomMember = createDefaultRoomMember(A_USER_ID_2)
val room = FakeMatrixRoom()
val presenter = createPresenter(matrixRoom = room)
moleculeFlow(RecompositionMode.Immediate) {
@ -96,7 +99,11 @@ class TypingNotificationPresenterTest {
room.givenRoomTypingMembers(listOf(A_USER_ID_2))
val oneMemberTypingState = awaitItem()
assertThat(oneMemberTypingState.typingMembers.size).isEqualTo(1)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(aDefaultRoomMember)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(
TypingRoomMember(
disambiguatedDisplayName = A_USER_ID_2.value,
)
)
// User stops typing
room.givenRoomTypingMembers(emptyList())
skipItems(1)
@ -129,7 +136,11 @@ class TypingNotificationPresenterTest {
room.givenRoomTypingMembers(listOf(A_USER_ID_2))
val oneMemberTypingState = awaitItem()
assertThat(oneMemberTypingState.typingMembers.size).isEqualTo(1)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(aKnownRoomMember)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(
TypingRoomMember(
disambiguatedDisplayName = "Alice Doe (@bob:server.org)",
)
)
// User stops typing
room.givenRoomTypingMembers(emptyList())
skipItems(1)
@ -152,7 +163,11 @@ class TypingNotificationPresenterTest {
room.givenRoomTypingMembers(listOf(A_USER_ID_2))
val oneMemberTypingState = awaitItem()
assertThat(oneMemberTypingState.typingMembers.size).isEqualTo(1)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(aDefaultRoomMember)
assertThat(oneMemberTypingState.typingMembers.first()).isEqualTo(
TypingRoomMember(
disambiguatedDisplayName = A_USER_ID_2.value,
)
)
// User is getting known
room.givenRoomMembersState(
MatrixRoomMembersState.Ready(
@ -161,7 +176,11 @@ class TypingNotificationPresenterTest {
)
skipItems(1)
val finalState = awaitItem()
assertThat(finalState.typingMembers.first()).isEqualTo(aKnownRoomMember)
assertThat(finalState.typingMembers.first()).isEqualTo(
TypingRoomMember(
disambiguatedDisplayName = "Alice Doe (@bob:server.org)",
)
)
}
}
@ -204,7 +223,7 @@ class TypingNotificationPresenterTest {
private fun createDefaultRoomMember(
userId: UserId,
) = aTypingRoomMember(
) = aRoomMember(
userId = userId,
displayName = null,
isNameAmbiguous = false,
@ -212,7 +231,7 @@ class TypingNotificationPresenterTest {
private fun createKnownRoomMember(
userId: UserId,
) = aTypingRoomMember(
) = aRoomMember(
userId = userId,
displayName = "Alice Doe",
isNameAmbiguous = true,