Save and restore the user's reading position more frequently (#3685)

The previous code gets the user's reading position *once*, when the
viewmodel is created, and uses that whenever it needs to be restored.

This is a problem. Suppose the user is a few days behind on their
notifications, and opens the app.

The reading position is restored, as expected. They scroll up to start
reading newer notifications.

Then they change their notification filters. This causes the
notifications list to change, and when it does their reading position
is set back to what it was when they first switched to the Notifications
tab.

Fix this by:

NotificationsFragment:
- Save the reading position whenever the user stops scrolling.

NotificationsViewModel:
- Use the saved reading position whenever the list of notifications
  can change, not just when the view model is created.
This commit is contained in:
Nik Clayton 2023-05-23 20:07:39 +02:00 committed by GitHub
parent abf15ccfde
commit 57e79b3f00
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
2 changed files with 26 additions and 9 deletions

View file

@ -41,6 +41,7 @@ import androidx.recyclerview.widget.DiffUtil
import androidx.recyclerview.widget.DividerItemDecoration
import androidx.recyclerview.widget.LinearLayoutManager
import androidx.recyclerview.widget.RecyclerView
import androidx.recyclerview.widget.RecyclerView.SCROLL_STATE_IDLE
import androidx.recyclerview.widget.SimpleItemAnimator
import androidx.swiperefreshlayout.widget.SwipeRefreshLayout.OnRefreshListener
import at.connyduck.sparkbutton.helpers.Utils
@ -193,6 +194,19 @@ class NotificationsFragment :
}
}
}
@Suppress("SyntheticAccessor")
override fun onScrollStateChanged(recyclerView: RecyclerView, newState: Int) {
newState != SCROLL_STATE_IDLE && return
// Save the ID of the first notification visible in the list, so the user's
// reading position is always restorable.
layoutManager.findFirstVisibleItemPosition().takeIf { it >= 0 }?.let { position ->
adapter.snapshot().getOrNull(position)?.id?.let { id ->
viewModel.accept(InfallibleUiAction.SaveVisibleId(visibleId = id))
}
}
}
})
binding.recyclerView.adapter = adapter.withLoadStateHeaderAndFooter(

View file

@ -455,17 +455,9 @@ class NotificationsViewModel @Inject constructor(
}
}
// The database stores "0" as the last notification ID if notifications have not been
// fetched. Convert to null to ensure a full fetch in this case
val lastNotificationId = when (val id = accountManager.activeAccount?.lastNotificationId) {
"0" -> null
else -> id
}
Log.d(TAG, "Restoring at $lastNotificationId")
pagingData = notificationFilter
.flatMapLatest { action ->
getNotifications(filters = action.filter, initialKey = lastNotificationId)
getNotifications(filters = action.filter, initialKey = getInitialKey())
}
.cachedIn(viewModelScope)
@ -499,6 +491,17 @@ class NotificationsViewModel @Inject constructor(
}
}
// The database stores "0" as the last notification ID if notifications have not been
// fetched. Convert to null to ensure a full fetch in this case
private fun getInitialKey(): String? {
val initialKey = when (val id = accountManager.activeAccount?.lastNotificationId) {
"0" -> null
else -> id
}
Log.d(TAG, "Restoring at $initialKey")
return initialKey
}
/**
* @return Flow of relevant preferences that change the UI
*/