From 561c26ad4d2ca046d88cd92303e48f6c088cd77b Mon Sep 17 00:00:00 2001
From: Nik Clayton <nik@ngo.org.uk>
Date: Wed, 25 Sep 2024 11:45:33 +0200
Subject: [PATCH] fix: Save the reading position if no post is fully visible
 (#941)

Previous code saved the reading position of a fully visible status. But
there are situations where no status is fully visible.

1. The user is in the middle of viewing a status longer than the screen
height, and the top/bottom of the status are off the top/bottom of the
screen.

2. The user has scrolled between two statuses. Collectively they are
longer than the screen height, and the top of one status is off the top
of the screen and the bottom of the other status is off the bottom of
the screen.

In both cases the user's reading position was not saved.

In these situations use the ID of the status closest to the bottom of
the screen, even if not fully visible. This should ensure the user never
missing anything.

Fixes #936
---
 .../components/timeline/TimelineFragment.kt   | 31 +++++++++++++++----
 1 file changed, 25 insertions(+), 6 deletions(-)

diff --git a/app/src/main/java/app/pachli/components/timeline/TimelineFragment.kt b/app/src/main/java/app/pachli/components/timeline/TimelineFragment.kt
index df89bcfcf..8c1720db6 100644
--- a/app/src/main/java/app/pachli/components/timeline/TimelineFragment.kt
+++ b/app/src/main/java/app/pachli/components/timeline/TimelineFragment.kt
@@ -25,6 +25,8 @@ import android.view.MenuItem
 import android.view.View
 import android.view.ViewGroup
 import android.view.accessibility.AccessibilityManager
+import android.widget.Toast
+import android.widget.Toast.LENGTH_LONG
 import androidx.core.content.ContextCompat
 import androidx.core.view.MenuProvider
 import androidx.fragment.app.viewModels
@@ -39,6 +41,7 @@ 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 app.pachli.BuildConfig
 import app.pachli.R
 import app.pachli.adapter.StatusBaseViewHolder
 import app.pachli.components.timeline.util.isExpected
@@ -522,7 +525,7 @@ class TimelineFragment :
                 }
             }
             R.id.action_load_newest -> {
-                Timber.d("Reload because user choose load newest menu item")
+                Timber.d("Reload because user chose load newest menu item")
                 viewModel.accept(InfallibleUiAction.LoadNewest)
                 refreshContent()
                 true
@@ -532,15 +535,31 @@ class TimelineFragment :
     }
 
     /**
-     * Save [statusId] as the reading position. If null then the ID of the first completely visible
-     * status is used. It is the first status so that when performing a pull-refresh the
-     * previous first status always remains visible.
+     * Save [statusId] as the reading position. If null then the ID of the best status is used.
+     *
+     * The best status is the first completely visible status, if available. We assume the user
+     * has read this far, or will recognise it on return.
+     *
+     * However, there may not be a completely visible status. E.g., the user is viewing one
+     * status that is longer the the height of the screen, or the user is at the midpoint of
+     * two statuses that are each longer than half the height of the screen.
+     *
+     * In this case the best status is the last partially visible status, as we can assume the
+     * user has read this far.
      */
     fun saveVisibleId(statusId: String? = null) {
-        val id = statusId ?: layoutManager.findFirstCompletelyVisibleItemPosition()
-            .takeIf { it != RecyclerView.NO_POSITION }
+        val id = statusId ?: (
+            layoutManager.findFirstCompletelyVisibleItemPosition()
+                .takeIf { it != RecyclerView.NO_POSITION }
+                ?: layoutManager.findLastVisibleItemPosition()
+                    .takeIf { it != RecyclerView.NO_POSITION }
+            )
             ?.let { adapter.snapshot().getOrNull(it)?.id }
 
+        if (BuildConfig.DEBUG && id == null) {
+            Toast.makeText(requireActivity(), "Could not find ID of item to save", LENGTH_LONG).show()
+        }
+
         id?.let {
             Timber.d("Saving ID: %s", it)
             viewModel.accept(InfallibleUiAction.SaveVisibleId(visibleId = it))