Merge pull request #7223 from vector-im/feature/bca/fix_new_layout_list_flickering

Fix new layout flicker/leaks

vector/src/main/java/im/vector/app/features/home/room/list/home/HomeFilteredRoomsController.kt

@@ -16,6 +16,7 @@
 
 package im.vector.app.features.home.room.list.home
 
+import androidx.paging.PagedList
 import com.airbnb.epoxy.EpoxyModel
 import com.airbnb.epoxy.paging.PagedListEpoxyController
 import im.vector.app.core.platform.StateView
@@ -47,7 +48,6 @@ class HomeFilteredRoomsController @Inject constructor(
     var listener: RoomListListener? = null
 
     private var emptyStateData: StateView.State.Empty? = null
-    private var currentState: StateView.State = StateView.State.Content
 
     private val shouldUseSingleLine: Boolean
 
@@ -56,17 +56,22 @@ class HomeFilteredRoomsController @Inject constructor(
         shouldUseSingleLine = fontScale.scale > FontScalePreferences.SCALE_LARGE
     }
 
+    fun submitRoomsList(roomsList: PagedList<RoomSummary>) {
+        submitList(roomsList)
+        // If room is empty we may have a new EmptyState to display
+        if (roomsList.isEmpty()) {
+            requestForcedModelBuild()
+        }
+    }
+
     override fun addModels(models: List<EpoxyModel<*>>) {
+        val emptyStateData = this.emptyStateData
         if (models.isEmpty() && emptyStateData != null) {
-            emptyStateData?.let { emptyState ->
-                roomListEmptyItem {
-                    id("state_item")
-                    emptyData(emptyState)
-                }
-                currentState = emptyState
+            roomListEmptyItem {
+                id("state_item")
+                emptyData(emptyStateData)
             }
         } else {
-            currentState = StateView.State.Content
             super.addModels(models)
         }
     }
@@ -83,7 +88,14 @@ class HomeFilteredRoomsController @Inject constructor(
                 useSingleLineForLastEvent(host.shouldUseSingleLine)
             }
         } else {
-            roomSummaryItemFactory.create(item, roomChangeMembershipStates.orEmpty(), emptySet(), RoomListDisplayMode.ROOMS, listener, shouldUseSingleLine)
+            roomSummaryItemFactory.create(
+                    roomSummary = item,
+                    roomChangeMembershipStates = roomChangeMembershipStates.orEmpty(),
+                    selectedRoomIds = emptySet(),
+                    displayMode = RoomListDisplayMode.ROOMS,
+                    listener = listener,
+                    singleLineLastEvent = shouldUseSingleLine
+            )
         }
     }
 }

vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListFragment.kt

@@ -26,7 +26,6 @@ import androidx.recyclerview.widget.ConcatAdapter
 import androidx.recyclerview.widget.LinearLayoutManager
 import com.airbnb.epoxy.OnModelBuildFinishedListener
 import com.airbnb.mvrx.fragmentViewModel
-import com.airbnb.mvrx.withState
 import com.google.android.material.dialog.MaterialAlertDialogBuilder
 import dagger.hilt.android.AndroidEntryPoint
 import im.vector.app.R
@@ -137,6 +136,7 @@ class HomeRoomListFragment :
     }
 
     private fun setupRecyclerView() {
+        views.stateView.state = StateView.State.Content
         val layoutManager = LinearLayoutManager(context)
         firstItemObserver = FirstItemUpdatedObserver(layoutManager) {
             layoutManager.scrollToPosition(0)
@@ -151,17 +151,12 @@ class HomeRoomListFragment :
         roomListViewModel.onEach(HomeRoomListViewState::headersData) {
             headersController.submitData(it)
         }
-
         roomListViewModel.roomsLivePagedList.observe(viewLifecycleOwner) { roomsList ->
-            roomsController.submitList(roomsList)
-            if (roomsList.isEmpty()) {
-                roomsController.requestForcedModelBuild()
-            }
+            roomsController.submitRoomsList(roomsList)
+        }
+        roomListViewModel.onEach(HomeRoomListViewState::emptyState) { emptyState ->
+            roomsController.submitEmptyStateData(emptyState)
         }
-
-        roomListViewModel.emptyStateFlow.onEach { emptyStateOptional ->
-            roomsController.submitEmptyStateData(emptyStateOptional.getOrNull())
-        }.launchIn(lifecycleScope)
 
         setUpAdapters()
 
@@ -170,9 +165,7 @@ class HomeRoomListFragment :
         concatAdapter.registerAdapterDataObserver(firstItemObserver)
     }
 
-    override fun invalidate() = withState(roomListViewModel) { state ->
-        views.stateView.state = state.state
-    }
+    override fun invalidate() = Unit
 
     private fun setUpAdapters() {
         val headersAdapter = headersController.also { controller ->

vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt

@@ -18,10 +18,9 @@ package im.vector.app.features.home.room.list.home
 
 import android.widget.ImageView
 import androidx.lifecycle.LiveData
-import androidx.lifecycle.MutableLiveData
+import androidx.lifecycle.MediatorLiveData
 import androidx.lifecycle.Observer
 import androidx.paging.PagedList
-import arrow.core.Option
 import arrow.core.toOption
 import com.airbnb.mvrx.MavericksViewModelFactory
 import dagger.assisted.Assisted
@@ -37,15 +36,11 @@ import im.vector.app.core.resources.DrawableProvider
 import im.vector.app.core.resources.StringProvider
 import im.vector.app.features.displayname.getBestName
 import im.vector.app.features.home.room.list.home.header.HomeRoomFilter
-import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.flow.Flow
-import kotlinx.coroutines.flow.MutableSharedFlow
-import kotlinx.coroutines.flow.asSharedFlow
 import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
-import kotlinx.coroutines.flow.first
 import kotlinx.coroutines.flow.flatMapLatest
-import kotlinx.coroutines.flow.flow
+import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.launchIn
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.onEach
@@ -88,24 +83,19 @@ class HomeRoomListViewModel @AssistedInject constructor(
 
     companion object : MavericksViewModelFactory<HomeRoomListViewModel, HomeRoomListViewState> by hiltMavericksViewModelFactory()
 
-    private var roomsFlow: Flow<Option<RoomSummary>>? = null
     private val pagedListConfig = PagedList.Config.Builder()
             .setPageSize(10)
             .setInitialLoadSizeHint(20)
             .setEnablePlaceholders(true)
             .build()
 
-    private val _roomsLivePagedList = MutableLiveData<PagedList<RoomSummary>>()
+    private val _roomsLivePagedList = MediatorLiveData<PagedList<RoomSummary>>()
     val roomsLivePagedList: LiveData<PagedList<RoomSummary>> = _roomsLivePagedList
 
     private val internalPagedListObserver = Observer<PagedList<RoomSummary>> {
         _roomsLivePagedList.postValue(it)
     }
 
-    private var currentFilter: HomeRoomFilter = HomeRoomFilter.ALL
-    private val _emptyStateFlow = MutableSharedFlow<Optional<StateView.State.Empty>>(replay = 1)
-    val emptyStateFlow = _emptyStateFlow.asSharedFlow()
-
     private var filteredPagedRoomSummariesLive: UpdatableLivePageResult? = null
 
     init {
@@ -113,7 +103,25 @@ class HomeRoomListViewModel @AssistedInject constructor(
         observeInvites()
         observeRecents()
         observeFilterTabs()
-        observeRooms()
+        observeSpaceChanges()
+    }
+
+    private fun observeSpaceChanges() {
+        spaceStateHandler.getSelectedSpaceFlow()
+                .distinctUntilChanged()
+                .onStart {
+                    emit(spaceStateHandler.getCurrentSpace().toOption())
+                }
+                .onEach { selectedSpaceOption ->
+                    val selectedSpace = selectedSpaceOption.orNull()
+                    updateEmptyState()
+                    filteredPagedRoomSummariesLive?.let { liveResults ->
+                        liveResults.queryParams = liveResults.queryParams.copy(
+                                spaceFilter = selectedSpace?.roomId.toActiveSpaceOrNoFilter()
+                        )
+                    }
+                }
+                .launchIn(viewModelScope)
     }
 
     private fun observeInvites() {
@@ -139,7 +147,7 @@ class HomeRoomListViewModel @AssistedInject constructor(
                                 })
                                 .map { Optional.from(it) }
                     } else {
-                        flow { emit(Optional.empty()) }
+                        flowOf(Optional.empty())
                     }.onEach { listOptional ->
                         setState { copy(headersData = headersData.copy(recents = listOptional.getOrNull())) }
                     }
@@ -150,31 +158,30 @@ class HomeRoomListViewModel @AssistedInject constructor(
         preferencesStore.areFiltersEnabledFlow
                 .distinctUntilChanged()
                 .flatMapLatest { areEnabled ->
-                    if (areEnabled) {
-                        getFilterTabsFlow()
-                    } else {
-                        flow { emit(Optional.empty()) }
-                    }.onEach { filtersOptional ->
-                        setState {
-                            validateCurrentFilter(filtersOptional.getOrNull())
-                            copy(
-                                    headersData = headersData.copy(
-                                            filtersList = filtersOptional.getOrNull(),
-                                            currentFilter = currentFilter
-                                    )
-                            )
-                        }
+                    getFilterTabsFlow(areEnabled)
+                }.onEach { filtersOptional ->
+                    val filters = filtersOptional.getOrNull()
+                    if (!isCurrentFilterStillValid(filters)) {
+                        changeRoomFilter(HomeRoomFilter.ALL)
+                    }
+                    setState {
+                        copy(
+                                headersData = headersData.copy(
+                                        filtersList = filters,
+                                )
+                        )
                     }
                 }.launchIn(viewModelScope)
     }
 
-    private fun validateCurrentFilter(filtersList: List<HomeRoomFilter>?) {
-        if (filtersList?.contains(currentFilter) != true) {
-            handleChangeRoomFilter(HomeRoomFilter.ALL)
-        }
+    private suspend fun isCurrentFilterStillValid(filtersList: List<HomeRoomFilter>?): Boolean {
+        if (filtersList.isNullOrEmpty()) return false
+        val currentFilter = awaitState().headersData.currentFilter
+        return filtersList.contains(currentFilter)
     }
 
-    private fun getFilterTabsFlow(): Flow<Optional<MutableList<HomeRoomFilter>>> {
+    private fun getFilterTabsFlow(isEnabled: Boolean): Flow<Optional<MutableList<HomeRoomFilter>>> {
+        if (!isEnabled) return flowOf(Optional.empty())
         val spaceFLow = spaceStateHandler.getSelectedSpaceFlow()
                 .distinctUntilChanged()
                 .onStart {
@@ -228,15 +235,16 @@ class HomeRoomListViewModel @AssistedInject constructor(
         }
     }
 
-    private fun observeRooms() = viewModelScope.launch {
-        filteredPagedRoomSummariesLive?.livePagedList?.removeObserver(internalPagedListObserver)
-
+    private fun observeRooms(currentFilter: HomeRoomFilter, isAZOrdering: Boolean) {
+        filteredPagedRoomSummariesLive?.livePagedList?.let { livePagedList ->
+            _roomsLivePagedList.removeSource(livePagedList)
+        }
         val builder = RoomSummaryQueryParams.Builder().also {
             it.memberships = listOf(Membership.JOIN)
+            it.spaceFilter = spaceStateHandler.getCurrentSpace()?.roomId.toActiveSpaceOrNoFilter()
         }
-
         val params = getFilteredQueryParams(currentFilter, builder.build())
-        val sortOrder = if (preferencesStore.isAZOrderingEnabledFlow.first()) {
+        val sortOrder = if (isAZOrdering) {
             RoomSortOrder.NAME
         } else {
             RoomSortOrder.ACTIVITY
@@ -248,36 +256,21 @@ class HomeRoomListViewModel @AssistedInject constructor(
         ).also {
             filteredPagedRoomSummariesLive = it
         }
-
-        spaceStateHandler.getSelectedSpaceFlow()
-                .distinctUntilChanged()
-                .onStart {
-                    emit(spaceStateHandler.getCurrentSpace().toOption())
-                }
-                .onEach { selectedSpaceOption ->
-                    val selectedSpace = selectedSpaceOption.orNull()
-                    filteredPagedRoomSummariesLive?.queryParams = liveResults.queryParams.copy(
-                            spaceFilter = selectedSpace?.roomId.toActiveSpaceOrNoFilter()
-                    )
-                    emitEmptyState()
-                }
-                .also { roomsFlow = it }
-                .launchIn(viewModelScope)
-
-        liveResults.livePagedList.observeForever(internalPagedListObserver)
+        _roomsLivePagedList.addSource(liveResults.livePagedList, internalPagedListObserver)
     }
 
     private fun observeOrderPreferences() {
-        preferencesStore.isAZOrderingEnabledFlow.onEach {
-            observeRooms()
-        }.launchIn(viewModelScope)
+        preferencesStore.isAZOrderingEnabledFlow
+                .onEach { isAZOrdering ->
+                    val currentFilter = awaitState().headersData.currentFilter
+                    observeRooms(currentFilter, isAZOrdering)
+                }.launchIn(viewModelScope)
     }
 
-    private fun emitEmptyState() {
-        viewModelScope.launch {
-            val emptyState = getEmptyStateData(currentFilter, spaceStateHandler.getCurrentSpace())
-            _emptyStateFlow.emit(Optional.from(emptyState))
-        }
+    private suspend fun updateEmptyState() {
+        val currentFilter = awaitState().headersData.currentFilter
+        val emptyState = getEmptyStateData(currentFilter, spaceStateHandler.getCurrentSpace())
+        setState { copy(emptyState = emptyState) }
     }
 
     private fun getFilteredQueryParams(filter: HomeRoomFilter, currentParams: RoomSummaryQueryParams): RoomSummaryQueryParams {
@@ -346,21 +339,28 @@ class HomeRoomListViewModel @AssistedInject constructor(
     }
 
     override fun onCleared() {
+        filteredPagedRoomSummariesLive?.livePagedList?.let { livePagedList ->
+            _roomsLivePagedList.removeSource(livePagedList)
+        }
         super.onCleared()
-        filteredPagedRoomSummariesLive?.livePagedList?.removeObserver(internalPagedListObserver)
     }
 
     private fun handleChangeRoomFilter(newFilter: HomeRoomFilter) {
+        viewModelScope.launch {
+            changeRoomFilter(newFilter)
+        }
+    }
+
+    private suspend fun changeRoomFilter(newFilter: HomeRoomFilter) {
+        val currentFilter = awaitState().headersData.currentFilter
         if (currentFilter == newFilter) {
             return
         }
-        currentFilter = newFilter
+        setState { copy(headersData = headersData.copy(currentFilter = newFilter)) }
+        updateEmptyState()
         filteredPagedRoomSummariesLive?.let { liveResults ->
-            liveResults.queryParams = getFilteredQueryParams(currentFilter, liveResults.queryParams)
+            liveResults.queryParams = getFilteredQueryParams(newFilter, liveResults.queryParams)
         }
-
-        setState { copy(headersData = headersData.copy(currentFilter = currentFilter)) }
-        emitEmptyState()
     }
 
     fun isPublicRoom(roomId: String): Boolean {
@@ -381,9 +381,9 @@ class HomeRoomListViewModel @AssistedInject constructor(
     }
 
     private fun handleChangeNotificationMode(action: HomeRoomListAction.ChangeRoomNotificationState) {
-        val room = session.getRoom(action.roomId)
-        if (room != null) {
-            viewModelScope.launch {
+        viewModelScope.launch {
+            val room = session.getRoom(action.roomId)
+            if (room != null) {
                 try {
                     room.roomPushRuleService().setRoomNotificationState(action.notificationState)
                 } catch (failure: Throwable) {
@@ -394,8 +394,8 @@ class HomeRoomListViewModel @AssistedInject constructor(
     }
 
     private fun handleToggleTag(action: HomeRoomListAction.ToggleTag) {
-        session.getRoom(action.roomId)?.let { room ->
-            viewModelScope.launch(Dispatchers.IO) {
+        viewModelScope.launch {
+            session.getRoom(action.roomId)?.let { room ->
                 try {
                     if (room.roomSummary()?.hasTag(action.tag) == false) {
                         // Favorite and low priority tags are exclusive, so maybe delete the other tag first
@@ -418,11 +418,11 @@ class HomeRoomListViewModel @AssistedInject constructor(
     }
 
     private fun handleDeleteLocalRooms() = withState {
-        val localRoomIds = session.roomService()
-                .getRoomSummaries(roomSummaryQueryParams { roomId = QueryStringValue.Contains(RoomLocalEcho.PREFIX) })
-                .map { it.roomId }
-
         viewModelScope.launch {
+            val localRoomIds = session.roomService()
+                    .getRoomSummaries(roomSummaryQueryParams { roomId = QueryStringValue.Contains(RoomLocalEcho.PREFIX) })
+                    .map { it.roomId }
+
             localRoomIds.forEach {
                 session.roomService().deleteLocalRoom(it)
             }

vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewState.kt

@@ -21,6 +21,6 @@ import im.vector.app.core.platform.StateView
 import im.vector.app.features.home.room.list.home.header.RoomsHeadersData
 
 data class HomeRoomListViewState(
-        val state: StateView.State = StateView.State.Content,
+        val emptyState: StateView.State.Empty? = null,
         val headersData: RoomsHeadersData = RoomsHeadersData(),
 ) : MavericksState

vector/src/main/java/im/vector/app/features/home/room/list/home/header/RoomsHeadersData.kt

@@ -21,6 +21,6 @@ import org.matrix.android.sdk.api.session.room.model.RoomSummary
 data class RoomsHeadersData(
         val invitesCount: Int = 0,
         val filtersList: List<HomeRoomFilter>? = null,
-        val currentFilter: HomeRoomFilter? = null,
+        val currentFilter: HomeRoomFilter = HomeRoomFilter.ALL,
         val recents: List<RoomSummary>? = null
 )