From 57e79b3f00bef3c025bdba6aab13dc8f5dc14fd8 Mon Sep 17 00:00:00 2001
From: Nik Clayton <nik@ngo.org.uk>
Date: Tue, 23 May 2023 20:07:39 +0200
Subject: [PATCH] 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.
---
 .../notifications/NotificationsFragment.kt    | 14 +++++++++++++
 .../notifications/NotificationsViewModel.kt   | 21 +++++++++++--------
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt
index b2eb1dac..ab5c1b53 100644
--- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsFragment.kt
@@ -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(
diff --git a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt
index 329bce64..3c07ed01 100644
--- a/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt
+++ b/app/src/main/java/com/keylesspalace/tusky/components/notifications/NotificationsViewModel.kt
@@ -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
      */