Do not display empty room list state before the loading one (#2402)

* Do not display empty room list state before the loading one
element-hq · Feb 21, 2024 · 598bf96 · 598bf96
1 parent 7532f96
commit 598bf96
Show file tree

Hide file tree

Showing 10 changed files with 41 additions and 39 deletions.
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
@@ -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/...t/impl/src/main/kotlin/io/element/android/features/invitelist/impl/InviteListPresenter.kt b/...t/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<Set<RoomId>>(emptySet()) }
 

diff --git a/...l/src/test/kotlin/io/element/android/features/invitelist/impl/InviteListPresenterTests.kt b/...l/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<RoomId>(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<InviteListState>.awaitInitialItem(): InviteListState {
+        skipItems(1)
+        return awaitItem()
+    }
+
     private fun createPresenter(
         client: MatrixClient,
         seenInvitesStore: SeenInvitesStore = FakeSeenInvitesStore(),

diff --git a/...tlin/io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSource.kt b/...tlin/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/...rc/main/kotlin/io/element/android/features/roomlist/impl/datasource/RoomListDataSource.kt b/...rc/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<RoomSummary>) {
-        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<RoomListRoomSummary>()
-            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<RoomSummary>, index: Int): RoomListRoomSummary? {

diff --git a/.../io/element/android/features/roomlist/impl/datasource/DefaultInviteStateDataSourceTest.kt b/.../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/...s/matrix/api/src/main/kotlin/io/element/android/libraries/matrix/api/roomlist/RoomList.kt b/...s/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<List<RoomSummary>>
+    val summaries: SharedFlow<List<RoomSummary>>
 
     /**
      * The loading state of the room list as a flow.

diff --git a/...impl/src/main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomListFactory.kt b/...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<RoomList.LoadingState> = MutableStateFlow(RoomList.LoadingState.NotLoaded)
-        val summariesFlow = MutableStateFlow<List<RoomSummary>>(emptyList())
+        val summariesFlow = MutableSharedFlow<List<RoomSummary>>(replay = 1, extraBufferCapacity = 1)
         val processor = RoomSummaryListProcessor(summariesFlow, innerRoomListService, coroutineContext, roomSummaryDetailsFactory)
         // Makes sure we don't miss any events
         val dynamicEvents = MutableSharedFlow<RoomListDynamicEvents>(replay = 100)
@@ -92,7 +92,7 @@ internal class RoomListFactory(
 }
 
 private class RustDynamicRoomList(
-    override val summaries: MutableStateFlow<List<RoomSummary>>,
+    override val summaries: MutableSharedFlow<List<RoomSummary>>,
     override val loadingState: MutableStateFlow<RoomList.LoadingState>,
     override val currentFilter: MutableStateFlow<RoomListFilter>,
     override val loadedPages: MutableStateFlow<Int>,

diff --git a/...main/kotlin/io/element/android/libraries/matrix/impl/roomlist/RoomSummaryListProcessor.kt b/...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<List<RoomSummary>>,
+    private val roomSummaries: MutableSharedFlow<List<RoomSummary>>,
     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<RoomSummary>.() -> 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/.../impl/src/main/kotlin/io/element/android/libraries/roomselect/impl/RoomSelectPresenter.kt b/.../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<ImmutableList<RoomSummaryDetails>> 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<RoomSummary.Filled>()