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

bmarty · bmarty · commit 41749ed5b113 · 2024-10-08T14:54:49.000+02:00
Also remove TypingNotificationStateForMessagesProvider which was not used anymore.
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeState.kt
@@ -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,
 )
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenter.kt
@@ -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,
+    ),
+)
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateProvider.kt
@@ -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,
+)
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStateView.kt
@@ -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,
         )
     }
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityRoomMember.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityRoomMember.kt
@@ -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,
+)
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationPresenter.kt
@@ -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,
     )
 }
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationState.kt
@@ -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,
 )
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationStateForMessagesProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationStateForMessagesProvider.kt
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationStateProvider.kt
@@ -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,
+)
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationView.kt
@@ -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()) {
diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingRoomMember.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/typing/TypingRoomMember.kt
diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/crypto/identity/IdentityChangeStatePresenterTest.kt
diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/impl/typing/TypingNotificationPresenterTest.kt

Original file line number	Diff line number	Diff line change
`@@ -8,7 +8,6 @@`
`8`	`8`	`package io.element.android.features.messages.impl.crypto.identity`
`9`	`9`
`10`	`10`	`import io.element.android.libraries.matrix.api.encryption.identity.IdentityState`
`11`		`-import io.element.android.libraries.matrix.api.room.RoomMember`
`12`	`11`	`import kotlinx.collections.immutable.ImmutableList`
`13`	`12`
`14`	`13`	`data class IdentityChangeState(`
`@@ -17,6 +16,6 @@ data class IdentityChangeState(`
`17`	`16`	`)`
`18`	`17`
`19`	`18`	`data class RoomMemberIdentityStateChange(`
`20`		`- val roomMember: RoomMember,`
	`19`	`+ val identityRoomMember: IdentityRoomMember,`
`21`	`20`	`val identityState: IdentityState,`
`22`	`21`	`)`
Original file line number	Diff line number	Diff line change
`@@ -39,12 +39,12 @@ fun IdentityChangeStateView(`
`39`	`39`	`if (pinViolationIdentityChange != null) {`
`40`	`40`	`ComposerAlertMolecule(`
`41`	`41`	`modifier = modifier,`
`42`		`- avatar = pinViolationIdentityChange.roomMember.getAvatarData(AvatarSize.ComposerAlert),`
	`42`	`+ avatar = pinViolationIdentityChange.identityRoomMember.avatarData,`
`43`	`43`	`content = buildAnnotatedString {`
`44`	`44`	`val learnMoreStr = stringResource(CommonStrings.action_learn_more)`
`45`	`45`	`val fullText = stringResource(`
`46`	`46`	`id = CommonStrings.crypto_identity_change_pin_violation,`
`47`		`- pinViolationIdentityChange.roomMember.disambiguatedDisplayName,`
	`47`	`+ pinViolationIdentityChange.identityRoomMember.disambiguatedDisplayName,`
`48`	`48`	`learnMoreStr,`
`49`	`49`	`)`
`50`	`50`	`val learnMoreStartIndex = fullText.indexOf(learnMoreStr)`
`@@ -68,7 +68,7 @@ fun IdentityChangeStateView(`
`68`	`68`	`end = learnMoreStartIndex + learnMoreStr.length,`
`69`	`69`	`)`
`70`	`70`	`},`
`71`		`- onSubmitClick = { state.eventSink(IdentityChangeEvent.Submit(pinViolationIdentityChange.roomMember.userId)) },`
	`71`	`+ onSubmitClick = { state.eventSink(IdentityChangeEvent.Submit(pinViolationIdentityChange.identityRoomMember.userId)) },`
`72`	`72`	`isCritical = pinViolationIdentityChange.identityState == IdentityState.VerificationViolation,`
`73`	`73`	`)`
`74`	`74`	`}`