Skip to content

Commit

Permalink
fix: Scroll and restore position correctly in Home and Notifications (#…
Browse files Browse the repository at this point in the history
…1240)

These are cached timelines, backed by Room. Room **requires** the
`PagingConfig` to have `enablePlaceholders = true`. Otherwise the list
is corrupted when scrolling down the list and paging in new items.

To restore the user's reading position correctly in the UI, wait for the
adapter to emit the very first page. Combine this with the user's
refresh key, and the number of placeholders in the page, to scroll the
user to the correct place in the list.

To make all this work, ensure that Room loads a large enough page of
data around the refresh key (in the `initialKey` calculation).
  • Loading branch information
nikclayton authored Jan 28, 2025
1 parent ebdd586 commit 0a42ab0
Show file tree
Hide file tree
Showing 6 changed files with 75 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@ import kotlin.properties.Delegates
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.conflate
import kotlinx.coroutines.flow.distinctUntilChangedBy
import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.take
Expand Down Expand Up @@ -219,6 +221,21 @@ class NotificationsFragment :

viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
launch {
// Wait for the very first page load, then scroll recyclerview
// to the refresh key.
viewModel.initialRefreshKey.combine(adapter.onPagesUpdatedFlow) { key, _ -> key }
.take(1)
.filterNotNull()
.collect { key ->
val snapshot = adapter.snapshot()
val index = snapshot.items.indexOfFirst { it.id == key }
binding.recyclerView.scrollToPosition(
snapshot.placeholdersBefore + index,
)
}
}

launch { viewModel.pagingData.collectLatest { adapter.submitData(it) } }

launch { viewModel.uiResult.collect(::bindUiResult) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ import kotlinx.coroutines.flow.filter
import kotlinx.coroutines.flow.filterIsInstance
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.flow.onStart
import kotlinx.coroutines.flow.receiveAsFlow
Expand Down Expand Up @@ -373,6 +374,10 @@ class NotificationsViewModel @AssistedInject constructor(
.filterNotNull()
.shareIn(viewModelScope, SharingStarted.WhileSubscribed(5000), replay = 1)

val initialRefreshKey = accountFlow.flatMapLatest {
flow { emit(repository.getRefreshKey(it.id)) }
}

/** The account to display notifications for */
val account: AccountEntity
get() = accountFlow.replayCache.first().entity
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,16 +80,16 @@ class CachedTimelineRepository @Inject constructor(

factory = InvalidatingPagingSourceFactory { timelineDao.getStatuses(account.id) }

val initialKey = remoteKeyDao.remoteKeyForKind(account.id, RKE_TIMELINE_ID, RemoteKeyKind.REFRESH)
val row = initialKey?.key?.let { timelineDao.getStatusRowNumber(account.id, it) }
val initialKey = remoteKeyDao.remoteKeyForKind(account.id, RKE_TIMELINE_ID, RemoteKeyKind.REFRESH)?.key
val row = initialKey?.let { timelineDao.getStatusRowNumber(account.id, it) } ?: 0

Timber.d("initialKey: %s is row: %d", initialKey, row)

return Pager(
initialKey = row,
initialKey = (row - ((PAGE_SIZE * 3) / 2)).coerceAtLeast(0),
config = PagingConfig(
pageSize = PAGE_SIZE,
enablePlaceholders = false,
enablePlaceholders = true,
),
remoteMediator = CachedTimelineRemoteMediator(
mastodonApi,
Expand Down Expand Up @@ -188,16 +188,26 @@ class CachedTimelineRepository @Inject constructor(
}

/**
* Saves the ID of the notification that future refreshes will try and restore
* Saves the ID of the status that future refreshes will try and restore
* from.
*
* @param pachliAccountId
* @param key Notification ID to restore from. Null indicates the refresh should
* refresh the newest notifications.
* @param key Status ID to restore from. Null indicates the refresh should
* refresh the newest statuses.
*/
suspend fun saveRefreshKey(pachliAccountId: Long, key: String?) = externalScope.async {
Timber.d("saveRefreshKey: $key")
remoteKeyDao.upsert(
RemoteKeyEntity(pachliAccountId, RKE_TIMELINE_ID, RemoteKeyKind.REFRESH, key),
)
}.await()

/**
* @param pachliAccountId
* @return The most recent saved refresh key. Null if not set, or the refresh
* should fetch the latest statuses.
*/
suspend fun getRefreshKey(pachliAccountId: Long): String? = externalScope.async {
remoteKeyDao.getRefreshKey(pachliAccountId, RKE_TIMELINE_ID)
}.await()
}
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,10 @@ import kotlin.time.Duration.Companion.seconds
import kotlinx.coroutines.delay
import kotlinx.coroutines.flow.collect
import kotlinx.coroutines.flow.collectLatest
import kotlinx.coroutines.flow.combine
import kotlinx.coroutines.flow.conflate
import kotlinx.coroutines.flow.distinctUntilChangedBy
import kotlinx.coroutines.flow.filterNotNull
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.onEach
import kotlinx.coroutines.flow.take
Expand Down Expand Up @@ -221,6 +223,23 @@ class TimelineFragment :

viewLifecycleOwner.lifecycleScope.launch {
viewLifecycleOwner.repeatOnLifecycle(Lifecycle.State.STARTED) {
launch {
// Wait for the very first page load, then scroll recyclerview
// to the refresh key.
(viewModel as? CachedTimelineViewModel)?.let { vm ->
vm.initialRefreshKey.combine(adapter.onPagesUpdatedFlow) { key, _ -> key }
.take(1)
.filterNotNull()
.collect { key ->
val snapshot = adapter.snapshot()
val index = snapshot.items.indexOfFirst { it.id == key }
binding.recyclerView.scrollToPosition(
snapshot.placeholdersBefore + index,
)
}
}
}

launch { viewModel.statuses.collectLatest { adapter.submitData(it) } }

launch { viewModel.uiResult.collect(::bindUiResult) }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ import javax.inject.Inject
import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.Flow
import kotlinx.coroutines.flow.flatMapLatest
import kotlinx.coroutines.flow.flow
import kotlinx.coroutines.flow.map
import kotlinx.coroutines.launch
import timber.log.Timber
Expand All @@ -69,6 +70,10 @@ class CachedTimelineViewModel @Inject constructor(
statusDisplayOptionsRepository,
sharedPreferencesRepository,
) {
val initialRefreshKey = accountFlow.flatMapLatest {
flow { emit(repository.getRefreshKey(it.data!!.id)) }
}

override var statuses = accountFlow.flatMapLatest {
getStatuses(it.data!!)
}.cachedIn(viewModelScope)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -110,14 +110,14 @@ class NotificationsRepository @Inject constructor(

// Room is row-keyed, not item-keyed. Find the user's REFRESH key, then find the
// row of the notification with that ID, and use that as the Pager's initialKey.
val initialKey = remoteKeyDao.remoteKeyForKind(pachliAccountId, RKE_TIMELINE_ID, RemoteKeyKind.REFRESH)
val row = initialKey?.key?.let { notificationDao.getNotificationRowNumber(pachliAccountId, it) }
val initialKey = remoteKeyDao.remoteKeyForKind(pachliAccountId, RKE_TIMELINE_ID, RemoteKeyKind.REFRESH)?.key
val row = initialKey?.let { notificationDao.getNotificationRowNumber(pachliAccountId, it) } ?: 0

return Pager(
initialKey = row,
initialKey = (row - ((PAGE_SIZE * 3) / 2)).coerceAtLeast(0),
config = PagingConfig(
pageSize = PAGE_SIZE,
enablePlaceholders = false,
enablePlaceholders = true,
),
remoteMediator = NotificationsRemoteMediator(
pachliAccountId,
Expand Down Expand Up @@ -147,8 +147,14 @@ class NotificationsRepository @Inject constructor(
)
}.await()

/** @return The notification ID to use when refreshing. */
suspend fun getRefreshKey(pachliAccountId: Long) = remoteKeyDao.getRefreshKey(pachliAccountId, RKE_TIMELINE_ID)
/**
* @param pachliAccountId
* @return The most recent saved refresh key. Null if not set, or the refresh
* should fetch the latest notifications.
*/
suspend fun getRefreshKey(pachliAccountId: Long): String? = externalScope.async {
remoteKeyDao.getRefreshKey(pachliAccountId, RKE_TIMELINE_ID)
}.await()

/**
* Clears (deletes) all notifications from the server. Invalidates the repository
Expand Down

0 comments on commit 0a42ab0

Please sign in to comment.