Improve room member list loading UX (#2543)

Improve room member list UX:

- Don't display the list in chunks anymore.
- Use an indeterminate linear progress indicator to display some loading is being done (either loading the cached list or the updated one).
- Try to make sure we don't display the members loaded from timeline items as the cached room list by mistake.
* Update screenshots
* Simplify member loading logic.

---------

Co-authored-by: ElementBot <benoitm+elementbot@element.io>

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListPresenter.kt

@@ -31,7 +31,6 @@ import dagger.assisted.AssistedFactory
 import dagger.assisted.AssistedInject
 import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationEvents
 import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationPresenter
-import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.architecture.Presenter
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
@@ -43,6 +42,7 @@ import io.element.android.libraries.matrix.api.room.RoomMembershipState
 import io.element.android.libraries.matrix.api.room.powerlevels.canInvite
 import io.element.android.libraries.matrix.api.room.roomMembers
 import kotlinx.collections.immutable.toImmutableList
+import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.launch
 import kotlinx.coroutines.withContext
 
@@ -62,7 +62,7 @@ class RoomMemberListPresenter @AssistedInject constructor(
     @Composable
     override fun present(): RoomMemberListState {
         val coroutineScope = rememberCoroutineScope()
-        var roomMembers by remember { mutableStateOf<AsyncData<RoomMembers>>(AsyncData.Loading()) }
+        var roomMembers by remember { mutableStateOf(RoomMembers.loading()) }
         var searchQuery by rememberSaveable { mutableStateOf("") }
         var searchResults by remember {
             mutableStateOf<SearchBarResultState<RoomMembers>>(SearchBarResultState.Initial())
@@ -90,21 +90,26 @@ class RoomMemberListPresenter @AssistedInject constructor(
             }
             withContext(coroutineDispatchers.io) {
                 val members = membersState.roomMembers().orEmpty().groupBy { it.membership }
-                roomMembers = AsyncData.Success(
-                    RoomMembers(
-                        invited = members.getOrDefault(RoomMembershipState.INVITE, emptyList()).toImmutableList(),
-                        joined = members.getOrDefault(RoomMembershipState.JOIN, emptyList())
-                            .sortedWith(PowerLevelRoomMemberComparator())
-                            .toImmutableList(),
-                        banned = members.getOrDefault(RoomMembershipState.BAN, emptyList()).sortedBy { it.userId.value }.toImmutableList(),
-                    )
+                val info = room.roomInfoFlow.first()
+                if (members.getOrDefault(RoomMembershipState.JOIN, emptyList()).size < info.joinedMembersCount / 2) {
+                    // Don't display initial room member list if we have less than half of the joined members:
+                    // This result will come from the timeline loading membership events and it'll be wrong.
+                    return@withContext
+                }
+                roomMembers = RoomMembers(
+                    invited = members.getOrDefault(RoomMembershipState.INVITE, emptyList()).toImmutableList(),
+                    joined = members.getOrDefault(RoomMembershipState.JOIN, emptyList())
+                        .sortedWith(PowerLevelRoomMemberComparator())
+                        .toImmutableList(),
+                    banned = members.getOrDefault(RoomMembershipState.BAN, emptyList()).sortedBy { it.userId.value }.toImmutableList(),
+                    isLoading = membersState is MatrixRoomMembersState.Pending,
                 )
             }
         }
 
-        LaunchedEffect(searchQuery) {
+        LaunchedEffect(membersState, searchQuery, isSearchActive) {
             withContext(coroutineDispatchers.io) {
-                searchResults = if (searchQuery.isEmpty()) {
+                searchResults = if (searchQuery.isEmpty() || !isSearchActive) {
                     SearchBarResultState.Initial()
                 } else {
                     val results = roomMemberListDataSource.search(searchQuery).groupBy { it.membership }
@@ -118,6 +123,7 @@ class RoomMemberListPresenter @AssistedInject constructor(
                                     .sortedWith(PowerLevelRoomMemberComparator())
                                     .toImmutableList(),
                                 banned = results.getOrDefault(RoomMembershipState.BAN, emptyList()).sortedBy { it.userId.value }.toImmutableList(),
+                                isLoading = membersState is MatrixRoomMembersState.Pending,
                             )
                         )
                     }

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListState.kt

@@ -17,13 +17,13 @@
 package io.element.android.features.roomdetails.impl.members
 
 import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationState
-import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
 import io.element.android.libraries.matrix.api.room.RoomMember
 import kotlinx.collections.immutable.ImmutableList
+import kotlinx.collections.immutable.persistentListOf
 
 data class RoomMemberListState(
-    val roomMembers: AsyncData<RoomMembers>,
+    val roomMembers: RoomMembers,
     val searchQuery: String,
     val searchResults: SearchBarResultState<RoomMembers>,
     val isSearchActive: Boolean,
@@ -36,4 +36,14 @@ data class RoomMembers(
     val invited: ImmutableList<RoomMember>,
     val joined: ImmutableList<RoomMember>,
     val banned: ImmutableList<RoomMember>,
-)
+    val isLoading: Boolean,
+) {
+    companion object {
+        fun loading() = RoomMembers(
+            invited = persistentListOf(),
+            joined = persistentListOf(),
+            banned = persistentListOf(),
+            isLoading = true,
+        )
+    }
+}

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListStateProvider.kt

@@ -19,7 +19,6 @@ package io.element.android.features.roomdetails.impl.members
 import androidx.compose.ui.tooling.preview.PreviewParameterProvider
 import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationState
 import io.element.android.features.roomdetails.impl.members.moderation.aRoomMembersModerationState
-import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
 import io.element.android.libraries.matrix.api.core.UserId
 import io.element.android.libraries.matrix.api.room.RoomMember
@@ -30,15 +29,14 @@ internal class RoomMemberListStateProvider : PreviewParameterProvider<RoomMember
     override val values: Sequence<RoomMemberListState>
         get() = sequenceOf(
             aRoomMemberListState(
-                roomMembers = AsyncData.Success(
-                    RoomMembers(
-                        invited = persistentListOf(aVictor(), aWalter()),
-                        joined = persistentListOf(anAlice(), aBob(), aWalter()),
-                        banned = persistentListOf(),
-                    )
+                roomMembers = RoomMembers(
+                    invited = persistentListOf(aVictor(), aWalter()),
+                    joined = persistentListOf(anAlice(), aBob(), aWalter()),
+                    banned = persistentListOf(),
+                    isLoading = false,
                 )
             ),
-            aRoomMemberListState(roomMembers = AsyncData.Loading()),
+            aRoomMemberListState(roomMembers = RoomMembers.loading()),
             aRoomMemberListState().copy(canInvite = true),
             aRoomMemberListState().copy(isSearchActive = false),
             aRoomMemberListState().copy(isSearchActive = true),
@@ -51,6 +49,7 @@ internal class RoomMemberListStateProvider : PreviewParameterProvider<RoomMember
                         invited = persistentListOf(aVictor()),
                         joined = persistentListOf(anAlice()),
                         banned = persistentListOf(),
+                        isLoading = false,
                     )
                 ),
             ),
@@ -66,26 +65,37 @@ internal class RoomMemberListStateBannedProvider : PreviewParameterProvider<Room
     override val values: Sequence<RoomMemberListState>
         get() = sequenceOf(
             aRoomMemberListState(
-                roomMembers = AsyncData.Success(
-                    RoomMembers(
-                        invited = persistentListOf(),
-                        joined = persistentListOf(),
-                        banned = persistentListOf(
-                            aRoomMember(userId = UserId("@alice:example.com"), displayName = "Alice"),
-                            aRoomMember(userId = UserId("@bob:example.com"), displayName = "Bob"),
-                            aRoomMember(userId = UserId("@charlie:example.com"), displayName = "Charlie"),
-                        ),
-                    )
+                roomMembers = RoomMembers(
+                    invited = persistentListOf(),
+                    joined = persistentListOf(),
+                    banned = persistentListOf(
+                        aRoomMember(userId = UserId("@alice:example.com"), displayName = "Alice"),
+                        aRoomMember(userId = UserId("@bob:example.com"), displayName = "Bob"),
+                        aRoomMember(userId = UserId("@charlie:example.com"), displayName = "Charlie"),
+                    ),
+                    isLoading = false,
                 ),
                 moderationState = aRoomMembersModerationState(canDisplayBannedUsers = true),
             ),
             aRoomMemberListState(
-                roomMembers = AsyncData.Success(
-                    RoomMembers(
-                        invited = persistentListOf(),
-                        joined = persistentListOf(),
-                        banned = persistentListOf(),
-                    )
+                roomMembers = RoomMembers(
+                    invited = persistentListOf(),
+                    joined = persistentListOf(),
+                    banned = persistentListOf(
+                        aRoomMember(userId = UserId("@alice:example.com"), displayName = "Alice"),
+                        aRoomMember(userId = UserId("@bob:example.com"), displayName = "Bob"),
+                        aRoomMember(userId = UserId("@charlie:example.com"), displayName = "Charlie"),
+                    ),
+                    isLoading = true,
+                ),
+                moderationState = aRoomMembersModerationState(canDisplayBannedUsers = true),
+            ),
+            aRoomMemberListState(
+                roomMembers = RoomMembers(
+                    invited = persistentListOf(),
+                    joined = persistentListOf(),
+                    banned = persistentListOf(),
+                    isLoading = false,
                 ),
                 moderationState = aRoomMembersModerationState(canDisplayBannedUsers = true),
             )
@@ -93,7 +103,7 @@ internal class RoomMemberListStateBannedProvider : PreviewParameterProvider<Room
 }
 
 internal fun aRoomMemberListState(
-    roomMembers: AsyncData<RoomMembers> = AsyncData.Uninitialized,
+    roomMembers: RoomMembers = RoomMembers.loading(),
     searchResults: SearchBarResultState<RoomMembers> = SearchBarResultState.Initial(),
     moderationState: RoomMembersModerationState = aRoomMembersModerationState(),
 ) = RoomMemberListState(

features/roomdetails/impl/src/main/kotlin/io/element/android/features/roomdetails/impl/members/RoomMemberListView.kt

@@ -16,6 +16,11 @@
 
 package io.element.android.features.roomdetails.impl.members
 
+import androidx.compose.animation.AnimatedVisibility
+import androidx.compose.animation.expandVertically
+import androidx.compose.animation.fadeIn
+import androidx.compose.animation.fadeOut
+import androidx.compose.animation.shrinkVertically
 import androidx.compose.foundation.ExperimentalFoundationApi
 import androidx.compose.foundation.background
 import androidx.compose.foundation.clickable
@@ -23,7 +28,6 @@ import androidx.compose.foundation.layout.Arrangement
 import androidx.compose.foundation.layout.Box
 import androidx.compose.foundation.layout.Column
 import androidx.compose.foundation.layout.consumeWindowInsets
-import androidx.compose.foundation.layout.fillMaxSize
 import androidx.compose.foundation.layout.fillMaxWidth
 import androidx.compose.foundation.layout.padding
 import androidx.compose.foundation.lazy.LazyColumn
@@ -49,13 +53,12 @@ import androidx.compose.ui.unit.dp
 import io.element.android.compound.theme.ElementTheme
 import io.element.android.features.roomdetails.impl.R
 import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationView
-import io.element.android.libraries.architecture.AsyncData
 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.ElementPreview
 import io.element.android.libraries.designsystem.preview.PreviewsDayNight
 import io.element.android.libraries.designsystem.theme.aliasScreenTitle
-import io.element.android.libraries.designsystem.theme.components.CircularProgressIndicator
+import io.element.android.libraries.designsystem.theme.components.LinearProgressIndicator
 import io.element.android.libraries.designsystem.theme.components.Scaffold
 import io.element.android.libraries.designsystem.theme.components.SearchBar
 import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
@@ -124,20 +127,15 @@ fun RoomMemberListView(
             )
 
             if (!state.isSearchActive) {
-                if (state.roomMembers is AsyncData.Success) {
-                    RoomMemberList(
-                        roomMembers = state.roomMembers.data,
-                        showMembersCount = true,
-                        canDisplayBannedUsersControls = state.moderationState.canDisplayBannedUsers,
-                        selectedSection = selectedSection,
-                        onSelectedSectionChanged = { selectedSection = it },
-                        onUserSelected = ::onUserSelected,
-                    )
-                } else if (state.roomMembers.isLoading()) {
-                    Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
-                        CircularProgressIndicator()
-                    }
-                }
+                RoomMemberList(
+                    isLoading = state.roomMembers.isLoading,
+                    roomMembers = state.roomMembers,
+                    showMembersCount = true,
+                    canDisplayBannedUsersControls = state.moderationState.canDisplayBannedUsers,
+                    selectedSection = selectedSection,
+                    onSelectedSectionChanged = { selectedSection = it },
+                    onUserSelected = ::onUserSelected,
+                )
             }
         }
     }
@@ -151,6 +149,7 @@ fun RoomMemberListView(
 @OptIn(ExperimentalMaterial3Api::class, ExperimentalFoundationApi::class)
 @Composable
 private fun RoomMemberList(
+    isLoading: Boolean,
     roomMembers: RoomMembers,
     showMembersCount: Boolean,
     selectedSection: SelectedSection,
@@ -159,28 +158,37 @@ private fun RoomMemberList(
     onUserSelected: (RoomMember) -> Unit,
 ) {
     LazyColumn(modifier = Modifier.fillMaxWidth(), state = rememberLazyListState()) {
-        if (canDisplayBannedUsersControls) {
-            stickyHeader {
-                val segmentedButtonTitles = persistentListOf(
-                    stringResource(id = R.string.screen_room_member_list_mode_members),
-                    stringResource(id = R.string.screen_room_member_list_mode_banned),
-                )
-                SingleChoiceSegmentedButtonRow(
-                    modifier = Modifier
-                        .background(ElementTheme.colors.bgCanvasDefault)
-                        .fillMaxWidth()
-                        .padding(start = 16.dp, end = 16.dp, bottom = 16.dp),
-                ) {
-                    for ((index, title) in segmentedButtonTitles.withIndex()) {
-                        SegmentedButton(
-                            index = index,
-                            count = segmentedButtonTitles.size,
-                            selected = selectedSection.ordinal == index,
-                            onClick = { onSelectedSectionChanged(SelectedSection.entries[index]) },
-                            text = title,
-                        )
+        stickyHeader {
+            Column {
+                if (canDisplayBannedUsersControls) {
+                    val segmentedButtonTitles = persistentListOf(
+                        stringResource(id = R.string.screen_room_member_list_mode_members),
+                        stringResource(id = R.string.screen_room_member_list_mode_banned),
+                    )
+                    SingleChoiceSegmentedButtonRow(
+                        modifier = Modifier
+                            .background(ElementTheme.colors.bgCanvasDefault)
+                            .fillMaxWidth()
+                            .padding(start = 16.dp, end = 16.dp, bottom = 16.dp),
+                    ) {
+                        for ((index, title) in segmentedButtonTitles.withIndex()) {
+                            SegmentedButton(
+                                index = index,
+                                count = segmentedButtonTitles.size,
+                                selected = selectedSection.ordinal == index,
+                                onClick = { onSelectedSectionChanged(SelectedSection.entries[index]) },
+                                text = title,
+                            )
+                        }
                     }
                 }
+                AnimatedVisibility(
+                    visible = isLoading,
+                    enter = fadeIn() + expandVertically(),
+                    exit = fadeOut() + shrinkVertically(),
+                ) {
+                    LinearProgressIndicator(modifier = Modifier.fillMaxWidth())
+                }
             }
         }
         when (selectedSection) {
@@ -335,6 +343,7 @@ private fun RoomMemberSearchBar(
         resultState = state,
         resultHandler = { results ->
             RoomMemberList(
+                isLoading = false,
                 roomMembers = results,
                 showMembersCount = false,
                 onUserSelected = { onUserSelected(it) },

features/roomdetails/impl/src/test/kotlin/io/element/android/features/roomdetails/members/RoomMemberListPresenterTests.kt

@@ -30,7 +30,6 @@ import io.element.android.features.roomdetails.impl.members.aWalter
 import io.element.android.features.roomdetails.impl.members.moderation.RoomMembersModerationEvents
 import io.element.android.features.roomdetails.impl.members.moderation.aRoomMembersModerationState
 import io.element.android.features.roomdetails.members.moderation.FakeRoomMembersModerationPresenter
-import io.element.android.libraries.architecture.AsyncData
 import io.element.android.libraries.core.coroutine.CoroutineDispatchers
 import io.element.android.libraries.designsystem.theme.components.SearchBarResultState
 import io.element.android.libraries.featureflag.api.FeatureFlagService
@@ -40,6 +39,7 @@ 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.MatrixRoomMembersState
 import io.element.android.libraries.matrix.test.room.FakeMatrixRoom
+import io.element.android.libraries.matrix.test.room.aRoomInfo
 import io.element.android.tests.testutils.WarmUpRule
 import io.element.android.tests.testutils.testCoroutineDispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
@@ -54,25 +54,28 @@ class RoomMemberListPresenterTests {
     val warmUpRule = WarmUpRule()
 
     @Test
-    fun `search is done automatically on start, but is async`() = runTest {
-        val room = FakeMatrixRoom()
+    fun `member loading is done automatically on start, but is async`() = runTest {
+        val room = FakeMatrixRoom().apply {
+            // Needed to avoid discarding the loaded members as a partial and invalid result
+            givenRoomInfo(aRoomInfo(joinedMembersCount = 2))
+        }
         val presenter = createPresenter(matrixRoom = room)
         moleculeFlow(RecompositionMode.Immediate) {
             presenter.present()
         }.test {
             skipItems(1)
             val initialState = awaitItem()
-            assertThat(initialState.roomMembers).isInstanceOf(AsyncData.Loading::class.java)
+            assertThat(initialState.roomMembers.isLoading).isTrue()
             assertThat(initialState.searchQuery).isEmpty()
             assertThat(initialState.searchResults).isInstanceOf(SearchBarResultState.Initial::class.java)
             assertThat(initialState.isSearchActive).isFalse()
             room.givenRoomMembersState(MatrixRoomMembersState.Ready(aRoomMemberList()))
             // Skip item while the new members state is processed
             skipItems(1)
-            val loadedState = awaitItem()
-            assertThat(loadedState.roomMembers).isInstanceOf(AsyncData.Success::class.java)
-            assertThat((loadedState.roomMembers as AsyncData.Success).data.invited).isEqualTo(listOf(aVictor(), aWalter()))
-            assertThat((loadedState.roomMembers as AsyncData.Success).data.joined).isNotEmpty()
+            val loadedMembersState = awaitItem()
+            assertThat(loadedMembersState.roomMembers.isLoading).isFalse()
+            assertThat(loadedMembersState.roomMembers.invited).isEqualTo(listOf(aVictor(), aWalter()))
+            assertThat(loadedMembersState.roomMembers.joined).isNotEmpty()
         }
     }
 
@@ -85,6 +88,7 @@ class RoomMemberListPresenterTests {
             skipItems(1)
             val loadedState = awaitItem()
             loadedState.eventSink(RoomMemberListEvents.OnSearchActiveChanged(true))
+            skipItems(1)
             val searchActiveState = awaitItem()
             assertThat(searchActiveState.isSearchActive).isTrue()
         }
@@ -101,6 +105,7 @@ class RoomMemberListPresenterTests {
             loadedState.eventSink(RoomMemberListEvents.OnSearchActiveChanged(true))
             val searchActiveState = awaitItem()
             searchActiveState.eventSink(RoomMemberListEvents.UpdateSearchQuery("something"))
+            skipItems(1)
             val searchQueryUpdatedState = awaitItem()
             assertThat(searchQueryUpdatedState.searchQuery).isEqualTo("something")
             val searchSearchResultDelivered = awaitItem()
@@ -119,6 +124,7 @@ class RoomMemberListPresenterTests {
             loadedState.eventSink(RoomMemberListEvents.OnSearchActiveChanged(true))
             val searchActiveState = awaitItem()
             searchActiveState.eventSink(RoomMemberListEvents.UpdateSearchQuery("Alice"))
+            skipItems(1)
             val searchQueryUpdatedState = awaitItem()
             assertThat(searchQueryUpdatedState.searchQuery).isEqualTo("Alice")
             val searchSearchResultDelivered = awaitItem()