From 2f92203d85329876771fa6f75f4f6050aca6b3ba Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 21 Jul 2023 15:19:19 +0200 Subject: [PATCH] Room: avoid calling displayName/avatarData on each recomposition --- .../appnav/room/LoadingRoomNodeView.kt | 25 +-------- .../messages/impl/MessagesPresenter.kt | 14 +++-- .../features/messages/impl/MessagesState.kt | 4 +- .../messages/impl/MessagesStateProvider.kt | 8 ++- .../features/messages/impl/MessagesView.kt | 55 +++++++++++++------ .../IconTitlePlaceholdersRowMolecule.kt | 43 +++++++++++++++ 6 files changed, 99 insertions(+), 50 deletions(-) create mode 100644 libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/molecules/IconTitlePlaceholdersRowMolecule.kt diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomNodeView.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomNodeView.kt index e8d68a3e94..aa01b259de 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomNodeView.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomNodeView.kt @@ -16,19 +16,13 @@ package io.element.android.appnav.room -import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box import androidx.compose.foundation.layout.Column import androidx.compose.foundation.layout.ExperimentalLayoutApi -import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.consumeWindowInsets import androidx.compose.foundation.layout.fillMaxSize import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size -import androidx.compose.foundation.layout.width -import androidx.compose.foundation.shape.CircleShape import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable import androidx.compose.ui.Alignment @@ -37,9 +31,8 @@ import androidx.compose.ui.res.stringResource import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp -import androidx.compose.ui.unit.sp import io.element.android.features.networkmonitor.api.ui.ConnectivityIndicatorView -import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom +import io.element.android.libraries.designsystem.atomic.molecules.IconTitlePlaceholdersRowMolecule import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.components.button.BackButton import io.element.android.libraries.designsystem.preview.ElementPreviewDark @@ -48,7 +41,6 @@ import io.element.android.libraries.designsystem.theme.components.CircularProgre import io.element.android.libraries.designsystem.theme.components.Scaffold import io.element.android.libraries.designsystem.theme.components.Text import io.element.android.libraries.designsystem.theme.components.TopAppBar -import io.element.android.libraries.designsystem.theme.placeholderBackground import io.element.android.libraries.theme.ElementTheme import io.element.android.libraries.ui.strings.CommonStrings @@ -103,20 +95,7 @@ private fun LoadingRoomTopBar( BackButton(onClick = onBackClicked) }, title = { - Row( - verticalAlignment = Alignment.CenterVertically - ) { - Box( - modifier = Modifier - .size(AvatarSize.TimelineRoom.dp) - .align(Alignment.CenterVertically) - .background(color = ElementTheme.colors.placeholderBackground, shape = CircleShape) - ) - Spacer(modifier = Modifier.width(8.dp)) - PlaceholderAtom(width = 20.dp, height = 7.dp) - Spacer(modifier = Modifier.width(7.dp)) - PlaceholderAtom(width = 45.dp, height = 7.dp) - } + IconTitlePlaceholdersRowMolecule(iconSize = AvatarSize.TimelineRoom.dp) }, windowInsets = WindowInsets(0.dp), ) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt index a0a3e3a286..acaaf54c9e 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesPresenter.kt @@ -24,7 +24,6 @@ import androidx.compose.runtime.collectAsState import androidx.compose.runtime.derivedStateOf import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf -import androidx.compose.runtime.produceState import androidx.compose.runtime.remember import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.saveable.rememberSaveable @@ -76,6 +75,7 @@ import io.element.android.libraries.matrix.ui.room.canSendMessageAsState import io.element.android.libraries.textcomposer.MessageComposerMode import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import timber.log.Timber class MessagesPresenter @AssistedInject constructor( @@ -109,11 +109,13 @@ class MessagesPresenter @AssistedInject constructor( val syncUpdateFlow = room.syncUpdateFlow.collectAsState() val userHasPermissionToSendMessage by room.canSendMessageAsState(type = MessageEventType.ROOM_MESSAGE, updateKey = syncUpdateFlow.value) - val roomName by produceState(initialValue = room.displayName, key1 = syncUpdateFlow.value) { - value = room.displayName - } - val roomAvatar by produceState(initialValue = room.avatarData(), key1 = syncUpdateFlow.value) { - value = room.avatarData() + var roomName: Async by remember { mutableStateOf(Async.Uninitialized) } + var roomAvatar: Async by remember { mutableStateOf(Async.Uninitialized) } + LaunchedEffect(syncUpdateFlow.value) { + withContext(dispatchers.io) { + roomName = Async.Success(room.displayName) + roomAvatar = Async.Success(room.avatarData()) + } } var hasDismissedInviteDialog by rememberSaveable { mutableStateOf(false) diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesState.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesState.kt index 8a067a3a26..a042ec1ac4 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesState.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesState.kt @@ -30,8 +30,8 @@ import io.element.android.libraries.matrix.api.core.RoomId @Immutable data class MessagesState( val roomId: RoomId, - val roomName: String, - val roomAvatar: AvatarData, + val roomName: Async, + val roomAvatar: Async, val userHasPermissionToSendMessage: Boolean, val composerState: MessageComposerState, val timelineState: TimelineState, diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt index d0ddcf68f4..582cd2e7ab 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesStateProvider.kt @@ -34,6 +34,10 @@ open class MessagesStateProvider : PreviewParameterProvider { override val values: Sequence get() = sequenceOf( aMessagesState(), + aMessagesState().copy( + roomName = Async.Uninitialized, + roomAvatar = Async.Uninitialized, + ), aMessagesState().copy(hasNetworkConnection = false), aMessagesState().copy(composerState = aMessageComposerState().copy(showAttachmentSourcePicker = true)), aMessagesState().copy(userHasPermissionToSendMessage = false), @@ -43,8 +47,8 @@ open class MessagesStateProvider : PreviewParameterProvider { fun aMessagesState() = MessagesState( roomId = RoomId("!id:domain"), - roomName = "Room name", - roomAvatar = AvatarData("!id:domain", "Room name", size = AvatarSize.TimelineRoom), + roomName = Async.Success("Room name"), + roomAvatar = Async.Success(AvatarData("!id:domain", "Room name", size = AvatarSize.TimelineRoom)), userHasPermissionToSendMessage = true, composerState = aMessageComposerState().copy( text = "Hello", diff --git a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt index 6d8f2792e0..d8a8446c8c 100644 --- a/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt +++ b/features/messages/impl/src/main/kotlin/io/element/android/features/messages/impl/MessagesView.kt @@ -43,13 +43,11 @@ import androidx.compose.ui.Modifier import androidx.compose.ui.platform.LocalView import androidx.compose.ui.res.stringResource import androidx.compose.ui.text.font.FontStyle -import androidx.compose.ui.text.font.FontWeight import androidx.compose.ui.text.style.TextAlign import androidx.compose.ui.text.style.TextOverflow import androidx.compose.ui.tooling.preview.Preview import androidx.compose.ui.tooling.preview.PreviewParameter import androidx.compose.ui.unit.dp -import androidx.compose.ui.unit.sp import io.element.android.features.messages.impl.actionlist.ActionListEvents import io.element.android.features.messages.impl.actionlist.ActionListView import io.element.android.features.messages.impl.actionlist.model.TimelineItemAction @@ -64,10 +62,12 @@ import io.element.android.features.messages.impl.timeline.components.retrysendme import io.element.android.features.messages.impl.timeline.model.TimelineItem import io.element.android.features.networkmonitor.api.ui.ConnectivityIndicatorView import io.element.android.libraries.androidutils.ui.hideKeyboard +import io.element.android.libraries.designsystem.atomic.molecules.IconTitlePlaceholdersRowMolecule import io.element.android.libraries.designsystem.components.ProgressDialog import io.element.android.libraries.designsystem.components.ProgressDialogType 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.dialogs.ConfirmationDialog import io.element.android.libraries.designsystem.preview.ElementPreviewDark @@ -137,8 +137,8 @@ fun MessagesView( Column { ConnectivityIndicatorView(isOnline = state.hasNetworkConnection) MessagesViewTopBar( - roomTitle = state.roomName, - roomAvatar = state.roomAvatar, + roomName = state.roomName.dataOrNull(), + roomAvatar = state.roomAvatar.dataOrNull(), onBackPressed = onBackPressed, onRoomDetailsClicked = onRoomDetailsClicked, ) @@ -289,29 +289,50 @@ fun MessagesViewContent( @OptIn(ExperimentalMaterial3Api::class) @Composable fun MessagesViewTopBar( - roomTitle: String, - roomAvatar: AvatarData, + roomName: String?, + roomAvatar: AvatarData?, modifier: Modifier = Modifier, onRoomDetailsClicked: () -> Unit = {}, onBackPressed: () -> Unit = {}, ) { + @Composable + fun RoomAvatarAndNameRow( + roomName: String, + roomAvatar: AvatarData, + modifier: Modifier = Modifier + ) { + Row( + modifier = modifier, + verticalAlignment = Alignment.CenterVertically + ) { + Avatar(roomAvatar) + Spacer(modifier = Modifier.width(8.dp)) + Text( + text = roomName, + style = ElementTheme.typography.fontBodyLgMedium, + maxLines = 1, + overflow = TextOverflow.Ellipsis + ) + } + } + TopAppBar( modifier = modifier, navigationIcon = { BackButton(onClick = onBackPressed) }, title = { - Row( - modifier = Modifier.clickable { onRoomDetailsClicked() }, - verticalAlignment = Alignment.CenterVertically - ) { - Avatar(roomAvatar) - Spacer(modifier = Modifier.width(8.dp)) - Text( - text = roomTitle, - style = ElementTheme.typography.fontBodyLgMedium, - maxLines = 1, - overflow = TextOverflow.Ellipsis + val titleModifier = Modifier.clickable { onRoomDetailsClicked() } + if (roomName != null && roomAvatar != null) { + RoomAvatarAndNameRow( + roomName = roomName, + roomAvatar = roomAvatar, + modifier = titleModifier + ) + } else { + IconTitlePlaceholdersRowMolecule( + iconSize = AvatarSize.TimelineRoom.dp, + modifier = titleModifier ) } }, diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/molecules/IconTitlePlaceholdersRowMolecule.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/molecules/IconTitlePlaceholdersRowMolecule.kt new file mode 100644 index 0000000000..ea98a0283d --- /dev/null +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/molecules/IconTitlePlaceholdersRowMolecule.kt @@ -0,0 +1,43 @@ +package io.element.android.libraries.designsystem.atomic.molecules + +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Arrangement +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.width +import androidx.compose.foundation.shape.CircleShape +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.Dp +import androidx.compose.ui.unit.dp +import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom +import io.element.android.libraries.designsystem.theme.placeholderBackground +import io.element.android.libraries.theme.ElementTheme + +@Composable +fun IconTitlePlaceholdersRowMolecule( + iconSize: Dp, + modifier: Modifier = Modifier, + horizontalArrangement: Arrangement.Horizontal = Arrangement.Start, + verticalAlignment: Alignment.Vertical = Alignment.CenterVertically, +) { + Row( + modifier = modifier, + horizontalArrangement = horizontalArrangement, + verticalAlignment = verticalAlignment, + ) { + Box( + modifier = Modifier + .size(iconSize) + .align(Alignment.CenterVertically) + .background(color = ElementTheme.colors.placeholderBackground, shape = CircleShape) + ) + Spacer(modifier = Modifier.width(8.dp)) + PlaceholderAtom(width = 20.dp, height = 7.dp) + Spacer(modifier = Modifier.width(7.dp)) + PlaceholderAtom(width = 45.dp, height = 7.dp) + } +}