Skip to content

Fixes Room List not getting updated when fragment is not in focus #7186

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 21, 2022
Merged

Fixes Room List not getting updated when fragment is not in focus #7186

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
fe1e74f
Fixes room list not getting updated when not in focus
ericdecanini Sep 20, 2022
1a93bbf
Renames roomsPagedList
ericdecanini Sep 20, 2022
821636b
Adds changelog file
ericdecanini Sep 20, 2022
602b378
cancel flow when order is changed
Sep 21, 2022
d8060a7
review fixes
Sep 21, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/7186.bugfix
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fixes Room List not getting updated when fragment is not in focus
8 changes: 8 additions & 0 deletions ...r/src/main/java/im/vector/app/features/home/room/list/home/HomeFilteredRoomsController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -75,6 +76,13 @@ class HomeFilteredRoomsController @Inject constructor(
this.emptyStateData = state
}

fun submitPagedList(newList: PagedList<RoomSummary>) {
submitList(newList)
if (newList.isEmpty()) {
requestForcedModelBuild()
}
}

override fun buildItemModel(currentPosition: Int, item: RoomSummary?): EpoxyModel<*> {
return if (item == null) {
val host = this
Expand Down
8 changes: 4 additions & 4 deletions vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,10 +152,10 @@ class HomeRoomListFragment :
headersController.submitData(it)
}

roomListViewModel.onEach(HomeRoomListViewState::roomsLivePagedList) { roomsListLive ->
roomsListLive?.observe(viewLifecycleOwner) { roomsList ->
roomsController.submitList(roomsList)
if (roomsList.isEmpty()) {
roomListViewModel.onEach(HomeRoomListViewState::roomsPagedList) { roomsList ->
roomsList?.let {
roomsController.submitPagedList(it)
if (it.isEmpty()) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a bit weird to have both call to roomsController, this should be his responsability to handle empty list

Copy link
Contributor

@fedrunov fedrunov Sep 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What exactly do you mean? The problem here is that there are different empty states for different tabs, so we need to force rebuild models, to show different empty state view. But I can just create method in controller and move this code there 🤔

Copy link
Member

@ganfra ganfra Sep 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The controller should be able to handle this regarding the ViewState, but keep it like this for now.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

there are different empty states for different tabs, so we need to force rebuild models, to show different empty state view

This should be a comment in the code, since this is not usual to call requestForcedModelBuild()

roomsController.requestForcedModelBuild()
}
}
Expand Down
17 changes: 13 additions & 4 deletions vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,8 @@
package im.vector.app.features.home.room.list.home

import android.widget.ImageView
import androidx.lifecycle.asFlow
import androidx.paging.PagedList
import arrow.core.Option
import arrow.core.toOption
import com.airbnb.mvrx.MavericksViewModelFactory
import dagger.assisted.Assisted
Expand All @@ -35,6 +35,7 @@ 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.Job
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.MutableSharedFlow
import kotlinx.coroutines.flow.asSharedFlow
Expand Down Expand Up @@ -68,6 +69,7 @@ import org.matrix.android.sdk.api.session.room.state.isPublic
import org.matrix.android.sdk.api.util.Optional
import org.matrix.android.sdk.api.util.toMatrixItem
import org.matrix.android.sdk.flow.flow
import java.util.concurrent.CancellationException

class HomeRoomListViewModel @AssistedInject constructor(
@Assisted initialState: HomeRoomListViewState,
Expand All @@ -85,7 +87,6 @@ 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)
Expand All @@ -97,6 +98,8 @@ class HomeRoomListViewModel @AssistedInject constructor(
private val _emptyStateFlow = MutableSharedFlow<Optional<StateView.State.Empty>>(replay = 1)
val emptyStateFlow = _emptyStateFlow.asSharedFlow()

private var roomsFlowJob: Job? = null

private var filteredPagedRoomSummariesLive: UpdatableLivePageResult? = null

init {
Expand Down Expand Up @@ -251,10 +254,16 @@ class HomeRoomListViewModel @AssistedInject constructor(
)
emitEmptyState()
}
.also { roomsFlow = it }
.launchIn(viewModelScope)

setState { copy(roomsLivePagedList = liveResults.livePagedList) }
roomsFlowJob?.cancel(CancellationException())

roomsFlowJob = liveResults.livePagedList
.asFlow()
.onEach {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: you could use setOnEach

setState { copy(roomsPagedList = it) }
}
.launchIn(viewModelScope)
}

private fun observeOrderPreferences() {
Expand Down
3 changes: 1 addition & 2 deletions vector/src/main/java/im/vector/app/features/home/room/list/home/HomeRoomListViewState.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package im.vector.app.features.home.room.list.home

import androidx.lifecycle.LiveData
import androidx.paging.PagedList
import com.airbnb.mvrx.MavericksState
import im.vector.app.core.platform.StateView
Expand All @@ -26,5 +25,5 @@ import org.matrix.android.sdk.api.session.room.model.RoomSummary
data class HomeRoomListViewState(
val state: StateView.State = StateView.State.Content,
val headersData: RoomsHeadersData = RoomsHeadersData(),
val roomsLivePagedList: LiveData<PagedList<RoomSummary>>? = null,
val roomsPagedList: PagedList<RoomSummary>? = null,
) : MavericksState