From e7ed466b4fb81545d2b85f19497b41bc484c0aaa Mon Sep 17 00:00:00 2001 From: Jorge Martin Espinosa Date: Wed, 21 Feb 2024 12:16:00 +0100 Subject: [PATCH] Do not display empty room list state before the loading one (#2402) * Do not display empty room list state before the loading one --- ...empty-state-before-loading-roomlist.bugfix | 1 + .../invitelist/impl/InviteListPresenter.kt | 2 +- .../impl/InviteListPresenterTests.kt | 28 +++++++++++-------- .../DefaultInviteStateDataSource.kt | 2 +- .../impl/datasource/RoomListDataSource.kt | 27 +++++++----------- .../DefaultInviteStateDataSourceTest.kt | 2 +- .../libraries/matrix/api/roomlist/RoomList.kt | 3 +- .../matrix/impl/roomlist/RoomListFactory.kt | 4 +-- .../impl/roomlist/RoomSummaryListProcessor.kt | 9 +++--- .../roomselect/impl/RoomSelectPresenter.kt | 2 +- 10 files changed, 41 insertions(+), 39 deletions(-) create mode 100644 changelog.d/+do-not-display-empty-state-before-loading-roomlist.bugfix diff --git a/changelog.d/+do-not-display-empty-state-before-loading-roomlist.bugfix b/changelog.d/+do-not-display-empty-state-before-loading-roomlist.bugfix new file mode 100644 index 0000000000..21b409821d --- /dev/null +++ b/changelog.d/+do-not-display-empty-state-before-loading-roomlist.bugfix @@ -0,0 +1 @@ +Do not display empty room list state before the loading one when we still don't have any items diff --git a/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListPresenter.kt b/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListPresenter.kt index 33ecb36913..aa7cead66e 100644 --- a/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListPresenter.kt +++ b/features/invitelist/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListPresenter.kt @@ -58,7 +58,7 @@ class InviteListPresenter @Inject constructor( .roomListService .invites .summaries - .collectAsState() + .collectAsState(initial = emptyList()) var seenInvites by remember { mutableStateOf>(emptySet()) } diff --git a/features/invitelist/impl/src/test/kotlin/io/element/android/features/invitelist/impl/InviteListPresenterTests.kt b/features/invitelist/impl/src/test/kotlin/io/element/android/features/invitelist/impl/InviteListPresenterTests.kt index ee64f2b428..fa0cf663a2 100644 --- a/features/invitelist/impl/src/test/kotlin/io/element/android/features/invitelist/impl/InviteListPresenterTests.kt +++ b/features/invitelist/impl/src/test/kotlin/io/element/android/features/invitelist/impl/InviteListPresenterTests.kt @@ -18,6 +18,7 @@ package io.element.android.features.invitelist.impl import app.cash.molecule.RecompositionMode import app.cash.molecule.moleculeFlow +import app.cash.turbine.TurbineTestContext import app.cash.turbine.test import com.google.common.truth.Truth.assertThat import io.element.android.features.invitelist.api.SeenInvitesStore @@ -83,7 +84,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val withInviteState = awaitItem() + val withInviteState = awaitInitialItem() assertThat(withInviteState.inviteList.size).isEqualTo(1) assertThat(withInviteState.inviteList[0].roomId).isEqualTo(A_ROOM_ID) assertThat(withInviteState.inviteList[0].roomAlias).isEqualTo(A_USER_ID.value) @@ -109,7 +110,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val withInviteState = awaitItem() + val withInviteState = awaitInitialItem() assertThat(withInviteState.inviteList.size).isEqualTo(1) assertThat(withInviteState.inviteList[0].sender?.displayName).isEqualTo(A_USER_NAME) assertThat(withInviteState.inviteList[0].sender?.userId).isEqualTo(A_USER_ID) @@ -138,7 +139,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.DeclineInvite(originalState.inviteList[0])) val newState = awaitItem() @@ -159,7 +160,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.DeclineInvite(originalState.inviteList[0])) val newState = awaitItem() @@ -180,7 +181,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.DeclineInvite(originalState.inviteList[0])) skipItems(1) @@ -206,7 +207,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.DeclineInvite(originalState.inviteList[0])) skipItems(1) @@ -234,7 +235,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.DeclineInvite(originalState.inviteList[0])) skipItems(1) @@ -264,7 +265,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.DeclineInvite(originalState.inviteList[0])) skipItems(1) @@ -295,7 +296,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.AcceptInvite(originalState.inviteList[0])) val newState = awaitItem() @@ -320,7 +321,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.AcceptInvite(originalState.inviteList[0])) assertThat(awaitItem().acceptedAction).isEqualTo(AsyncData.Failure(ex)) @@ -342,7 +343,7 @@ class InviteListPresenterTests { moleculeFlow(RecompositionMode.Immediate) { presenter.present() }.test { - val originalState = awaitItem() + val originalState = awaitInitialItem() originalState.eventSink(InviteListEvents.AcceptInvite(originalState.inviteList[0])) skipItems(1) @@ -485,6 +486,11 @@ class InviteListPresenterTests { ) ) + private suspend fun TurbineTestContext.awaitInitialItem(): InviteListState { + skipItems(1) + return awaitItem() + } + private fun createPresenter( client: MatrixClient, seenInvitesStore: SeenInvitesStore = FakeSeenInvitesStore(), diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSource.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSource.kt index cce14f316a..9057bf23ae 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSource.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSource.kt @@ -46,7 +46,7 @@ class DefaultInviteStateDataSource @Inject constructor( .roomListService .invites .summaries - .collectAsState() + .collectAsState(initial = emptyList()) val seenInvites by seenInvitesStore .seenRoomIds() diff --git a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt index da03122134..cb61701342 100644 --- a/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt +++ b/features/roomlist/impl/src/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt @@ -21,7 +21,6 @@ import io.element.android.libraries.androidutils.diff.DiffCacheUpdater import io.element.android.libraries.androidutils.diff.MutableListDiffCache import io.element.android.libraries.core.coroutine.CoroutineDispatchers import io.element.android.libraries.matrix.api.notificationsettings.NotificationSettingsService -import io.element.android.libraries.matrix.api.roomlist.RoomList import io.element.android.libraries.matrix.api.roomlist.RoomListService import io.element.android.libraries.matrix.api.roomlist.RoomSummary import kotlinx.collections.immutable.ImmutableList @@ -33,6 +32,7 @@ import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.debounce import kotlinx.coroutines.flow.launchIn import kotlinx.coroutines.flow.onEach +import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -62,6 +62,12 @@ class RoomListDataSource @Inject constructor( roomListService .allRooms .summaries + .onStart { + // If we have no cached results, display a placeholder loading state + if (diffCache.isEmpty()) { + _allRooms.emit(RoomListRoomSummaryFactory.createFakeList()) + } + } .onEach { roomSummaries -> replaceWith(roomSummaries) } @@ -88,23 +94,10 @@ class RoomListDataSource @Inject constructor( } private suspend fun buildAndEmitAllRooms(roomSummaries: List) { - if (diffCache.isEmpty() && roomListService.allRooms.loadingState.value is RoomList.LoadingState.NotLoaded) { - // If the room list is not loaded, we emit a fake placeholders list - _allRooms.emit(RoomListRoomSummaryFactory.createFakeList()) - } else { - val roomListRoomSummaries = ArrayList() - for (index in diffCache.indices()) { - val cacheItem = diffCache.get(index) - if (cacheItem == null) { - buildAndCacheItem(roomSummaries, index)?.also { timelineItemState -> - roomListRoomSummaries.add(timelineItemState) - } - } else { - roomListRoomSummaries.add(cacheItem) - } - } - _allRooms.emit(roomListRoomSummaries.toImmutableList()) + val roomListRoomSummaries = diffCache.indices().mapNotNull { index -> + diffCache.get(index) ?: buildAndCacheItem(roomSummaries, index) } + _allRooms.emit(roomListRoomSummaries.toImmutableList()) } private fun buildAndCacheItem(roomSummaries: List, index: Int): RoomListRoomSummary? { diff --git a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSourceTest.kt b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSourceTest.kt index 0ff0f02d32..802d8ee40f 100644 --- a/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSourceTest.kt +++ b/features/roomlist/impl/src/test/kotlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSourceTest.kt @@ -57,7 +57,7 @@ internal class DefaultInviteStateDataSourceTest { moleculeFlow(RecompositionMode.Immediate) { dataSource.inviteState() }.test { - skipItems(1) + skipItems(2) assertThat(awaitItem()).isEqualTo(InvitesState.NewInvites) } } diff --git a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomList.kt b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomList.kt index 0132e35092..1af22cd4c8 100644 --- a/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomList.kt +++ b/libraries/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomList.kt @@ -17,6 +17,7 @@ package io.element.android.libraries.matrix.api.roomlist import kotlinx.coroutines.TimeoutCancellationException +import kotlinx.coroutines.flow.SharedFlow import kotlinx.coroutines.flow.StateFlow import kotlinx.coroutines.flow.firstOrNull import kotlinx.coroutines.withTimeout @@ -51,7 +52,7 @@ interface RoomList { /** * The list of room summaries as a flow. */ - val summaries: StateFlow> + val summaries: SharedFlow> /** * The loading state of the room list as a flow. diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomListFactory.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomListFactory.kt index 421a1296a8..586dafba4c 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomListFactory.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomListFactory.kt @@ -50,7 +50,7 @@ internal class RoomListFactory( innerProvider: suspend () -> InnerRoomList ): DynamicRoomList { val loadingStateFlow: MutableStateFlow = MutableStateFlow(RoomList.LoadingState.NotLoaded) - val summariesFlow = MutableStateFlow>(emptyList()) + val summariesFlow = MutableSharedFlow>(replay = 1, extraBufferCapacity = 1) val processor = RoomSummaryListProcessor(summariesFlow, innerRoomListService, coroutineContext, roomSummaryDetailsFactory) // Makes sure we don't miss any events val dynamicEvents = MutableSharedFlow(replay = 100) @@ -92,7 +92,7 @@ internal class RoomListFactory( } private class RustDynamicRoomList( - override val summaries: MutableStateFlow>, + override val summaries: MutableSharedFlow>, override val loadingState: MutableStateFlow, override val currentFilter: MutableStateFlow, override val loadedPages: MutableStateFlow, diff --git a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessor.kt b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessor.kt index a5028bb614..35f301c87e 100644 --- a/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessor.kt +++ b/libraries/matrix/impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessor.kt @@ -17,7 +17,7 @@ package io.element.android.libraries.matrix.impl.roomlist import io.element.android.libraries.matrix.api.roomlist.RoomSummary -import kotlinx.coroutines.flow.MutableStateFlow +import kotlinx.coroutines.flow.MutableSharedFlow import kotlinx.coroutines.sync.Mutex import kotlinx.coroutines.sync.withLock import kotlinx.coroutines.withContext @@ -30,7 +30,7 @@ import java.util.UUID import kotlin.coroutines.CoroutineContext class RoomSummaryListProcessor( - private val roomSummaries: MutableStateFlow>, + private val roomSummaries: MutableSharedFlow>, private val roomListService: RoomListServiceInterface, private val coroutineContext: CoroutineContext, private val roomSummaryDetailsFactory: RoomSummaryDetailsFactory = RoomSummaryDetailsFactory(), @@ -132,9 +132,10 @@ class RoomSummaryListProcessor( private suspend fun updateRoomSummaries(block: suspend MutableList.() -> Unit) = withContext(coroutineContext) { mutex.withLock { - val mutableRoomSummaries = roomSummaries.value.toMutableList() + val current = roomSummaries.replayCache.lastOrNull() + val mutableRoomSummaries = current.orEmpty().toMutableList() block(mutableRoomSummaries) - roomSummaries.value = mutableRoomSummaries + roomSummaries.emit(mutableRoomSummaries) } } } diff --git a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt index 51a1436e11..5a32a64ac1 100644 --- a/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt +++ b/libraries/roomselect/impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt @@ -52,7 +52,7 @@ class RoomSelectPresenter @AssistedInject constructor( var isSearchActive by remember { mutableStateOf(false) } var results: SearchBarResultState> by remember { mutableStateOf(SearchBarResultState.Initial()) } - val summaries by client.roomListService.allRooms.summaries.collectAsState() + val summaries by client.roomListService.allRooms.summaries.collectAsState(initial = emptyList()) LaunchedEffect(query, summaries) { val filteredSummaries = summaries.filterIsInstance()