From d59f59e9f687ff66cf944eab23807f425189f79d Mon Sep 17 00:00:00 2001 From: ganfra Date: Tue, 4 Jul 2023 18:19:06 +0200 Subject: [PATCH 01/11] Await room: first attempt to wait for a room to be ready --- .../element/android/appnav/AwaitRoomNode.kt | 129 ++++++++++++++++++ .../android/appnav/LoggedInFlowNode.kt | 18 +-- .../impl/root/CreateRoomRootPresenter.kt | 29 ++-- .../leaveroom/impl/LeaveRoomPresenterImpl.kt | 2 +- .../roomlist/impl/RoomListPresenter.kt | 2 +- .../libraries/matrix/api/MatrixClient.kt | 6 +- .../matrix/api/room/RoomSummaryDataSource.kt | 2 +- .../libraries/matrix/impl/RustMatrixClient.kt | 31 ++++- .../matrix/impl/room/RoomListExtensions.kt | 5 +- .../impl/room/RustRoomSummaryDataSource.kt | 2 +- .../libraries/matrix/test/FakeMatrixClient.kt | 2 +- .../test/room/FakeRoomSummaryDataSource.kt | 2 +- 12 files changed, 186 insertions(+), 44 deletions(-) create mode 100644 appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt diff --git a/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt new file mode 100644 index 0000000000..6b8962e30c --- /dev/null +++ b/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt @@ -0,0 +1,129 @@ +/* + * 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.appnav + +import android.os.Parcelable +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.runtime.Composable +import androidx.compose.ui.Alignment +import androidx.compose.ui.Modifier +import androidx.lifecycle.lifecycleScope +import com.bumble.appyx.core.composable.Children +import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.node.Node +import com.bumble.appyx.core.node.node +import com.bumble.appyx.core.plugin.Plugin +import com.bumble.appyx.core.plugin.plugins +import com.bumble.appyx.navmodel.backstack.BackStack +import dagger.assisted.Assisted +import dagger.assisted.AssistedInject +import io.element.android.anvilannotations.ContributesNode +import io.element.android.libraries.architecture.BackstackNode +import io.element.android.libraries.architecture.NodeInputs +import io.element.android.libraries.architecture.createNode +import io.element.android.libraries.architecture.inputs +import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator +import io.element.android.libraries.di.SessionScope +import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.RoomId +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.asFlow +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.stateIn +import kotlinx.parcelize.Parcelize + +@ContributesNode(SessionScope::class) +class AwaitRoomNode @AssistedInject constructor( + @Assisted val buildContext: BuildContext, + @Assisted plugins: List, + private val matrixClient: MatrixClient, +) : + BackstackNode( + backstack = BackStack( + initialElement = NavTarget.Loading, + savedStateMap = buildContext.savedStateMap, + ), + buildContext = buildContext, + plugins = plugins + ) { + + data class Inputs( + val roomId: RoomId, + val initialElement: RoomFlowNode.NavTarget = RoomFlowNode.NavTarget.Messages, + ) : NodeInputs + + private val inputs: Inputs = inputs() + private val roomStateFlow = suspend { + matrixClient.getRoom(roomId = inputs.roomId) + } + .asFlow() + .stateIn(lifecycleScope, SharingStarted.Eagerly, null) + + sealed interface NavTarget : Parcelable { + @Parcelize + object Loading : NavTarget + + @Parcelize + object Loaded : NavTarget + } + + init { + roomStateFlow.onEach { room -> + if (room == null) { + backstack.safeRoot(NavTarget.Loading) + } else { + backstack.safeRoot(NavTarget.Loaded) + } + }.launchIn(lifecycleScope) + } + + override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { + return when (navTarget) { + NavTarget.Loaded -> { + val nodeLifecycleCallbacks = plugins() + val roomFlowNodeCallback = plugins() + val room = roomStateFlow.value + if (room == null) { + loadingNode(buildContext) + } else { + val inputs = RoomFlowNode.Inputs(room, initialElement = inputs.initialElement) + createNode(buildContext, plugins = listOf(inputs) + roomFlowNodeCallback + nodeLifecycleCallbacks) + } + } + NavTarget.Loading -> { + loadingNode(buildContext) + } + } + } + + private fun loadingNode(buildContext: BuildContext) = node(buildContext) { + Box(modifier = it.fillMaxSize(), contentAlignment = Alignment.Center) { + CircularProgressIndicator() + } + } + + @Composable + override fun View(modifier: Modifier) { + Children( + navModel = backstack, + modifier = modifier, + ) + } +} + diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index 4d6553f51c..ba3d228566 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -57,7 +57,6 @@ import io.element.android.libraries.architecture.bindings import io.element.android.libraries.architecture.createNode import io.element.android.libraries.architecture.inputs import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator -import io.element.android.libraries.designsystem.theme.components.Text import io.element.android.libraries.designsystem.utils.SnackbarDispatcher import io.element.android.libraries.di.AppScope import io.element.android.libraries.matrix.api.MatrixClient @@ -147,6 +146,7 @@ class LoggedInFlowNode @AssistedInject constructor( observeAnalyticsState() lifecycle.subscribe( onCreate = { + syncService.startSync() plugins().forEach { it.onFlowCreated(id, inputs.matrixClient) } val imageLoaderFactory = bindings().loggedInImageLoaderFactory() Coil.setImageLoader(imageLoaderFactory) @@ -267,24 +267,14 @@ class LoggedInFlowNode @AssistedInject constructor( .build() } is NavTarget.Room -> { - val room = inputs.matrixClient.getRoom(roomId = navTarget.roomId) - if (room == null) { - // TODO CREATE UNKNOWN ROOM NODE - node(buildContext) { - Box(modifier = it.fillMaxSize(), contentAlignment = Alignment.Center) { - Text(text = "Unknown room with id = ${navTarget.roomId}") - } - } - } else { val nodeLifecycleCallbacks = plugins() val callback = object : RoomFlowNode.Callback { override fun onForwardedToSingleRoom(roomId: RoomId) { coroutineScope.launch { attachRoom(roomId) } } } - val inputs = RoomFlowNode.Inputs(room, initialElement = navTarget.initialElement) - createNode(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks) - } + val inputs = AwaitRoomNode.Inputs(roomId = navTarget.roomId, initialElement = navTarget.initialElement) + createNode(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks) } NavTarget.Settings -> { val callback = object : PreferencesEntryPoint.Callback { @@ -342,7 +332,7 @@ class LoggedInFlowNode @AssistedInject constructor( } } - suspend fun attachRoom(roomId: RoomId): RoomFlowNode { + suspend fun attachRoom(roomId: RoomId): AwaitRoomNode { return attachChild { backstack.singleTop(NavTarget.RoomList) backstack.push(NavTarget.Room(roomId)) diff --git a/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenter.kt b/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenter.kt index 67bf22d22a..20d3309a5f 100644 --- a/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenter.kt +++ b/features/createroom/impl/src/main/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenter.kt @@ -65,20 +65,9 @@ class CreateRoomRootPresenter @Inject constructor( val localCoroutineScope = rememberCoroutineScope() val startDmAction: MutableState> = remember { mutableStateOf(Async.Uninitialized) } - fun startDm(matrixUser: MatrixUser) { - startDmAction.value = Async.Uninitialized - matrixClient.findDM(matrixUser.userId).use { existingDM -> - if (existingDM == null) { - localCoroutineScope.createDM(matrixUser, startDmAction) - } else { - startDmAction.value = Async.Success(existingDM.roomId) - } - } - } - fun handleEvents(event: CreateRoomRootEvents) { when (event) { - is CreateRoomRootEvents.StartDM -> startDm(event.matrixUser) + is CreateRoomRootEvents.StartDM -> localCoroutineScope.startDm(event.matrixUser, startDmAction) CreateRoomRootEvents.CancelStartDM -> startDmAction.value = Async.Uninitialized } } @@ -91,10 +80,20 @@ class CreateRoomRootPresenter @Inject constructor( ) } - private fun CoroutineScope.createDM(user: MatrixUser, startDmAction: MutableState>) = launch { + private fun CoroutineScope.startDm(matrixUser: MatrixUser, startDmAction: MutableState>) = launch { suspend { - matrixClient.createDM(user.userId).getOrThrow() - .also { analyticsService.capture(CreatedRoom(isDM = true)) } + matrixClient.findDM(matrixUser.userId).use { existingDM -> + existingDM?.roomId ?: createDM(matrixUser) + } }.runCatchingUpdatingState(startDmAction) } + + private suspend fun createDM(user: MatrixUser): RoomId { + return matrixClient + .createDM(user.userId) + .onSuccess { + analyticsService.capture(CreatedRoom(isDM = true)) + } + .getOrThrow() + } } diff --git a/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt b/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt index 4978a87815..68a8d80fe6 100644 --- a/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt +++ b/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt @@ -78,7 +78,7 @@ class LeaveRoomPresenterImpl @Inject constructor( } } -private fun showLeaveRoomAlert( +private suspend fun showLeaveRoomAlert( matrixClient: MatrixClient, roomId: RoomId, confirmation: MutableState, diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt index 7d752ea8b6..aa44b4f001 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/RoomListPresenter.kt @@ -172,7 +172,7 @@ class RoomListPresenter @Inject constructor( // Safe to give bigger size than room list val extendedRangeEnd = range.last + midExtendedRangeSize val extendedRange = IntRange(extendedRangeStart, extendedRangeEnd) - client.roomSummaryDataSource.updateRoomListVisibleRange(extendedRange) + client.roomSummaryDataSource.updateAllRoomsVisibleRange(extendedRange) } private suspend fun mapRoomSummaries( diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt index 84855adb96..ca03263618 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/MatrixClient.kt @@ -31,14 +31,16 @@ import io.element.android.libraries.matrix.api.sync.SyncService import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults import io.element.android.libraries.matrix.api.user.MatrixUser import io.element.android.libraries.matrix.api.verification.SessionVerificationService +import kotlinx.coroutines.TimeoutCancellationException import java.io.Closeable +import kotlin.time.Duration interface MatrixClient : Closeable { val sessionId: SessionId val roomSummaryDataSource: RoomSummaryDataSource val mediaLoader: MatrixMediaLoader - fun getRoom(roomId: RoomId): MatrixRoom? - fun findDM(userId: UserId): MatrixRoom? + suspend fun getRoom(roomId: RoomId): MatrixRoom? + suspend fun findDM(userId: UserId): MatrixRoom? suspend fun ignoreUser(userId: UserId): Result suspend fun unignoreUser(userId: UserId): Result suspend fun createRoom(createRoomParams: CreateRoomParameters): Result diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt index e0aaecd6a9..d8e67a40c3 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt @@ -25,8 +25,8 @@ interface RoomSummaryDataSource { data class Loaded(val numberOfRooms: Int): LoadingState() } + fun updateAllRoomsVisibleRange(range: IntRange) fun allRoomsLoadingState(): StateFlow fun allRooms(): StateFlow> fun inviteRooms(): StateFlow> - fun updateRoomListVisibleRange(range: IntRange) } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index 6cfa71d097..84a5a38784 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -57,12 +57,15 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancel import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.withContext import kotlinx.coroutines.withTimeout import org.matrix.rustcomponents.sdk.Client import org.matrix.rustcomponents.sdk.ClientDelegate +import org.matrix.rustcomponents.sdk.Room +import org.matrix.rustcomponents.sdk.RoomListItem import org.matrix.rustcomponents.sdk.use import timber.log.Timber import java.io.File @@ -91,7 +94,6 @@ class RustMatrixClient constructor( ) private val notificationService = RustNotificationService(client) - private val clientDelegate = object : ClientDelegate { override fun didReceiveAuthError(isSoftLogout: Boolean) { //TODO handle this @@ -127,9 +129,16 @@ class RustMatrixClient constructor( }.launchIn(sessionCoroutineScope) } - override fun getRoom(roomId: RoomId): MatrixRoom? { - val roomListItem = roomListService.roomOrNull(roomId.value) ?: return null - val fullRoom = roomListItem.fullRoom() + override suspend fun getRoom(roomId: RoomId): MatrixRoom? { + var cachedPairOfRoom = pairOfRoom(roomId) + if (cachedPairOfRoom == null) { + roomSummaryDataSource.allRoomsLoadingState().firstOrNull { + it is RoomSummaryDataSource.LoadingState.Loaded + } + cachedPairOfRoom = pairOfRoom(roomId) + } + if (cachedPairOfRoom == null) return null + val (roomListItem, fullRoom) = cachedPairOfRoom return RustMatrixRoom( sessionId = sessionId, roomListItem = roomListItem, @@ -141,7 +150,19 @@ class RustMatrixClient constructor( ) } - override fun findDM(userId: UserId): MatrixRoom? { + private suspend fun pairOfRoom(roomId: RoomId): Pair? { + Timber.v("Resume get pair of room for $roomId") + val cachedRoomListItem = roomListService.roomOrNull(roomId.value) + val fullRoom = cachedRoomListItem?.fullRoom() + Timber.v("Finish get pair of room for $roomId") + return if (cachedRoomListItem == null || fullRoom == null) { + null + } else { + Pair(cachedRoomListItem, fullRoom) + } + } + + override suspend fun findDM(userId: UserId): MatrixRoom? { val roomId = client.getDmRoom(userId.value)?.use { RoomId(it.id()) } return roomId?.let { getRoom(it) } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomListExtensions.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomListExtensions.kt index a6569158d0..824d477fd3 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomListExtensions.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RoomListExtensions.kt @@ -25,6 +25,7 @@ import org.matrix.rustcomponents.sdk.RoomList import org.matrix.rustcomponents.sdk.RoomListEntriesListener import org.matrix.rustcomponents.sdk.RoomListEntriesUpdate import org.matrix.rustcomponents.sdk.RoomListEntry +import org.matrix.rustcomponents.sdk.RoomListException import org.matrix.rustcomponents.sdk.RoomListItem import org.matrix.rustcomponents.sdk.RoomListLoadingState import org.matrix.rustcomponents.sdk.RoomListLoadingStateListener @@ -60,8 +61,8 @@ fun RoomList.entriesFlow(onInitialList: suspend (List) -> Unit): fun RoomListService.roomOrNull(roomId: String): RoomListItem? { return try { room(roomId) - } catch (failure: Throwable) { - Timber.e(failure, "Failed finding room with id=$roomId") + } catch (exception: RoomListException) { + Timber.e(exception, "Failed finding room with id=$roomId") return null } } diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomSummaryDataSource.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomSummaryDataSource.kt index 29e3989563..3da4c560e8 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomSummaryDataSource.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustRoomSummaryDataSource.kt @@ -90,7 +90,7 @@ internal class RustRoomSummaryDataSource( return allRoomsLoadingState } - override fun updateRoomListVisibleRange(range: IntRange) { + override fun updateAllRoomsVisibleRange(range: IntRange) { Timber.v("setVisibleRange=$range") sessionCoroutineScope.launch { try { diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt index cfc6e14fd8..4e43df2d2c 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt @@ -68,7 +68,7 @@ class FakeMatrixClient( return getRoomResults[roomId] } - override fun findDM(userId: UserId): MatrixRoom? { + override suspend fun findDM(userId: UserId): MatrixRoom? { return findDmResult } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeRoomSummaryDataSource.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeRoomSummaryDataSource.kt index 87169aa498..cae36e14c8 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeRoomSummaryDataSource.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeRoomSummaryDataSource.kt @@ -54,7 +54,7 @@ class FakeRoomSummaryDataSource : RoomSummaryDataSource { var latestSlidingSyncRange: IntRange? = null private set - override fun updateRoomListVisibleRange(range: IntRange) { + override fun updateAllRoomsVisibleRange(range: IntRange) { latestSlidingSyncRange = range } } From 2dcd94076f353d472c4175c13a443634c6be3b17 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 5 Jul 2023 12:01:51 +0200 Subject: [PATCH 02/11] Room: remove bestName and use displayName instead of name where it makes sense --- .../leaveroom/impl/LeaveRoomPresenterImpl.kt | 2 +- .../messages/impl/MessagesPresenter.kt | 33 +++++++++---------- .../features/messages/impl/MessagesState.kt | 4 +-- .../features/messages/impl/MessagesView.kt | 12 +++---- .../roomdetails/impl/RoomDetailsPresenter.kt | 2 +- .../roomdetails/RoomDetailsPresenterTests.kt | 2 +- .../libraries/matrix/api/room/MatrixRoom.kt | 1 - .../matrix/impl/room/RustMatrixRoom.kt | 5 --- .../matrix/test/room/FakeMatrixRoom.kt | 1 - 9 files changed, 26 insertions(+), 36 deletions(-) diff --git a/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt b/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt index 68a8d80fe6..2ad35b38c9 100644 --- a/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt +++ b/features/leaveroom/impl/src/main/kotlin/io/element/android/features/leaveroom/impl/LeaveRoomPresenterImpl.kt @@ -105,7 +105,7 @@ private suspend fun MatrixClient.leaveRoom( room.leave().onSuccess { roomMembershipObserver.notifyUserLeftRoom(room.roomId) }.onFailure { - Timber.e(it, "Error while leaving room ${room.name} - ${room.roomId}") + Timber.e(it, "Error while leaving room ${room.displayName} - ${room.roomId}") error.value = LeaveRoomState.Error.Shown } } 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 f2c8deca49..ae6b65f076 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,6 +24,7 @@ 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 @@ -107,13 +108,12 @@ class MessagesPresenter @AssistedInject constructor( val syncUpdateFlow = room.syncUpdateFlow.collectAsState() val userHasPermissionToSendMessage by room.canSendEventAsState(type = MessageEventType.ROOM_MESSAGE, updateKey = syncUpdateFlow.value) - val roomName: MutableState = rememberSaveable { - mutableStateOf(null) + val roomName by produceState(initialValue = room.displayName, key1 = syncUpdateFlow.value){ + value = room.displayName } - val roomAvatar: MutableState = remember { - mutableStateOf(null) + val roomAvatar by produceState(initialValue = room.avatarData(), key1 = syncUpdateFlow.value){ + value = room.avatarData() } - var hasDismissedInviteDialog by rememberSaveable { mutableStateOf(false) } @@ -134,16 +134,6 @@ class MessagesPresenter @AssistedInject constructor( val snackbarMessage by snackbarDispatcher.collectSnackbarMessageAsState() - LaunchedEffect(syncUpdateFlow.value) { - roomAvatar.value = - AvatarData( - id = room.roomId.value, - name = room.name, - url = room.avatarUrl, - size = AvatarSize.TimelineRoom - ) - roomName.value = room.name - } LaunchedEffect(composerState.mode.relatedEventId) { timelineState.eventSink(TimelineEvents.SetHighlightedEvent(composerState.mode.relatedEventId)) } @@ -169,8 +159,8 @@ class MessagesPresenter @AssistedInject constructor( return MessagesState( roomId = room.roomId, - roomName = roomName.value, - roomAvatar = roomAvatar.value, + roomName = roomName, + roomAvatar = roomAvatar, userHasPermissionToSendMessage = userHasPermissionToSendMessage, composerState = composerState, timelineState = timelineState, @@ -185,6 +175,15 @@ class MessagesPresenter @AssistedInject constructor( ) } + private fun MatrixRoom.avatarData(): AvatarData { + return AvatarData( + id = room.roomId.value, + name = room.displayName, + url = room.avatarUrl, + size = AvatarSize.TimelineRoom + ) + } + private fun CoroutineScope.handleTimelineAction( action: TimelineItemAction, targetEvent: TimelineItem.Event, 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 29fbeaedd9..8a067a3a26 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: String, + val roomAvatar: AvatarData, val userHasPermissionToSendMessage: Boolean, val composerState: MessageComposerState, val timelineState: TimelineState, 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 037199aba8..87ee434598 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 @@ -277,8 +277,8 @@ fun MessagesViewContent( @OptIn(ExperimentalMaterial3Api::class) @Composable fun MessagesViewTopBar( - roomTitle: String?, - roomAvatar: AvatarData?, + roomTitle: String, + roomAvatar: AvatarData, modifier: Modifier = Modifier, onRoomDetailsClicked: () -> Unit = {}, onBackPressed: () -> Unit = {}, @@ -293,14 +293,12 @@ fun MessagesViewTopBar( modifier = Modifier.clickable { onRoomDetailsClicked() }, verticalAlignment = Alignment.CenterVertically ) { - if (roomAvatar != null) { - Avatar(roomAvatar) - Spacer(modifier = Modifier.width(8.dp)) - } + Avatar(roomAvatar) + Spacer(modifier = Modifier.width(8.dp)) Text( fontSize = 16.sp, fontWeight = FontWeight.SemiBold, - text = roomTitle ?: "Unknown room", + text = roomTitle, maxLines = 1, overflow = TextOverflow.Ellipsis ) diff --git a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt index 944049d163..2c70e66ff6 100644 --- a/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt +++ b/features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/RoomDetailsPresenter.kt @@ -78,7 +78,7 @@ class RoomDetailsPresenter @Inject constructor( return RoomDetailsState( roomId = room.roomId.value, - roomName = room.name ?: room.displayName, + roomName = room.displayName, roomAlias = room.alias, roomAvatarUrl = room.avatarUrl, roomTopic = topicState, diff --git a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt index f85942c3ee..f9d947615a 100644 --- a/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt +++ b/features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/RoomDetailsPresenterTests.kt @@ -62,7 +62,7 @@ class RoomDetailsPresenterTests { }.test { val initialState = awaitItem() assertThat(initialState.roomId).isEqualTo(room.roomId.value) - assertThat(initialState.roomName).isEqualTo(room.name) + assertThat(initialState.roomName).isEqualTo(room.displayName) assertThat(initialState.roomAvatarUrl).isEqualTo(room.avatarUrl) assertThat(initialState.roomTopic).isEqualTo(RoomTopicState.ExistingTopic(room.topic!!)) assertThat(initialState.memberCount).isEqualTo(room.joinedMemberCount) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt index 12ffde3cd9..3960f74723 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/MatrixRoom.kt @@ -35,7 +35,6 @@ interface MatrixRoom : Closeable { val sessionId: SessionId val roomId: RoomId val name: String? - val bestName: String val displayName: String val alias: String? val alternativeAliases: List diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt index 680bed0cac..cc3a03e95f 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/room/RustMatrixRoom.kt @@ -130,11 +130,6 @@ class RustMatrixRoom( return roomListItem.name() } - override val bestName: String - get() { - return name?.takeIf { it.isNotEmpty() } ?: innerRoom.id() - } - override val displayName: String get() { return innerRoom.displayName() diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt index e50fe5441e..be4cd4cbed 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/room/FakeMatrixRoom.kt @@ -43,7 +43,6 @@ class FakeMatrixRoom( override val sessionId: SessionId = A_SESSION_ID, override val roomId: RoomId = A_ROOM_ID, override val name: String? = null, - override val bestName: String = "", override val displayName: String = "", override val topic: String? = null, override val avatarUrl: String? = null, From 27df1f21124d0d85368a9a3b27b9eba4b4fbccf8 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 5 Jul 2023 12:42:01 +0200 Subject: [PATCH 03/11] AwaitRoom : create loading state with placeholders --- .../element/android/appnav/AwaitRoomNode.kt | 72 ++++++++++++++++--- .../android/appnav/LoggedInFlowNode.kt | 14 ++-- .../components/RoomSummaryPlaceholderRow.kt | 4 +- .../atomic/atoms/PlaceholderAtom.kt | 4 +- .../designsystem/theme/ColorAliases.kt | 4 +- .../components/MatrixUserHeaderPlaceholder.kt | 4 +- 6 files changed, 79 insertions(+), 23 deletions(-) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt index 6b8962e30c..0da87e046a 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt @@ -14,14 +14,27 @@ * limitations under the License. */ +@file:OptIn(ExperimentalMaterial3Api::class) + package io.element.android.appnav import android.os.Parcelable +import androidx.compose.foundation.background import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.WindowInsets import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.systemBars +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 import androidx.compose.ui.Modifier +import androidx.compose.ui.unit.dp import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.composable.Children import com.bumble.appyx.core.modality.BuildContext @@ -30,6 +43,7 @@ import com.bumble.appyx.core.node.node import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.core.plugin.plugins import com.bumble.appyx.navmodel.backstack.BackStack +import com.bumble.appyx.navmodel.backstack.operation.newRoot import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode @@ -37,10 +51,17 @@ import io.element.android.libraries.architecture.BackstackNode import io.element.android.libraries.architecture.NodeInputs import io.element.android.libraries.architecture.createNode import io.element.android.libraries.architecture.inputs +import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom +import io.element.android.libraries.designsystem.components.avatar.AvatarSize +import io.element.android.libraries.designsystem.components.button.BackButton import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator +import io.element.android.libraries.designsystem.theme.components.Scaffold +import io.element.android.libraries.designsystem.theme.components.TopAppBar +import io.element.android.libraries.designsystem.theme.placeholderBackground import io.element.android.libraries.di.SessionScope import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.RoomId +import io.element.android.libraries.theme.ElementTheme import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.launchIn @@ -86,9 +107,9 @@ class AwaitRoomNode @AssistedInject constructor( init { roomStateFlow.onEach { room -> if (room == null) { - backstack.safeRoot(NavTarget.Loading) + backstack.newRoot(NavTarget.Loading) } else { - backstack.safeRoot(NavTarget.Loaded) + backstack.newRoot(NavTarget.Loaded) } }.launchIn(lifecycleScope) } @@ -100,22 +121,57 @@ class AwaitRoomNode @AssistedInject constructor( val roomFlowNodeCallback = plugins() val room = roomStateFlow.value if (room == null) { - loadingNode(buildContext) + loadingNode(buildContext, this::navigateUp) } else { val inputs = RoomFlowNode.Inputs(room, initialElement = inputs.initialElement) createNode(buildContext, plugins = listOf(inputs) + roomFlowNodeCallback + nodeLifecycleCallbacks) } } NavTarget.Loading -> { - loadingNode(buildContext) + loadingNode(buildContext, this::navigateUp) } } } - private fun loadingNode(buildContext: BuildContext) = node(buildContext) { - Box(modifier = it.fillMaxSize(), contentAlignment = Alignment.Center) { - CircularProgressIndicator() - } + private fun loadingNode(buildContext: BuildContext, onBackPressed: () -> Unit) = node(buildContext) { modifier -> + Scaffold( + modifier = modifier, + contentWindowInsets = WindowInsets.systemBars, + topBar = { + TopAppBar( + modifier = Modifier, + navigationIcon = { + BackButton(onClick = onBackPressed) + }, + 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) + } + }, + ) + }, + content = { padding -> + Box( + modifier = Modifier + .fillMaxSize() + .padding(padding), contentAlignment = Alignment.Center + ) { + CircularProgressIndicator() + } + }, + ) + } @Composable diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index ba3d228566..fb9c5162df 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -267,14 +267,14 @@ class LoggedInFlowNode @AssistedInject constructor( .build() } is NavTarget.Room -> { - val nodeLifecycleCallbacks = plugins() - val callback = object : RoomFlowNode.Callback { - override fun onForwardedToSingleRoom(roomId: RoomId) { - coroutineScope.launch { attachRoom(roomId) } - } + val nodeLifecycleCallbacks = plugins() + val callback = object : RoomFlowNode.Callback { + override fun onForwardedToSingleRoom(roomId: RoomId) { + coroutineScope.launch { attachRoom(roomId) } } - val inputs = AwaitRoomNode.Inputs(roomId = navTarget.roomId, initialElement = navTarget.initialElement) - createNode(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks) + } + val inputs = AwaitRoomNode.Inputs(roomId = navTarget.roomId, initialElement = navTarget.initialElement) + createNode(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks) } NavTarget.Settings -> { val callback = object : PreferencesEntryPoint.Callback { diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/components/RoomSummaryPlaceholderRow.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/components/RoomSummaryPlaceholderRow.kt index 8ce5be0a85..0261f1bc16 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/components/RoomSummaryPlaceholderRow.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/components/RoomSummaryPlaceholderRow.kt @@ -36,7 +36,7 @@ import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.preview.ElementPreviewDark import io.element.android.libraries.designsystem.preview.ElementPreviewLight -import io.element.android.libraries.designsystem.theme.roomListPlaceholder +import io.element.android.libraries.designsystem.theme.placeholderBackground import io.element.android.libraries.theme.ElementTheme /** @@ -56,7 +56,7 @@ internal fun RoomSummaryPlaceholderRow( modifier = Modifier .size(AvatarSize.RoomListItem.dp) .align(Alignment.CenterVertically) - .background(color = ElementTheme.colors.roomListPlaceholder, shape = CircleShape) + .background(color = ElementTheme.colors.placeholderBackground, shape = CircleShape) ) Column( modifier = Modifier diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/PlaceholderAtom.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/PlaceholderAtom.kt index 250177399e..75ed007d97 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/PlaceholderAtom.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/atomic/atoms/PlaceholderAtom.kt @@ -29,7 +29,7 @@ import androidx.compose.ui.unit.Dp import androidx.compose.ui.unit.dp import io.element.android.libraries.designsystem.preview.ElementPreviewDark import io.element.android.libraries.designsystem.preview.ElementPreviewLight -import io.element.android.libraries.designsystem.theme.roomListPlaceholder +import io.element.android.libraries.designsystem.theme.placeholderBackground import io.element.android.libraries.theme.ElementTheme @Composable @@ -37,7 +37,7 @@ fun PlaceholderAtom( width: Dp, height: Dp, modifier: Modifier = Modifier, - color: Color = ElementTheme.colors.roomListPlaceholder, + color: Color = ElementTheme.colors.placeholderBackground, ) { Box( modifier = modifier diff --git a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/ColorAliases.kt b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/ColorAliases.kt index 3514928938..563b9d903a 100644 --- a/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/ColorAliases.kt +++ b/libraries/designsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/ColorAliases.kt @@ -42,7 +42,7 @@ fun MaterialTheme.roomListRoomMessageDate() = colorScheme.secondary val SemanticColors.unreadIndicator get() = iconAccentTertiary -val SemanticColors.roomListPlaceholder +val SemanticColors.placeholderBackground get() = bgSubtleSecondary // This color is not present in Semantic color, so put hard-coded value for now @@ -83,7 +83,7 @@ private fun ContentToPreview() { "roomListRoomMessage" to MaterialTheme.roomListRoomMessage(), "roomListRoomMessageDate" to MaterialTheme.roomListRoomMessageDate(), "unreadIndicator" to ElementTheme.colors.unreadIndicator, - "roomListPlaceholder" to ElementTheme.colors.roomListPlaceholder, + "placeholderBackground" to ElementTheme.colors.placeholderBackground, "messageFromMeBackground" to ElementTheme.colors.messageFromMeBackground, "messageFromOtherBackground" to ElementTheme.colors.messageFromOtherBackground, ) diff --git a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserHeaderPlaceholder.kt b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserHeaderPlaceholder.kt index b43fc36fa6..57901edc03 100644 --- a/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserHeaderPlaceholder.kt +++ b/libraries/matrixui/src/main/kotlin/io/element/android/libraries/matrix/ui/components/MatrixUserHeaderPlaceholder.kt @@ -36,7 +36,7 @@ import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.preview.ElementPreviewDark import io.element.android.libraries.designsystem.preview.ElementPreviewLight -import io.element.android.libraries.designsystem.theme.roomListPlaceholder +import io.element.android.libraries.designsystem.theme.placeholderBackground import io.element.android.libraries.theme.ElementTheme @Composable @@ -53,7 +53,7 @@ fun MatrixUserHeaderPlaceholder( modifier = Modifier .padding(vertical = 12.dp) .size(AvatarSize.UserPreference.dp) - .background(color = ElementTheme.colors.roomListPlaceholder, shape = CircleShape) + .background(color = ElementTheme.colors.placeholderBackground, shape = CircleShape) ) Spacer(modifier = Modifier.width(16.dp)) Column( From 11f236aba531a1df16182adf2dfbe84a553ee947 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 5 Jul 2023 12:56:39 +0200 Subject: [PATCH 04/11] Room: dispatch getRooms on background thread --- .../android/libraries/matrix/impl/RustMatrixClient.kt | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index 84a5a38784..85aef441d2 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -55,6 +55,7 @@ import io.element.android.services.toolbox.api.systemclock.SystemClock import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancel +import kotlinx.coroutines.delay import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.firstOrNull @@ -150,12 +151,10 @@ class RustMatrixClient constructor( ) } - private suspend fun pairOfRoom(roomId: RoomId): Pair? { - Timber.v("Resume get pair of room for $roomId") + private suspend fun pairOfRoom(roomId: RoomId): Pair? = withContext(dispatchers.io) { val cachedRoomListItem = roomListService.roomOrNull(roomId.value) val fullRoom = cachedRoomListItem?.fullRoom() - Timber.v("Finish get pair of room for $roomId") - return if (cachedRoomListItem == null || fullRoom == null) { + if (cachedRoomListItem == null || fullRoom == null) { null } else { Pair(cachedRoomListItem, fullRoom) From 43b4351732a0c5c0748473975b74aade97eeaf09 Mon Sep 17 00:00:00 2001 From: ganfra Date: Wed, 5 Jul 2023 16:10:47 +0200 Subject: [PATCH 05/11] Room : small refactoring --- .../kotlin/io/element/android/x/MainNode.kt | 2 +- .../android/appnav/LoggedInFlowNode.kt | 2 + .../appnav/{ => room}/AwaitRoomNode.kt | 36 +++++-------- .../android/appnav/room/AwaitRoomState.kt | 53 +++++++++++++++++++ .../android/appnav/{ => room}/RoomFlowNode.kt | 3 +- .../android/appnav/RoomFlowNodeTest.kt | 1 + 6 files changed, 73 insertions(+), 24 deletions(-) rename appnav/src/main/kotlin/io/element/android/appnav/{ => room}/AwaitRoomNode.kt (87%) create mode 100644 appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomState.kt rename appnav/src/main/kotlin/io/element/android/appnav/{ => room}/RoomFlowNode.kt (98%) diff --git a/app/src/main/kotlin/io/element/android/x/MainNode.kt b/app/src/main/kotlin/io/element/android/x/MainNode.kt index 7f56ef4d63..fc4e30e749 100644 --- a/app/src/main/kotlin/io/element/android/x/MainNode.kt +++ b/app/src/main/kotlin/io/element/android/x/MainNode.kt @@ -28,7 +28,7 @@ import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode import com.bumble.appyx.core.plugin.Plugin import io.element.android.appnav.LoggedInFlowNode -import io.element.android.appnav.RoomFlowNode +import io.element.android.appnav.room.RoomFlowNode import io.element.android.appnav.RootFlowNode import io.element.android.libraries.architecture.bindings import io.element.android.libraries.architecture.createNode diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index fb9c5162df..369b3054fb 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -42,6 +42,8 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode import io.element.android.appnav.loggedin.LoggedInNode +import io.element.android.appnav.room.AwaitRoomNode +import io.element.android.appnav.room.RoomFlowNode import io.element.android.features.analytics.api.AnalyticsEntryPoint import io.element.android.features.createroom.api.CreateRoomEntryPoint import io.element.android.features.invitelist.api.InviteListEntryPoint diff --git a/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomNode.kt similarity index 87% rename from appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt rename to appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomNode.kt index 0da87e046a..bd894a4aeb 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/AwaitRoomNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomNode.kt @@ -16,7 +16,7 @@ @file:OptIn(ExperimentalMaterial3Api::class) -package io.element.android.appnav +package io.element.android.appnav.room import android.os.Parcelable import androidx.compose.foundation.background @@ -43,10 +43,11 @@ import com.bumble.appyx.core.node.node import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.core.plugin.plugins import com.bumble.appyx.navmodel.backstack.BackStack -import com.bumble.appyx.navmodel.backstack.operation.newRoot import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode +import io.element.android.appnav.NodeLifecycleCallback +import io.element.android.appnav.safeRoot import io.element.android.libraries.architecture.BackstackNode import io.element.android.libraries.architecture.NodeInputs import io.element.android.libraries.architecture.createNode @@ -59,21 +60,17 @@ import io.element.android.libraries.designsystem.theme.components.Scaffold import io.element.android.libraries.designsystem.theme.components.TopAppBar import io.element.android.libraries.designsystem.theme.placeholderBackground import io.element.android.libraries.di.SessionScope -import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.theme.ElementTheme -import kotlinx.coroutines.flow.SharingStarted -import kotlinx.coroutines.flow.asFlow import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.flow.stateIn import kotlinx.parcelize.Parcelize @ContributesNode(SessionScope::class) class AwaitRoomNode @AssistedInject constructor( @Assisted val buildContext: BuildContext, @Assisted plugins: List, - private val matrixClient: MatrixClient, + awaitRoomStateFlowFactory: AwaitRoomStateFlowFactory, ) : BackstackNode( backstack = BackStack( @@ -90,11 +87,7 @@ class AwaitRoomNode @AssistedInject constructor( ) : NodeInputs private val inputs: Inputs = inputs() - private val roomStateFlow = suspend { - matrixClient.getRoom(roomId = inputs.roomId) - } - .asFlow() - .stateIn(lifecycleScope, SharingStarted.Eagerly, null) + private val awaitRoomStateFlow = awaitRoomStateFlowFactory.create(lifecycleScope, inputs.roomId) sealed interface NavTarget : Parcelable { @Parcelize @@ -105,11 +98,10 @@ class AwaitRoomNode @AssistedInject constructor( } init { - roomStateFlow.onEach { room -> - if (room == null) { - backstack.newRoot(NavTarget.Loading) - } else { - backstack.newRoot(NavTarget.Loaded) + awaitRoomStateFlow.onEach { awaitRoomState -> + when (awaitRoomState) { + is AwaitRoomState.Loaded -> backstack.safeRoot(NavTarget.Loaded) + else -> backstack.safeRoot(NavTarget.Loading) } }.launchIn(lifecycleScope) } @@ -119,12 +111,12 @@ class AwaitRoomNode @AssistedInject constructor( NavTarget.Loaded -> { val nodeLifecycleCallbacks = plugins() val roomFlowNodeCallback = plugins() - val room = roomStateFlow.value - if (room == null) { - loadingNode(buildContext, this::navigateUp) - } else { - val inputs = RoomFlowNode.Inputs(room, initialElement = inputs.initialElement) + val awaitRoomState = awaitRoomStateFlow.value + if (awaitRoomState is AwaitRoomState.Loaded) { + val inputs = RoomFlowNode.Inputs(awaitRoomState.room, initialElement = inputs.initialElement) createNode(buildContext, plugins = listOf(inputs) + roomFlowNodeCallback + nodeLifecycleCallbacks) + } else { + loadingNode(buildContext, this::navigateUp) } } NavTarget.Loading -> { diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomState.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomState.kt new file mode 100644 index 0000000000..a0f0e1cd33 --- /dev/null +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomState.kt @@ -0,0 +1,53 @@ +/* + * 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.appnav.room + +import io.element.android.libraries.di.SessionScope +import io.element.android.libraries.di.SingleIn +import io.element.android.libraries.matrix.api.MatrixClient +import io.element.android.libraries.matrix.api.core.RoomId +import io.element.android.libraries.matrix.api.room.MatrixRoom +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.SharingStarted +import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.asFlow +import kotlinx.coroutines.flow.map +import kotlinx.coroutines.flow.stateIn +import javax.inject.Inject + +sealed interface AwaitRoomState { + object Loading : AwaitRoomState + object Error : AwaitRoomState + data class Loaded(val room: MatrixRoom) : AwaitRoomState +} + +@SingleIn(SessionScope::class) +class AwaitRoomStateFlowFactory @Inject constructor(private val matrixClient: MatrixClient) { + + fun create(lifecycleScope: CoroutineScope, roomId: RoomId): StateFlow = suspend { + matrixClient.getRoom(roomId = roomId) + } + .asFlow() + .map { room -> + if (room != null) { + AwaitRoomState.Loaded(room) + } else { + AwaitRoomState.Error + } + } + .stateIn(lifecycleScope, SharingStarted.Eagerly, AwaitRoomState.Loading) +} diff --git a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt similarity index 98% rename from appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt rename to appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt index a0867facf2..fd4b4e5174 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/RoomFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt @@ -14,7 +14,7 @@ * limitations under the License. */ -package io.element.android.appnav +package io.element.android.appnav.room import android.os.Parcelable import androidx.compose.runtime.Composable @@ -32,6 +32,7 @@ import com.bumble.appyx.navmodel.backstack.operation.push import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode +import io.element.android.appnav.NodeLifecycleCallback import io.element.android.features.messages.api.MessagesEntryPoint import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint import io.element.android.libraries.architecture.BackstackNode diff --git a/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt index daff06af16..6f816f9de9 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt @@ -26,6 +26,7 @@ import com.bumble.appyx.navmodel.backstack.activeElement import com.bumble.appyx.testing.junit4.util.MainDispatcherRule import com.bumble.appyx.testing.unit.common.helper.parentNodeTestHelper import com.google.common.truth.Truth +import io.element.android.appnav.room.RoomFlowNode import io.element.android.features.messages.api.MessagesEntryPoint import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint import io.element.android.libraries.architecture.childNode From d71a025f9d482ffd62e84d6f983c97c3aa46ed44 Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 6 Jul 2023 10:35:32 +0200 Subject: [PATCH 06/11] Room : rename the flows --- .../kotlin/io/element/android/x/MainNode.kt | 4 +- .../android/appnav/LoggedInFlowNode.kt | 14 +- .../android/appnav/room/AwaitRoomNode.kt | 177 ----------------- .../appnav/room/LoadingRoomNodeView.kt | 126 ++++++++++++ ...{AwaitRoomState.kt => LoadingRoomState.kt} | 39 ++-- .../android/appnav/room/RoomFlowNode.kt | 168 ++++++---------- .../android/appnav/room/RoomLoadedFlowNode.kt | 179 ++++++++++++++++++ .../android/appnav/RoomFlowNodeTest.kt | 18 +- 8 files changed, 411 insertions(+), 314 deletions(-) delete mode 100644 appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomNode.kt create mode 100644 appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomNodeView.kt rename appnav/src/main/kotlin/io/element/android/appnav/room/{AwaitRoomState.kt => LoadingRoomState.kt} (60%) create mode 100644 appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt diff --git a/app/src/main/kotlin/io/element/android/x/MainNode.kt b/app/src/main/kotlin/io/element/android/x/MainNode.kt index fc4e30e749..8e7d0f194d 100644 --- a/app/src/main/kotlin/io/element/android/x/MainNode.kt +++ b/app/src/main/kotlin/io/element/android/x/MainNode.kt @@ -28,7 +28,7 @@ import com.bumble.appyx.core.node.Node import com.bumble.appyx.core.node.ParentNode import com.bumble.appyx.core.plugin.Plugin import io.element.android.appnav.LoggedInFlowNode -import io.element.android.appnav.room.RoomFlowNode +import io.element.android.appnav.room.RoomLoadedFlowNode import io.element.android.appnav.RootFlowNode import io.element.android.libraries.architecture.bindings import io.element.android.libraries.architecture.createNode @@ -67,7 +67,7 @@ class MainNode( } } - private val roomFlowNodeCallback = object : RoomFlowNode.LifecycleCallback { + private val roomFlowNodeCallback = object : RoomLoadedFlowNode.LifecycleCallback { override fun onFlowCreated(identifier: String, room: MatrixRoom) { val component = bindings().roomComponentBuilder().room(room).build() mainDaggerComponentOwner.addComponent(identifier, component) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt index 369b3054fb..432b94e744 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/LoggedInFlowNode.kt @@ -42,8 +42,8 @@ import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode import io.element.android.appnav.loggedin.LoggedInNode -import io.element.android.appnav.room.AwaitRoomNode import io.element.android.appnav.room.RoomFlowNode +import io.element.android.appnav.room.RoomLoadedFlowNode import io.element.android.features.analytics.api.AnalyticsEntryPoint import io.element.android.features.createroom.api.CreateRoomEntryPoint import io.element.android.features.invitelist.api.InviteListEntryPoint @@ -208,7 +208,7 @@ class LoggedInFlowNode @AssistedInject constructor( @Parcelize data class Room( val roomId: RoomId, - val initialElement: RoomFlowNode.NavTarget = RoomFlowNode.NavTarget.Messages + val initialElement: RoomLoadedFlowNode.NavTarget = RoomLoadedFlowNode.NavTarget.Messages ) : NavTarget @Parcelize @@ -256,7 +256,7 @@ class LoggedInFlowNode @AssistedInject constructor( } override fun onRoomSettingsClicked(roomId: RoomId) { - backstack.push(NavTarget.Room(roomId, initialElement = RoomFlowNode.NavTarget.RoomDetails)) + backstack.push(NavTarget.Room(roomId, initialElement = RoomLoadedFlowNode.NavTarget.RoomDetails)) } override fun onReportBugClicked() { @@ -270,13 +270,13 @@ class LoggedInFlowNode @AssistedInject constructor( } is NavTarget.Room -> { val nodeLifecycleCallbacks = plugins() - val callback = object : RoomFlowNode.Callback { + val callback = object : RoomLoadedFlowNode.Callback { override fun onForwardedToSingleRoom(roomId: RoomId) { coroutineScope.launch { attachRoom(roomId) } } } - val inputs = AwaitRoomNode.Inputs(roomId = navTarget.roomId, initialElement = navTarget.initialElement) - createNode(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks) + val inputs = RoomFlowNode.Inputs(roomId = navTarget.roomId, initialElement = navTarget.initialElement) + createNode(buildContext, plugins = listOf(inputs, callback) + nodeLifecycleCallbacks) } NavTarget.Settings -> { val callback = object : PreferencesEntryPoint.Callback { @@ -334,7 +334,7 @@ class LoggedInFlowNode @AssistedInject constructor( } } - suspend fun attachRoom(roomId: RoomId): AwaitRoomNode { + suspend fun attachRoom(roomId: RoomId): RoomFlowNode { return attachChild { backstack.singleTop(NavTarget.RoomList) backstack.push(NavTarget.Room(roomId)) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomNode.kt deleted file mode 100644 index bd894a4aeb..0000000000 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomNode.kt +++ /dev/null @@ -1,177 +0,0 @@ -/* - * 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. - */ - -@file:OptIn(ExperimentalMaterial3Api::class) - -package io.element.android.appnav.room - -import android.os.Parcelable -import androidx.compose.foundation.background -import androidx.compose.foundation.layout.Box -import androidx.compose.foundation.layout.Row -import androidx.compose.foundation.layout.Spacer -import androidx.compose.foundation.layout.WindowInsets -import androidx.compose.foundation.layout.fillMaxSize -import androidx.compose.foundation.layout.padding -import androidx.compose.foundation.layout.size -import androidx.compose.foundation.layout.systemBars -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 -import androidx.compose.ui.Modifier -import androidx.compose.ui.unit.dp -import androidx.lifecycle.lifecycleScope -import com.bumble.appyx.core.composable.Children -import com.bumble.appyx.core.modality.BuildContext -import com.bumble.appyx.core.node.Node -import com.bumble.appyx.core.node.node -import com.bumble.appyx.core.plugin.Plugin -import com.bumble.appyx.core.plugin.plugins -import com.bumble.appyx.navmodel.backstack.BackStack -import dagger.assisted.Assisted -import dagger.assisted.AssistedInject -import io.element.android.anvilannotations.ContributesNode -import io.element.android.appnav.NodeLifecycleCallback -import io.element.android.appnav.safeRoot -import io.element.android.libraries.architecture.BackstackNode -import io.element.android.libraries.architecture.NodeInputs -import io.element.android.libraries.architecture.createNode -import io.element.android.libraries.architecture.inputs -import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom -import io.element.android.libraries.designsystem.components.avatar.AvatarSize -import io.element.android.libraries.designsystem.components.button.BackButton -import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator -import io.element.android.libraries.designsystem.theme.components.Scaffold -import io.element.android.libraries.designsystem.theme.components.TopAppBar -import io.element.android.libraries.designsystem.theme.placeholderBackground -import io.element.android.libraries.di.SessionScope -import io.element.android.libraries.matrix.api.core.RoomId -import io.element.android.libraries.theme.ElementTheme -import kotlinx.coroutines.flow.launchIn -import kotlinx.coroutines.flow.onEach -import kotlinx.parcelize.Parcelize - -@ContributesNode(SessionScope::class) -class AwaitRoomNode @AssistedInject constructor( - @Assisted val buildContext: BuildContext, - @Assisted plugins: List, - awaitRoomStateFlowFactory: AwaitRoomStateFlowFactory, -) : - BackstackNode( - backstack = BackStack( - initialElement = NavTarget.Loading, - savedStateMap = buildContext.savedStateMap, - ), - buildContext = buildContext, - plugins = plugins - ) { - - data class Inputs( - val roomId: RoomId, - val initialElement: RoomFlowNode.NavTarget = RoomFlowNode.NavTarget.Messages, - ) : NodeInputs - - private val inputs: Inputs = inputs() - private val awaitRoomStateFlow = awaitRoomStateFlowFactory.create(lifecycleScope, inputs.roomId) - - sealed interface NavTarget : Parcelable { - @Parcelize - object Loading : NavTarget - - @Parcelize - object Loaded : NavTarget - } - - init { - awaitRoomStateFlow.onEach { awaitRoomState -> - when (awaitRoomState) { - is AwaitRoomState.Loaded -> backstack.safeRoot(NavTarget.Loaded) - else -> backstack.safeRoot(NavTarget.Loading) - } - }.launchIn(lifecycleScope) - } - - override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { - return when (navTarget) { - NavTarget.Loaded -> { - val nodeLifecycleCallbacks = plugins() - val roomFlowNodeCallback = plugins() - val awaitRoomState = awaitRoomStateFlow.value - if (awaitRoomState is AwaitRoomState.Loaded) { - val inputs = RoomFlowNode.Inputs(awaitRoomState.room, initialElement = inputs.initialElement) - createNode(buildContext, plugins = listOf(inputs) + roomFlowNodeCallback + nodeLifecycleCallbacks) - } else { - loadingNode(buildContext, this::navigateUp) - } - } - NavTarget.Loading -> { - loadingNode(buildContext, this::navigateUp) - } - } - } - - private fun loadingNode(buildContext: BuildContext, onBackPressed: () -> Unit) = node(buildContext) { modifier -> - Scaffold( - modifier = modifier, - contentWindowInsets = WindowInsets.systemBars, - topBar = { - TopAppBar( - modifier = Modifier, - navigationIcon = { - BackButton(onClick = onBackPressed) - }, - 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) - } - }, - ) - }, - content = { padding -> - Box( - modifier = Modifier - .fillMaxSize() - .padding(padding), contentAlignment = Alignment.Center - ) { - CircularProgressIndicator() - } - }, - ) - - } - - @Composable - override fun View(modifier: Modifier) { - Children( - navModel = backstack, - modifier = modifier, - ) - } -} - 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 new file mode 100644 index 0000000000..394ed1223b --- /dev/null +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomNodeView.kt @@ -0,0 +1,126 @@ +/* + * 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.appnav.room + +import androidx.compose.foundation.background +import androidx.compose.foundation.layout.Box +import androidx.compose.foundation.layout.Row +import androidx.compose.foundation.layout.Spacer +import androidx.compose.foundation.layout.WindowInsets +import androidx.compose.foundation.layout.fillMaxSize +import androidx.compose.foundation.layout.padding +import androidx.compose.foundation.layout.size +import androidx.compose.foundation.layout.systemBars +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 +import androidx.compose.ui.Modifier +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.appnav.loggedin.LoggedInState +import io.element.android.appnav.loggedin.LoggedInStateProvider +import io.element.android.appnav.loggedin.LoggedInView +import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom +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 +import io.element.android.libraries.designsystem.preview.ElementPreviewLight +import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator +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 + +@OptIn(ExperimentalMaterial3Api::class) +@Composable +fun LoadingRoomNodeView( + state: LoadingRoomState, + onBackClicked: () -> Unit, + modifier: Modifier = Modifier, +) { + Scaffold( + modifier = modifier, + contentWindowInsets = WindowInsets.systemBars, + topBar = { + TopAppBar( + modifier = Modifier, + navigationIcon = { + 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) + } + }, + ) + }, + content = { padding -> + Box( + modifier = Modifier + .fillMaxSize() + .padding(padding) + .padding(16.dp), contentAlignment = Alignment.Center + ) { + if (state is LoadingRoomState.Error) { + Text( + text = stringResource(id = CommonStrings.error_unknown), + color = ElementTheme.colors.textSecondary, + fontSize = 14.sp, + ) + } else { + CircularProgressIndicator() + } + } + }, + ) +} + +@Preview +@Composable +fun LoadingRoomNodeViewLightPreview(@PreviewParameter(LoadingRoomStateProvider::class) state: LoadingRoomState) = + ElementPreviewLight { ContentToPreview(state) } + +@Preview +@Composable +fun LoadingRoomNodeViewDarkPreview(@PreviewParameter(LoadingRoomStateProvider::class) state: LoadingRoomState) = + ElementPreviewDark { ContentToPreview(state) } + +@Composable +private fun ContentToPreview(state: LoadingRoomState) { + LoadingRoomNodeView( + state = state, + onBackClicked = {} + ) +} diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomState.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomState.kt similarity index 60% rename from appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomState.kt rename to appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomState.kt index a0f0e1cd33..9e96fadbc4 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/AwaitRoomState.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomState.kt @@ -16,12 +16,14 @@ package io.element.android.appnav.room +import androidx.compose.ui.tooling.preview.PreviewParameterProvider import io.element.android.libraries.di.SessionScope import io.element.android.libraries.di.SingleIn import io.element.android.libraries.matrix.api.MatrixClient import io.element.android.libraries.matrix.api.core.RoomId import io.element.android.libraries.matrix.api.room.MatrixRoom import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.flow.Flow import kotlinx.coroutines.flow.SharingStarted import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.asFlow @@ -29,25 +31,36 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.stateIn import javax.inject.Inject -sealed interface AwaitRoomState { - object Loading : AwaitRoomState - object Error : AwaitRoomState - data class Loaded(val room: MatrixRoom) : AwaitRoomState +sealed interface LoadingRoomState { + object Loading : LoadingRoomState + object Error : LoadingRoomState + data class Loaded(val room: MatrixRoom) : LoadingRoomState +} + +open class LoadingRoomStateProvider : PreviewParameterProvider { + override val values: Sequence + get() = sequenceOf( + LoadingRoomState.Loading, + LoadingRoomState.Error + ) } @SingleIn(SessionScope::class) class AwaitRoomStateFlowFactory @Inject constructor(private val matrixClient: MatrixClient) { - fun create(lifecycleScope: CoroutineScope, roomId: RoomId): StateFlow = suspend { + fun create(lifecycleScope: CoroutineScope, roomId: RoomId): StateFlow = + getRoomFlow(roomId) + .map { room -> + if (room != null) { + LoadingRoomState.Loaded(room) + } else { + LoadingRoomState.Error + } + } + .stateIn(lifecycleScope, SharingStarted.Eagerly, LoadingRoomState.Loading) + + private fun getRoomFlow(roomId: RoomId): Flow = suspend { matrixClient.getRoom(roomId = roomId) } .asFlow() - .map { room -> - if (room != null) { - AwaitRoomState.Loaded(room) - } else { - AwaitRoomState.Error - } - } - .stateIn(lifecycleScope, SharingStarted.Eagerly, AwaitRoomState.Loading) } diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt index fd4b4e5174..a231664399 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt @@ -14,166 +14,122 @@ * limitations under the License. */ +@file:OptIn(ExperimentalMaterial3Api::class) + package io.element.android.appnav.room import android.os.Parcelable +import androidx.compose.material3.ExperimentalMaterial3Api import androidx.compose.runtime.Composable -import androidx.compose.runtime.DisposableEffect +import androidx.compose.runtime.collectAsState +import androidx.compose.runtime.getValue import androidx.compose.ui.Modifier import androidx.lifecycle.lifecycleScope import com.bumble.appyx.core.composable.Children -import com.bumble.appyx.core.lifecycle.subscribe import com.bumble.appyx.core.modality.BuildContext import com.bumble.appyx.core.node.Node +import com.bumble.appyx.core.node.node import com.bumble.appyx.core.plugin.Plugin import com.bumble.appyx.core.plugin.plugins import com.bumble.appyx.navmodel.backstack.BackStack -import com.bumble.appyx.navmodel.backstack.operation.push import dagger.assisted.Assisted import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode import io.element.android.appnav.NodeLifecycleCallback -import io.element.android.features.messages.api.MessagesEntryPoint -import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint +import io.element.android.appnav.safeRoot import io.element.android.libraries.architecture.BackstackNode import io.element.android.libraries.architecture.NodeInputs -import io.element.android.libraries.architecture.animation.rememberDefaultTransitionHandler +import io.element.android.libraries.architecture.createNode import io.element.android.libraries.architecture.inputs import io.element.android.libraries.di.SessionScope 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.room.MatrixRoom -import io.element.android.libraries.matrix.api.room.RoomMembershipObserver -import io.element.android.services.appnavstate.api.AppNavigationStateService -import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.distinctUntilChanged import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach -import kotlinx.coroutines.launch import kotlinx.parcelize.Parcelize -import timber.log.Timber @ContributesNode(SessionScope::class) class RoomFlowNode @AssistedInject constructor( - @Assisted buildContext: BuildContext, + @Assisted val buildContext: BuildContext, @Assisted plugins: List, - private val messagesEntryPoint: MessagesEntryPoint, - private val roomDetailsEntryPoint: RoomDetailsEntryPoint, - private val appNavigationStateService: AppNavigationStateService, - roomMembershipObserver: RoomMembershipObserver, -) : BackstackNode( - backstack = BackStack( - initialElement = plugins.filterIsInstance(Inputs::class.java).first().initialElement, - savedStateMap = buildContext.savedStateMap, - ), - buildContext = buildContext, - plugins = plugins, -) { - - interface Callback : Plugin { - fun onForwardedToSingleRoom(roomId: RoomId) - } - - interface LifecycleCallback : NodeLifecycleCallback { - fun onFlowCreated(identifier: String, room: MatrixRoom) = Unit - fun onFlowReleased(identifier: String, room: MatrixRoom) = Unit - } + awaitRoomStateFlowFactory: AwaitRoomStateFlowFactory, +) : + BackstackNode( + backstack = BackStack( + initialElement = NavTarget.Loading, + savedStateMap = buildContext.savedStateMap, + ), + buildContext = buildContext, + plugins = plugins + ) { data class Inputs( - val room: MatrixRoom, - val initialElement: NavTarget = NavTarget.Messages, + val roomId: RoomId, + val initialElement: RoomLoadedFlowNode.NavTarget = RoomLoadedFlowNode.NavTarget.Messages, ) : NodeInputs private val inputs: Inputs = inputs() - private val callbacks = plugins.filterIsInstance() + private val loadingRoomStateStateFlow = awaitRoomStateFlowFactory.create(lifecycleScope, inputs.roomId) - init { - lifecycle.subscribe( - onCreate = { - Timber.v("OnCreate") - plugins().forEach { it.onFlowCreated(id, inputs.room) } - appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId) - fetchRoomMembers() - }, - onDestroy = { - Timber.v("OnDestroy") - plugins().forEach { it.onFlowReleased(id, inputs.room) } - appNavigationStateService.onLeavingRoom(id) - } - ) - roomMembershipObserver.updates - .filter { update -> update.roomId == inputs.room.roomId && !update.isUserInRoom } - .onEach { - navigateUp() - } - .launchIn(lifecycleScope) - inputs() + sealed interface NavTarget : Parcelable { + @Parcelize + object Loading : NavTarget + + @Parcelize + object Loaded : NavTarget } - private fun fetchRoomMembers() = lifecycleScope.launch { - val room = inputs.room - room.updateMembers() - .onFailure { - Timber.e(it, "Fail to fetch members for room ${room.roomId}") - }.onSuccess { - Timber.v("Success fetching members for room ${room.roomId}") + override fun onBuilt() { + super.onBuilt() + loadingRoomStateStateFlow + .map { + it is LoadingRoomState.Loaded } + .distinctUntilChanged() + .onEach { isLoaded -> + if (isLoaded) { + backstack.safeRoot(NavTarget.Loaded) + } else { + backstack.safeRoot(NavTarget.Loading) + } + }.launchIn(lifecycleScope) } override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { return when (navTarget) { - NavTarget.Messages -> { - val callback = object : MessagesEntryPoint.Callback { - override fun onRoomDetailsClicked() { - backstack.push(NavTarget.RoomDetails) - } - - override fun onUserDataClicked(userId: UserId) { - backstack.push(NavTarget.RoomMemberDetails(userId)) - } - - override fun onForwardedToSingleRoom(roomId: RoomId) { - callbacks.forEach { it.onForwardedToSingleRoom(roomId) } - } + NavTarget.Loaded -> { + val nodeLifecycleCallbacks = plugins() + val roomFlowNodeCallback = plugins() + val awaitRoomState = loadingRoomStateStateFlow.value + if (awaitRoomState is LoadingRoomState.Loaded) { + val inputs = RoomLoadedFlowNode.Inputs(awaitRoomState.room, initialElement = inputs.initialElement) + createNode(buildContext, plugins = listOf(inputs) + roomFlowNodeCallback + nodeLifecycleCallbacks) + } else { + loadingNode(buildContext, this::navigateUp) } - messagesEntryPoint.createNode(this, buildContext, callback) } - NavTarget.RoomDetails -> { - val inputs = RoomDetailsEntryPoint.Inputs(RoomDetailsEntryPoint.InitialTarget.RoomDetails) - roomDetailsEntryPoint.createNode(this, buildContext, inputs, emptyList()) - } - is NavTarget.RoomMemberDetails -> { - val inputs = RoomDetailsEntryPoint.Inputs(RoomDetailsEntryPoint.InitialTarget.RoomMemberDetails(navTarget.userId)) - roomDetailsEntryPoint.createNode(this, buildContext, inputs, emptyList()) + NavTarget.Loading -> { + loadingNode(buildContext, this::navigateUp) } } } - sealed interface NavTarget : Parcelable { - @Parcelize - object Messages : NavTarget - - @Parcelize - object RoomDetails : NavTarget - - @Parcelize - data class RoomMemberDetails(val userId: UserId) : NavTarget + private fun loadingNode(buildContext: BuildContext, onBackClicked: () -> Unit) = node(buildContext) { modifier -> + val loadingRoomState by loadingRoomStateStateFlow.collectAsState() + LoadingRoomNodeView( + state = loadingRoomState, + modifier = modifier, + onBackClicked = onBackClicked + ) } @Composable override fun View(modifier: Modifier) { - // Rely on the View Lifecycle instead of the Node Lifecycle, - // because this node enters 'onDestroy' before his children, so it can leads to - // using the room in a child node where it's already closed. - DisposableEffect(Unit) { - inputs.room.open() - onDispose { - inputs.room.close() - } - } Children( navModel = backstack, modifier = modifier, - transitionHandler = rememberDefaultTransitionHandler(), ) } } + diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt new file mode 100644 index 0000000000..73a8579b07 --- /dev/null +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomLoadedFlowNode.kt @@ -0,0 +1,179 @@ +/* + * 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.appnav.room + +import android.os.Parcelable +import androidx.compose.runtime.Composable +import androidx.compose.runtime.DisposableEffect +import androidx.compose.ui.Modifier +import androidx.lifecycle.lifecycleScope +import com.bumble.appyx.core.composable.Children +import com.bumble.appyx.core.lifecycle.subscribe +import com.bumble.appyx.core.modality.BuildContext +import com.bumble.appyx.core.node.Node +import com.bumble.appyx.core.plugin.Plugin +import com.bumble.appyx.core.plugin.plugins +import com.bumble.appyx.navmodel.backstack.BackStack +import com.bumble.appyx.navmodel.backstack.operation.push +import dagger.assisted.Assisted +import dagger.assisted.AssistedInject +import io.element.android.anvilannotations.ContributesNode +import io.element.android.appnav.NodeLifecycleCallback +import io.element.android.features.messages.api.MessagesEntryPoint +import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint +import io.element.android.libraries.architecture.BackstackNode +import io.element.android.libraries.architecture.NodeInputs +import io.element.android.libraries.architecture.animation.rememberDefaultTransitionHandler +import io.element.android.libraries.architecture.inputs +import io.element.android.libraries.di.SessionScope +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.room.MatrixRoom +import io.element.android.libraries.matrix.api.room.RoomMembershipObserver +import io.element.android.services.appnavstate.api.AppNavigationStateService +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.launchIn +import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.launch +import kotlinx.parcelize.Parcelize +import timber.log.Timber + +@ContributesNode(SessionScope::class) +class RoomLoadedFlowNode @AssistedInject constructor( + @Assisted buildContext: BuildContext, + @Assisted plugins: List, + private val messagesEntryPoint: MessagesEntryPoint, + private val roomDetailsEntryPoint: RoomDetailsEntryPoint, + private val appNavigationStateService: AppNavigationStateService, + roomMembershipObserver: RoomMembershipObserver, +) : BackstackNode( + backstack = BackStack( + initialElement = plugins.filterIsInstance(Inputs::class.java).first().initialElement, + savedStateMap = buildContext.savedStateMap, + ), + buildContext = buildContext, + plugins = plugins, +) { + + interface Callback : Plugin { + fun onForwardedToSingleRoom(roomId: RoomId) + } + + interface LifecycleCallback : NodeLifecycleCallback { + fun onFlowCreated(identifier: String, room: MatrixRoom) = Unit + fun onFlowReleased(identifier: String, room: MatrixRoom) = Unit + } + + data class Inputs( + val room: MatrixRoom, + val initialElement: NavTarget = NavTarget.Messages, + ) : NodeInputs + + private val inputs: Inputs = inputs() + private val callbacks = plugins.filterIsInstance() + + init { + lifecycle.subscribe( + onCreate = { + Timber.v("OnCreate") + plugins().forEach { it.onFlowCreated(id, inputs.room) } + appNavigationStateService.onNavigateToRoom(id, inputs.room.roomId) + fetchRoomMembers() + }, + onDestroy = { + Timber.v("OnDestroy") + plugins().forEach { it.onFlowReleased(id, inputs.room) } + appNavigationStateService.onLeavingRoom(id) + } + ) + roomMembershipObserver.updates + .filter { update -> update.roomId == inputs.room.roomId && !update.isUserInRoom } + .onEach { + navigateUp() + } + .launchIn(lifecycleScope) + inputs() + } + + private fun fetchRoomMembers() = lifecycleScope.launch { + val room = inputs.room + room.updateMembers() + .onFailure { + Timber.e(it, "Fail to fetch members for room ${room.roomId}") + }.onSuccess { + Timber.v("Success fetching members for room ${room.roomId}") + } + } + + override fun resolve(navTarget: NavTarget, buildContext: BuildContext): Node { + return when (navTarget) { + NavTarget.Messages -> { + val callback = object : MessagesEntryPoint.Callback { + override fun onRoomDetailsClicked() { + backstack.push(NavTarget.RoomDetails) + } + + override fun onUserDataClicked(userId: UserId) { + backstack.push(NavTarget.RoomMemberDetails(userId)) + } + + override fun onForwardedToSingleRoom(roomId: RoomId) { + callbacks.forEach { it.onForwardedToSingleRoom(roomId) } + } + } + messagesEntryPoint.createNode(this, buildContext, callback) + } + NavTarget.RoomDetails -> { + val inputs = RoomDetailsEntryPoint.Inputs(RoomDetailsEntryPoint.InitialTarget.RoomDetails) + roomDetailsEntryPoint.createNode(this, buildContext, inputs, emptyList()) + } + is NavTarget.RoomMemberDetails -> { + val inputs = RoomDetailsEntryPoint.Inputs(RoomDetailsEntryPoint.InitialTarget.RoomMemberDetails(navTarget.userId)) + roomDetailsEntryPoint.createNode(this, buildContext, inputs, emptyList()) + } + } + } + + sealed interface NavTarget : Parcelable { + @Parcelize + object Messages : NavTarget + + @Parcelize + object RoomDetails : NavTarget + + @Parcelize + data class RoomMemberDetails(val userId: UserId) : NavTarget + } + + @Composable + override fun View(modifier: Modifier) { + // Rely on the View Lifecycle instead of the Node Lifecycle, + // because this node enters 'onDestroy' before his children, so it can leads to + // using the room in a child node where it's already closed. + DisposableEffect(Unit) { + inputs.room.open() + onDispose { + inputs.room.close() + } + } + Children( + navModel = backstack, + modifier = modifier, + transitionHandler = rememberDefaultTransitionHandler(), + ) + } +} diff --git a/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt index 6f816f9de9..0f5dcd84f6 100644 --- a/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt +++ b/appnav/src/test/kotlin/io/element/android/appnav/RoomFlowNodeTest.kt @@ -26,7 +26,7 @@ import com.bumble.appyx.navmodel.backstack.activeElement import com.bumble.appyx.testing.junit4.util.MainDispatcherRule import com.bumble.appyx.testing.unit.common.helper.parentNodeTestHelper import com.google.common.truth.Truth -import io.element.android.appnav.room.RoomFlowNode +import io.element.android.appnav.room.RoomLoadedFlowNode import io.element.android.features.messages.api.MessagesEntryPoint import io.element.android.features.roomdetails.api.RoomDetailsEntryPoint import io.element.android.libraries.architecture.childNode @@ -77,7 +77,7 @@ class RoomFlowNodeTest { plugins: List, messagesEntryPoint: MessagesEntryPoint = FakeMessagesEntryPoint(), roomDetailsEntryPoint: RoomDetailsEntryPoint = FakeRoomDetailsEntryPoint(), - ) = RoomFlowNode( + ) = RoomLoadedFlowNode( buildContext = BuildContext.root(savedStateMap = null), plugins = plugins, messagesEntryPoint = messagesEntryPoint, @@ -91,15 +91,15 @@ class RoomFlowNodeTest { // GIVEN val room = FakeMatrixRoom() val fakeMessagesEntryPoint = FakeMessagesEntryPoint() - val inputs = RoomFlowNode.Inputs(room) + val inputs = RoomLoadedFlowNode.Inputs(room) val roomFlowNode = aRoomFlowNode(listOf(inputs), fakeMessagesEntryPoint) // WHEN val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper() // THEN - Truth.assertThat(roomFlowNode.backstack.activeElement).isEqualTo(RoomFlowNode.NavTarget.Messages) - roomFlowNodeTestHelper.assertChildHasLifecycle(RoomFlowNode.NavTarget.Messages, Lifecycle.State.CREATED) - val messagesNode = roomFlowNode.childNode(RoomFlowNode.NavTarget.Messages)!! + Truth.assertThat(roomFlowNode.backstack.activeElement).isEqualTo(RoomLoadedFlowNode.NavTarget.Messages) + roomFlowNodeTestHelper.assertChildHasLifecycle(RoomLoadedFlowNode.NavTarget.Messages, Lifecycle.State.CREATED) + val messagesNode = roomFlowNode.childNode(RoomLoadedFlowNode.NavTarget.Messages)!! Truth.assertThat(messagesNode.id).isEqualTo(fakeMessagesEntryPoint.nodeId) } @@ -109,14 +109,14 @@ class RoomFlowNodeTest { val room = FakeMatrixRoom() val fakeMessagesEntryPoint = FakeMessagesEntryPoint() val fakeRoomDetailsEntryPoint = FakeRoomDetailsEntryPoint() - val inputs = RoomFlowNode.Inputs(room) + val inputs = RoomLoadedFlowNode.Inputs(room) val roomFlowNode = aRoomFlowNode(listOf(inputs), fakeMessagesEntryPoint, fakeRoomDetailsEntryPoint) val roomFlowNodeTestHelper = roomFlowNode.parentNodeTestHelper() // WHEN fakeMessagesEntryPoint.callback?.onRoomDetailsClicked() // THEN - roomFlowNodeTestHelper.assertChildHasLifecycle(RoomFlowNode.NavTarget.RoomDetails, Lifecycle.State.CREATED) - val roomDetailsNode = roomFlowNode.childNode(RoomFlowNode.NavTarget.RoomDetails)!! + roomFlowNodeTestHelper.assertChildHasLifecycle(RoomLoadedFlowNode.NavTarget.RoomDetails, Lifecycle.State.CREATED) + val roomDetailsNode = roomFlowNode.childNode(RoomLoadedFlowNode.NavTarget.RoomDetails)!! Truth.assertThat(roomDetailsNode.id).isEqualTo(fakeRoomDetailsEntryPoint.nodeId) } } From e6f40bbfe89cf1a8cd9ce207c2b0dacc7f0b910c Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 6 Jul 2023 12:24:20 +0200 Subject: [PATCH 07/11] Room: add network monitor in LoadingRoomView --- .../appnav/room/LoadingRoomNodeView.kt | 63 +++++++++++-------- .../android/appnav/room/LoadingRoomState.kt | 2 +- .../android/appnav/room/RoomFlowNode.kt | 9 ++- 3 files changed, 44 insertions(+), 30 deletions(-) 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 394ed1223b..b684d21703 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 @@ -18,6 +18,7 @@ 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.Row import androidx.compose.foundation.layout.Spacer import androidx.compose.foundation.layout.WindowInsets @@ -36,9 +37,7 @@ 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.appnav.loggedin.LoggedInState -import io.element.android.appnav.loggedin.LoggedInStateProvider -import io.element.android.appnav.loggedin.LoggedInView +import io.element.android.features.networkmonitor.api.ui.ConnectivityIndicatorView import io.element.android.libraries.designsystem.atomic.atoms.PlaceholderAtom import io.element.android.libraries.designsystem.components.avatar.AvatarSize import io.element.android.libraries.designsystem.components.button.BackButton @@ -52,10 +51,10 @@ import io.element.android.libraries.designsystem.theme.placeholderBackground import io.element.android.libraries.theme.ElementTheme import io.element.android.libraries.ui.strings.CommonStrings -@OptIn(ExperimentalMaterial3Api::class) @Composable fun LoadingRoomNodeView( state: LoadingRoomState, + hasNetworkConnection: Boolean, onBackClicked: () -> Unit, modifier: Modifier = Modifier, ) { @@ -63,28 +62,10 @@ fun LoadingRoomNodeView( modifier = modifier, contentWindowInsets = WindowInsets.systemBars, topBar = { - TopAppBar( - modifier = Modifier, - navigationIcon = { - 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) - } - }, - ) + Column { + ConnectivityIndicatorView(isOnline = hasNetworkConnection) + LoadingRoomTopBar(onBackClicked) + } }, content = { padding -> Box( @@ -107,6 +88,33 @@ fun LoadingRoomNodeView( ) } +@OptIn(ExperimentalMaterial3Api::class) +@Composable +private fun LoadingRoomTopBar(onBackClicked: () -> Unit) { + TopAppBar( + modifier = Modifier, + navigationIcon = { + 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) + } + }, + ) +} + @Preview @Composable fun LoadingRoomNodeViewLightPreview(@PreviewParameter(LoadingRoomStateProvider::class) state: LoadingRoomState) = @@ -121,6 +129,7 @@ fun LoadingRoomNodeViewDarkPreview(@PreviewParameter(LoadingRoomStateProvider::c private fun ContentToPreview(state: LoadingRoomState) { LoadingRoomNodeView( state = state, - onBackClicked = {} + onBackClicked = {}, + hasNetworkConnection = false ) } diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomState.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomState.kt index 9e96fadbc4..db4627c3b4 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomState.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/LoadingRoomState.kt @@ -46,7 +46,7 @@ open class LoadingRoomStateProvider : PreviewParameterProvider } @SingleIn(SessionScope::class) -class AwaitRoomStateFlowFactory @Inject constructor(private val matrixClient: MatrixClient) { +class LoadingRoomStateFlowFactory @Inject constructor(private val matrixClient: MatrixClient) { fun create(lifecycleScope: CoroutineScope, roomId: RoomId): StateFlow = getRoomFlow(roomId) diff --git a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt index a231664399..a03b6d61a2 100644 --- a/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt +++ b/appnav/src/main/kotlin/io/element/android/appnav/room/RoomFlowNode.kt @@ -37,6 +37,8 @@ import dagger.assisted.AssistedInject import io.element.android.anvilannotations.ContributesNode import io.element.android.appnav.NodeLifecycleCallback import io.element.android.appnav.safeRoot +import io.element.android.features.networkmonitor.api.NetworkMonitor +import io.element.android.features.networkmonitor.api.NetworkStatus import io.element.android.libraries.architecture.BackstackNode import io.element.android.libraries.architecture.NodeInputs import io.element.android.libraries.architecture.createNode @@ -53,7 +55,8 @@ import kotlinx.parcelize.Parcelize class RoomFlowNode @AssistedInject constructor( @Assisted val buildContext: BuildContext, @Assisted plugins: List, - awaitRoomStateFlowFactory: AwaitRoomStateFlowFactory, + loadingRoomStateFlowFactory: LoadingRoomStateFlowFactory, + private val networkMonitor: NetworkMonitor, ) : BackstackNode( backstack = BackStack( @@ -70,7 +73,7 @@ class RoomFlowNode @AssistedInject constructor( ) : NodeInputs private val inputs: Inputs = inputs() - private val loadingRoomStateStateFlow = awaitRoomStateFlowFactory.create(lifecycleScope, inputs.roomId) + private val loadingRoomStateStateFlow = loadingRoomStateFlowFactory.create(lifecycleScope, inputs.roomId) sealed interface NavTarget : Parcelable { @Parcelize @@ -117,8 +120,10 @@ class RoomFlowNode @AssistedInject constructor( private fun loadingNode(buildContext: BuildContext, onBackClicked: () -> Unit) = node(buildContext) { modifier -> val loadingRoomState by loadingRoomStateStateFlow.collectAsState() + val networkStatus by networkMonitor.connectivity.collectAsState() LoadingRoomNodeView( state = loadingRoomState, + hasNetworkConnection = networkStatus == NetworkStatus.Online, modifier = modifier, onBackClicked = onBackClicked ) From 4911fdc15ece88bebb9216a0ea81fa915fc1b49a Mon Sep 17 00:00:00 2001 From: ElementBot Date: Thu, 6 Jul 2023 10:40:20 +0000 Subject: [PATCH 08/11] Update screenshots --- ...ltGroup_ColorAliasesDarkPreview_0_null,NEXUS_5,1.0,en].png | 4 ++-- ...tGroup_ColorAliasesLightPreview_0_null,NEXUS_5,1.0,en].png | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesDarkPreview_0_null,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesDarkPreview_0_null,NEXUS_5,1.0,en].png index a0c55af7c6..1e9449f4b3 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesDarkPreview_0_null,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesDarkPreview_0_null,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:863eaa52c2c0f684d29ed6d6618f4fcd18c953381d6624bdeb70a484bb37520e -size 36819 +oid sha256:e6a92140902cde5caf356028c3f67e4d9813672b4799c079de4b634b12f739ba +size 37014 diff --git a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesLightPreview_0_null,NEXUS_5,1.0,en].png b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesLightPreview_0_null,NEXUS_5,1.0,en].png index cbe6abda35..d4a3322ddd 100644 --- a/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesLightPreview_0_null,NEXUS_5,1.0,en].png +++ b/tests/uitests/src/test/snapshots/images/io.element.android.tests.uitests_ScreenshotTest_preview_tests[io.element.android.libraries.designsystem.theme_null_DefaultGroup_ColorAliasesLightPreview_0_null,NEXUS_5,1.0,en].png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:6523908f527a20ac68e3dad227f5987e348411629c045ea2a9af2cfe48a66867 -size 36517 +oid sha256:32d3246e87a7ce66191f9a84dc6b0b3106e044d510fdb0b69bad9260f7be6d4e +size 36716 From 7dbac91cd3e442e283cde05de68350dcfecaf873 Mon Sep 17 00:00:00 2001 From: ganfra Date: Thu, 6 Jul 2023 19:58:06 +0200 Subject: [PATCH 09/11] Room : Fix tests as there is less recomposition --- .../impl/root/CreateRoomRootPresenterTests.kt | 1 - .../messages/impl/MessagesPresenter.kt | 6 +- .../messages/MessagesPresenterTest.kt | 70 ++++++++++--------- .../libraries/matrix/test/FakeMatrixClient.kt | 2 +- 4 files changed, 42 insertions(+), 37 deletions(-) diff --git a/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt b/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt index 408632e04a..8976c77b8e 100644 --- a/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt +++ b/features/createroom/impl/src/test/kotlin/io/element/android/features/createroom/impl/root/CreateRoomRootPresenterTests.kt @@ -176,7 +176,6 @@ class CreateRoomRootPresenterTests { // Retry with success fakeMatrixClient.givenCreateDmError(null) stateAfterSecondAttempt.eventSink(CreateRoomRootEvents.StartDM(matrixUser)) - assertThat(awaitItem().startDmAction).isInstanceOf(Async.Uninitialized::class.java) assertThat(awaitItem().startDmAction).isInstanceOf(Async.Loading::class.java) val stateAfterRetryStartDM = awaitItem() assertThat(stateAfterRetryStartDM.startDmAction).isInstanceOf(Async.Success::class.java) 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 ae6b65f076..1171e4b23f 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 @@ -177,9 +177,9 @@ class MessagesPresenter @AssistedInject constructor( private fun MatrixRoom.avatarData(): AvatarData { return AvatarData( - id = room.roomId.value, - name = room.displayName, - url = room.avatarUrl, + id = roomId.value, + name = displayName, + url = avatarUrl, size = AvatarSize.TimelineRoom ) } diff --git a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt index 604c9ec4a8..53034131dc 100644 --- a/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt +++ b/features/messages/impl/src/test/kotlin/io/element/android/features/messages/MessagesPresenterTest.kt @@ -20,6 +20,7 @@ import android.net.Uri import app.cash.molecule.RecompositionClock import app.cash.molecule.moleculeFlow import app.cash.turbine.test +import com.google.common.collect.Iterables.skip import com.google.common.truth.Truth.assertThat import io.element.android.features.analytics.test.FakeAnalyticsService import io.element.android.features.messages.fixtures.aMessageEvent @@ -82,7 +83,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() assertThat(initialState.roomId).isEqualTo(A_ROOM_ID) } @@ -96,7 +97,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.ToggleReaction("👍", AN_EVENT_ID)) assertThat(room.myReactions.count()).isEqualTo(1) @@ -117,7 +118,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.ToggleReaction("👍", AN_EVENT_ID)) assertThat(room.myReactions.count()).isEqualTo(1) @@ -134,7 +135,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Forward, aMessageEvent())) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -150,7 +151,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Copy, event)) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -164,10 +165,10 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, aMessageEvent())) - skipItems(1) + val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Reply::class.java) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -180,7 +181,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, aMessageEvent(eventId = null))) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -195,7 +196,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() val mediaMessage = aMessageEvent( content = TimelineItemImageContent( @@ -212,7 +213,7 @@ class MessagesPresenterTest { ) ) initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, mediaMessage)) - skipItems(1) + val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Reply::class.java) val replyMode = finalState.composerState.mode as MessageComposerMode.Reply @@ -227,7 +228,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() val mediaMessage = aMessageEvent( content = TimelineItemVideoContent( @@ -245,7 +246,7 @@ class MessagesPresenterTest { ) ) initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, mediaMessage)) - skipItems(1) + val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Reply::class.java) val replyMode = finalState.composerState.mode as MessageComposerMode.Reply @@ -260,7 +261,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() val mediaMessage = aMessageEvent( content = TimelineItemFileContent( @@ -273,7 +274,7 @@ class MessagesPresenterTest { ) ) initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Reply, mediaMessage)) - skipItems(1) + val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Reply::class.java) val replyMode = finalState.composerState.mode as MessageComposerMode.Reply @@ -288,10 +289,10 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Edit, aMessageEvent())) - skipItems(1) + val finalState = awaitItem() assertThat(finalState.composerState.mode).isInstanceOf(MessageComposerMode.Edit::class.java) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -306,7 +307,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Redact, aMessageEvent())) assertThat(matrixRoom.redactEventEventIdParam).isEqualTo(AN_EVENT_ID) @@ -321,7 +322,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.ReportContent, aMessageEvent())) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -335,7 +336,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.Dismiss) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -349,7 +350,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + val initialState = awaitItem() initialState.eventSink.invoke(MessagesEvents.HandleAction(TimelineItemAction.Developer, aMessageEvent())) assertThat(awaitItem().actionListState.target).isEqualTo(ActionListState.Target.None) @@ -364,7 +365,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(3) + val initialState = awaitItem() // Initially the composer doesn't have focus, so we don't show the alert @@ -389,7 +390,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(3) + val initialState = awaitItem() assertThat(initialState.showReinvitePrompt).isFalse() @@ -406,7 +407,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(3) + val initialState = awaitItem() assertThat(initialState.showReinvitePrompt).isFalse() @@ -432,9 +433,10 @@ class MessagesPresenterTest { presenter.present() }.test { val initialState = awaitItem() + skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) - skipItems(3) - + + skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() @@ -461,9 +463,10 @@ class MessagesPresenterTest { presenter.present() }.test { val initialState = awaitItem() + skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) - skipItems(3) - + + skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() @@ -482,9 +485,10 @@ class MessagesPresenterTest { presenter.present() }.test { val initialState = awaitItem() + skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) - skipItems(3) - + + skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() @@ -510,9 +514,9 @@ class MessagesPresenterTest { presenter.present() }.test { val initialState = awaitItem() + skipItems(1) initialState.eventSink(MessagesEvents.InviteDialogDismissed(InviteDialogAction.Invite)) - skipItems(3) - + skipItems(1) val loadingState = awaitItem() assertThat(loadingState.inviteProgress.isLoading()).isTrue() @@ -529,7 +533,7 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { - skipItems(1) + assertThat(awaitItem().userHasPermissionToSendMessage).isTrue() } } @@ -542,6 +546,8 @@ class MessagesPresenterTest { moleculeFlow(RecompositionClock.Immediate) { presenter.present() }.test { + // Default value + assertThat(awaitItem().userHasPermissionToSendMessage).isTrue() skipItems(1) assertThat(awaitItem().userHasPermissionToSendMessage).isFalse() } diff --git a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt index 4e43df2d2c..fcd2f161e9 100644 --- a/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt +++ b/libraries/matrix/test/src/main/kotlin/io/element/android/libraries/matrix/test/FakeMatrixClient.kt @@ -64,7 +64,7 @@ class FakeMatrixClient( private val getProfileResults = mutableMapOf>() private var uploadMediaResult: Result = Result.success(AN_AVATAR_URL) - override fun getRoom(roomId: RoomId): MatrixRoom? { + override suspend fun getRoom(roomId: RoomId): MatrixRoom? { return getRoomResults[roomId] } From 8852514652e1a4f3db6bdc83178dd6ad26b97bb0 Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 7 Jul 2023 10:51:43 +0200 Subject: [PATCH 10/11] Room: add extension method awaitAllRoomsAreLoaded with Timeout --- .../matrix/api/room/RoomSummaryDataSource.kt | 19 ++++++++++++++++++- .../libraries/matrix/impl/RustMatrixClient.kt | 9 ++++----- 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt index d8e67a40c3..e6c7b34056 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/room/RoomSummaryDataSource.kt @@ -16,13 +16,18 @@ package io.element.android.libraries.matrix.api.room +import kotlinx.coroutines.TimeoutCancellationException import kotlinx.coroutines.flow.StateFlow +import kotlinx.coroutines.flow.firstOrNull +import kotlinx.coroutines.withTimeout +import timber.log.Timber +import kotlin.time.Duration interface RoomSummaryDataSource { sealed class LoadingState { object NotLoaded : LoadingState() - data class Loaded(val numberOfRooms: Int): LoadingState() + data class Loaded(val numberOfRooms: Int) : LoadingState() } fun updateAllRoomsVisibleRange(range: IntRange) @@ -30,3 +35,15 @@ interface RoomSummaryDataSource { fun allRooms(): StateFlow> fun inviteRooms(): StateFlow> } + +suspend fun RoomSummaryDataSource.awaitAllRoomsAreLoaded(timeout: Duration = Duration.INFINITE) { + try { + withTimeout(timeout) { + allRoomsLoadingState().firstOrNull { + it is RoomSummaryDataSource.LoadingState.Loaded + } + } + } catch (timeoutException: TimeoutCancellationException) { + Timber.v("AwaitAllRooms: no response after $timeout") + } +} diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt index 85aef441d2..7a2f7ea6b8 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/RustMatrixClient.kt @@ -33,6 +33,7 @@ import io.element.android.libraries.matrix.api.pusher.PushersService import io.element.android.libraries.matrix.api.room.MatrixRoom import io.element.android.libraries.matrix.api.room.RoomMembershipObserver import io.element.android.libraries.matrix.api.room.RoomSummaryDataSource +import io.element.android.libraries.matrix.api.room.awaitAllRoomsAreLoaded import io.element.android.libraries.matrix.api.sync.SyncService import io.element.android.libraries.matrix.api.sync.SyncState import io.element.android.libraries.matrix.api.user.MatrixSearchUserResults @@ -55,10 +56,8 @@ import io.element.android.services.toolbox.api.systemclock.SystemClock import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.cancel -import kotlinx.coroutines.delay import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first -import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.withContext @@ -131,11 +130,11 @@ class RustMatrixClient constructor( } override suspend fun getRoom(roomId: RoomId): MatrixRoom? { + // Check if already in memory... var cachedPairOfRoom = pairOfRoom(roomId) if (cachedPairOfRoom == null) { - roomSummaryDataSource.allRoomsLoadingState().firstOrNull { - it is RoomSummaryDataSource.LoadingState.Loaded - } + //... otherwise, lets wait for the SS to load all rooms and check again. + roomSummaryDataSource.awaitAllRoomsAreLoaded() cachedPairOfRoom = pairOfRoom(roomId) } if (cachedPairOfRoom == null) return null From 7dd243cd70e843f156229ec103f1a7682a7817cf Mon Sep 17 00:00:00 2001 From: ganfra Date: Fri, 7 Jul 2023 11:30:55 +0200 Subject: [PATCH 11/11] Await room: add tests on LoadingRoomStateFlowFactory --- .../room/LoadingRoomStateFlowFactoryTest.kt | 79 +++++++++++++++++++ 1 file changed, 79 insertions(+) create mode 100644 appnav/src/test/kotlin/io/element/android/appnav/room/LoadingRoomStateFlowFactoryTest.kt diff --git a/appnav/src/test/kotlin/io/element/android/appnav/room/LoadingRoomStateFlowFactoryTest.kt b/appnav/src/test/kotlin/io/element/android/appnav/room/LoadingRoomStateFlowFactoryTest.kt new file mode 100644 index 0000000000..17b6f6deb9 --- /dev/null +++ b/appnav/src/test/kotlin/io/element/android/appnav/room/LoadingRoomStateFlowFactoryTest.kt @@ -0,0 +1,79 @@ +/* + * 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.appnav.room + +import app.cash.turbine.test +import com.google.common.truth.Truth +import io.element.android.libraries.matrix.api.room.RoomSummaryDataSource +import io.element.android.libraries.matrix.test.A_ROOM_ID +import io.element.android.libraries.matrix.test.A_SESSION_ID +import io.element.android.libraries.matrix.test.FakeMatrixClient +import io.element.android.libraries.matrix.test.room.FakeMatrixRoom +import io.element.android.libraries.matrix.test.room.FakeRoomSummaryDataSource +import kotlinx.coroutines.test.runTest +import org.junit.Test + +class LoadingRoomStateFlowFactoryTest { + + @Test + fun `flow should emit Loading and then Loaded when there is a room in cache`() = runTest { + val room = FakeMatrixRoom(sessionId= A_SESSION_ID, roomId = A_ROOM_ID) + val matrixClient = FakeMatrixClient(A_SESSION_ID).apply { + givenGetRoomResult(A_ROOM_ID, room) + } + val flowFactory = LoadingRoomStateFlowFactory(matrixClient) + flowFactory + .create(this, A_ROOM_ID) + .test { + Truth.assertThat(awaitItem()).isEqualTo(LoadingRoomState.Loading) + Truth.assertThat(awaitItem()).isEqualTo(LoadingRoomState.Loaded(room)) + } + } + + @Test + fun `flow should emit Loading and then Loaded when there is a room in cache after SS is loaded`() = runTest { + val room = FakeMatrixRoom(sessionId= A_SESSION_ID, roomId = A_ROOM_ID) + val roomSummaryDataSource = FakeRoomSummaryDataSource() + val matrixClient = FakeMatrixClient(A_SESSION_ID, roomSummaryDataSource = roomSummaryDataSource) + val flowFactory = LoadingRoomStateFlowFactory(matrixClient) + flowFactory + .create(this, A_ROOM_ID) + .test { + Truth.assertThat(awaitItem()).isEqualTo(LoadingRoomState.Loading) + matrixClient.givenGetRoomResult(A_ROOM_ID, room) + roomSummaryDataSource.postLoadingState(RoomSummaryDataSource.LoadingState.Loaded(1)) + Truth.assertThat(awaitItem()).isEqualTo(LoadingRoomState.Loaded(room)) + } + } + + @Test + fun `flow should emit Loading and then Error when there is no room in cache after SS is loaded`() = runTest { + val roomSummaryDataSource = FakeRoomSummaryDataSource() + val matrixClient = FakeMatrixClient(A_SESSION_ID, roomSummaryDataSource = roomSummaryDataSource) + val flowFactory = LoadingRoomStateFlowFactory(matrixClient) + flowFactory + .create(this, A_ROOM_ID) + .test { + Truth.assertThat(awaitItem()).isEqualTo(LoadingRoomState.Loading) + roomSummaryDataSource.postLoadingState(RoomSummaryDataSource.LoadingState.Loaded(1)) + Truth.assertThat(awaitItem()).isEqualTo(LoadingRoomState.Error) + } + } + + + +}