From 2ea357ddc0c62fd5f5d99c1b7fd4a09b41d2361e Mon Sep 17 00:00:00 2001 From: Valere Date: Fri, 23 Sep 2022 12:12:14 +0200 Subject: [PATCH 1/2] Fix new layout flicker/leaks --- .../room/list/home/HomeRoomListViewModel.kt | 80 +++++++++++-------- 1 file changed, 46 insertions(+), 34 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt index 41cf2189c0..63b667a697 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt +++ b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt @@ -51,6 +51,8 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.onEach import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.launch +import kotlinx.coroutines.sync.Mutex +import kotlinx.coroutines.sync.withLock import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.query.RoomCategoryFilter @@ -108,12 +110,34 @@ class HomeRoomListViewModel @AssistedInject constructor( private var filteredPagedRoomSummariesLive: UpdatableLivePageResult? = null + private val switchDataSourceMutex = Mutex() + init { observeOrderPreferences() observeInvites() observeRecents() observeFilterTabs() observeRooms() + observeSpaceChanges() + } + + private fun observeSpaceChanges() { + spaceStateHandler.getSelectedSpaceFlow() + .distinctUntilChanged() + .onStart { + emit(spaceStateHandler.getCurrentSpace().toOption()) + } + .onEach { selectedSpaceOption -> + val selectedSpace = selectedSpaceOption.orNull() + filteredPagedRoomSummariesLive?.let { liveResults -> + liveResults.queryParams = liveResults.queryParams.copy( + spaceFilter = selectedSpace?.roomId.toActiveSpaceOrNoFilter() + ) + emitEmptyState() + } + } + .also { roomsFlow = it } + .launchIn(viewModelScope) } private fun observeInvites() { @@ -229,42 +253,30 @@ class HomeRoomListViewModel @AssistedInject constructor( } private fun observeRooms() = viewModelScope.launch { - filteredPagedRoomSummariesLive?.livePagedList?.removeObserver(internalPagedListObserver) + switchDataSourceMutex.withLock { + filteredPagedRoomSummariesLive?.livePagedList?.removeObserver(internalPagedListObserver) - val builder = RoomSummaryQueryParams.Builder().also { - it.memberships = listOf(Membership.JOIN) + 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()) { + RoomSortOrder.NAME + } else { + RoomSortOrder.ACTIVITY + } + val liveResults = session.roomService().getFilteredPagedRoomSummariesLive( + params, + pagedListConfig, + sortOrder + ).also { + filteredPagedRoomSummariesLive = it + } + + liveResults.livePagedList.observeForever(internalPagedListObserver) } - - val params = getFilteredQueryParams(currentFilter, builder.build()) - val sortOrder = if (preferencesStore.isAZOrderingEnabledFlow.first()) { - RoomSortOrder.NAME - } else { - RoomSortOrder.ACTIVITY - } - val liveResults = session.roomService().getFilteredPagedRoomSummariesLive( - params, - pagedListConfig, - sortOrder - ).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) } private fun observeOrderPreferences() { From bf405394d86848dd9e7b5567d4c84b49adf87d83 Mon Sep 17 00:00:00 2001 From: ganfra Date: Mon, 26 Sep 2022 19:36:50 +0200 Subject: [PATCH 2/2] Home room list: make some clean up --- .../list/home/HomeFilteredRoomsController.kt | 30 +++- .../room/list/home/HomeRoomListFragment.kt | 19 +- .../room/list/home/HomeRoomListViewModel.kt | 162 ++++++++---------- .../room/list/home/HomeRoomListViewState.kt | 2 +- .../room/list/home/header/RoomsHeadersData.kt | 2 +- 5 files changed, 104 insertions(+), 111 deletions(-) diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeFilteredRoomsController.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeFilteredRoomsController.kt index ebf322dc23..cd245af0fc 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeFilteredRoomsController.kt +++ b/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) { + submitList(roomsList) + // If room is empty we may have a new EmptyState to display + if (roomsList.isEmpty()) { + requestForcedModelBuild() + } + } + override fun addModels(models: List>) { + 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 + ) } } } diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListFragment.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListFragment.kt index 767da24388..5677f3e4a8 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListFragment.kt +++ b/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 -> diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt index 63b667a697..7f62c68850 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt +++ b/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,22 +36,16 @@ 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 import kotlinx.coroutines.flow.onStart import kotlinx.coroutines.launch -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock import org.matrix.android.sdk.api.extensions.orFalse import org.matrix.android.sdk.api.query.QueryStringValue import org.matrix.android.sdk.api.query.RoomCategoryFilter @@ -90,34 +83,26 @@ class HomeRoomListViewModel @AssistedInject constructor( companion object : MavericksViewModelFactory by hiltMavericksViewModelFactory() - private var roomsFlow: Flow>? = null private val pagedListConfig = PagedList.Config.Builder() .setPageSize(10) .setInitialLoadSizeHint(20) .setEnablePlaceholders(true) .build() - private val _roomsLivePagedList = MutableLiveData>() + private val _roomsLivePagedList = MediatorLiveData>() val roomsLivePagedList: LiveData> = _roomsLivePagedList private val internalPagedListObserver = Observer> { _roomsLivePagedList.postValue(it) } - private var currentFilter: HomeRoomFilter = HomeRoomFilter.ALL - private val _emptyStateFlow = MutableSharedFlow>(replay = 1) - val emptyStateFlow = _emptyStateFlow.asSharedFlow() - private var filteredPagedRoomSummariesLive: UpdatableLivePageResult? = null - private val switchDataSourceMutex = Mutex() - init { observeOrderPreferences() observeInvites() observeRecents() observeFilterTabs() - observeRooms() observeSpaceChanges() } @@ -129,14 +114,13 @@ class HomeRoomListViewModel @AssistedInject constructor( } .onEach { selectedSpaceOption -> val selectedSpace = selectedSpaceOption.orNull() + updateEmptyState() filteredPagedRoomSummariesLive?.let { liveResults -> liveResults.queryParams = liveResults.queryParams.copy( spaceFilter = selectedSpace?.roomId.toActiveSpaceOrNoFilter() ) - emitEmptyState() } } - .also { roomsFlow = it } .launchIn(viewModelScope) } @@ -163,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())) } } @@ -174,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?) { - if (filtersList?.contains(currentFilter) != true) { - handleChangeRoomFilter(HomeRoomFilter.ALL) - } + private suspend fun isCurrentFilterStillValid(filtersList: List?): Boolean { + if (filtersList.isNullOrEmpty()) return false + val currentFilter = awaitState().headersData.currentFilter + return filtersList.contains(currentFilter) } - private fun getFilterTabsFlow(): Flow>> { + private fun getFilterTabsFlow(isEnabled: Boolean): Flow>> { + if (!isEnabled) return flowOf(Optional.empty()) val spaceFLow = spaceStateHandler.getSelectedSpaceFlow() .distinctUntilChanged() .onStart { @@ -252,44 +235,42 @@ class HomeRoomListViewModel @AssistedInject constructor( } } - private fun observeRooms() = viewModelScope.launch { - switchDataSourceMutex.withLock { - filteredPagedRoomSummariesLive?.livePagedList?.removeObserver(internalPagedListObserver) - - 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()) { - RoomSortOrder.NAME - } else { - RoomSortOrder.ACTIVITY - } - val liveResults = session.roomService().getFilteredPagedRoomSummariesLive( - params, - pagedListConfig, - sortOrder - ).also { - filteredPagedRoomSummariesLive = it - } - - liveResults.livePagedList.observeForever(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 (isAZOrdering) { + RoomSortOrder.NAME + } else { + RoomSortOrder.ACTIVITY + } + val liveResults = session.roomService().getFilteredPagedRoomSummariesLive( + params, + pagedListConfig, + sortOrder + ).also { + filteredPagedRoomSummariesLive = it + } + _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 { @@ -358,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 { @@ -393,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) { @@ -406,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 @@ -430,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) } diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewState.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewState.kt index c05e285ddb..ceaaf9b9dc 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewState.kt +++ b/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 diff --git a/vector/src/main/java/im/vector/app/features/home/room/list/home/header/RoomsHeadersData.kt b/vector/src/main/java/im/vector/app/features/home/room/list/home/header/RoomsHeadersData.kt index 87e45c99fa..db7f8b9a96 100644 --- a/vector/src/main/java/im/vector/app/features/home/room/list/home/header/RoomsHeadersData.kt +++ b/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? = null, - val currentFilter: HomeRoomFilter? = null, + val currentFilter: HomeRoomFilter = HomeRoomFilter.ALL, val recents: List? = null )