Skip to content
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

Jump to bottom

Keep scroll position when loading missing statuses #3000

Merged
merged 36 commits into from
Jan 13, 2023
Merged

Keep scroll position when loading missing statuses #3000

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
b45d574
Change "Load more" to load oldest statuses first in home timeline
nikclayton Nov 29, 2022
34c5622
Scroll to the bottom of new entries added by "Load more"
nikclayton Nov 30, 2022
8d05e0a
Change "Load more" to load oldest statuses first in home timeline
nikclayton Nov 29, 2022
0808099
Scroll to the bottom of new entries added by "Load more"
nikclayton Nov 30, 2022
9c5ba19
Merge branch 'load-more-jump' of https://github.com/nikclayton/Tusky …
nikclayton Dec 3, 2022
a1c17b4
Ensure the user can't have two simultaneous "Load more" coroutines
nikclayton Dec 3, 2022
dbfc9e4
Merge branch 'tuskyapp:develop' into load-more-jump
nikclayton Dec 4, 2022
465fbff
Merge branch 'tuskyapp:develop' into load-more-jump
nikclayton Dec 5, 2022
b87961d
Revert changes that loaded the oldest statuses instead of the newest
nikclayton Dec 6, 2022
c871fc8
Be more robust about locating the status to scroll to
nikclayton Dec 6, 2022
90cb7b8
Lint
nikclayton Dec 6, 2022
7f8fe7d
Add a preference to specify the reading order
nikclayton Dec 7, 2022
ae413e8
Move ReadingOrder enum construction logic in to the enum
nikclayton Dec 7, 2022
edaa54b
Merge branch 'tuskyapp:develop' into load-more-jump
nikclayton Dec 9, 2022
1480c6a
Jump to top if swipe/refresh while preferring newest-first order
nikclayton Dec 12, 2022
528e74a
Show a circular progress spinner during "Load more" operations
nikclayton Dec 19, 2022
d517514
Remove the "loadMoreActive" property
nikclayton Dec 19, 2022
26622ca
Update comments in TimelineFragment
nikclayton Dec 19, 2022
a9e585f
Respect the user's reading order preference if it changes
nikclayton Dec 19, 2022
5a4a7b8
Add developer tools
nikclayton Dec 19, 2022
86d780e
Adjust how content is loaded based on preferred reading order
nikclayton Dec 19, 2022
2c102db
Lint
nikclayton Dec 19, 2022
7b42ef1
Rename unused dialog parameter to _
nikclayton Dec 19, 2022
3c36dab
Update API arguments in tests
nikclayton Dec 19, 2022
826ce0d
Simplify ReadingOrder preference handling
nikclayton Dec 21, 2022
90287f1
Merge branch 'develop' into load-more-jump
nikclayton Dec 24, 2022
82350b2
Fix bug with Placeholder and the "expanded" property
nikclayton Dec 25, 2022
854cddc
Set the "Load more" button background to transparent
nikclayton Dec 25, 2022
6fd0682
Merge branch 'develop' into load-more-jump
nikclayton Dec 31, 2022
a2ab730
Fix typo.
nikclayton Dec 31, 2022
5928159
Inline spec, update comment
nikclayton Jan 1, 2023
5eebe05
Revert 1480c6aa3ac5c0c2d362fb271f47ea2259ab14e2
nikclayton Jan 3, 2023
3d70fc6
Remove unnecessary Log call
nikclayton Jan 3, 2023
3f7db5f
Extract function
nikclayton Jan 3, 2023
a8e4d72
Merge branch 'develop' into load-more-jump
nikclayton Jan 13, 2023
d12c319
Change default to newest first
nikclayton Jan 13, 2023
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
41 changes: 37 additions & 4 deletions app/src/main/java/com/keylesspalace/tusky/MainActivity.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import android.content.Context
import android.content.DialogInterface
import android.content.Intent
import android.content.pm.PackageManager
import android.content.res.ColorStateList
import android.graphics.Bitmap
import android.graphics.Color
import android.graphics.drawable.Animatable
Expand Down Expand Up @@ -84,6 +83,7 @@ import com.keylesspalace.tusky.interfaces.ActionButtonActivity
import com.keylesspalace.tusky.interfaces.ReselectableFragment
import com.keylesspalace.tusky.pager.MainPagerAdapter
import com.keylesspalace.tusky.settings.PrefKeys
import com.keylesspalace.tusky.usecase.DeveloperToolsUseCase
import com.keylesspalace.tusky.usecase.LogoutUsecase
import com.keylesspalace.tusky.util.deleteStaleCachedMedia
import com.keylesspalace.tusky.util.emojify
Expand Down Expand Up @@ -141,6 +141,9 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
@Inject
lateinit var logoutUsecase: LogoutUsecase

@Inject
lateinit var developerToolsUseCase: DeveloperToolsUseCase

private val binding by viewBinding(ActivityMainBinding::inflate)

private lateinit var header: AccountHeaderView
Expand Down Expand Up @@ -559,16 +562,46 @@ class MainActivity : BottomSheetActivity(), ActionButtonActivity, HasAndroidInje
}

if (BuildConfig.DEBUG) {
// Add a "Developer tools" entry. Code that makes it easier to
// set the app state at runtime belongs here, it will never
// be exposed to users.
binding.mainDrawer.addItems(
DividerDrawerItem(),
secondaryDrawerItem {
nameText = "debug"
isEnabled = false
textColor = ColorStateList.valueOf(Color.GREEN)
nameText = "Developer tools"
isEnabled = true
iconicsIcon = GoogleMaterial.Icon.gmd_developer_mode
onClick = {
buildDeveloperToolsDialog().show()
}
}
)
}
}

private fun buildDeveloperToolsDialog(): AlertDialog {
return AlertDialog.Builder(this)
.setTitle("Developer Tools")
.setItems(
arrayOf("Create \"Load more\" gap")
) { _, which ->
Log.d(TAG, "Developer tools: $which")
when (which) {
0 -> {
Log.d(TAG, "Creating \"Load more\" gap")
lifecycleScope.launch {
accountManager.activeAccount?.let {
developerToolsUseCase.createLoadMoreGap(
it.id
)
}
}
}
}
}
.create()
}

override fun onSaveInstanceState(outState: Bundle) {
super.onSaveInstanceState(binding.mainDrawer.saveInstanceState(outState))
}
Expand Down
48 changes: 37 additions & 11 deletions app/src/main/java/com/keylesspalace/tusky/adapter/PlaceholderViewHolder.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,26 +15,52 @@
package com.keylesspalace.tusky.adapter

import android.view.View
import android.widget.Button
import android.widget.ProgressBar
import androidx.recyclerview.widget.RecyclerView
import com.google.android.material.button.MaterialButton
import com.google.android.material.progressindicator.CircularProgressIndicatorSpec
import com.google.android.material.progressindicator.IndeterminateDrawable
import com.keylesspalace.tusky.R
import com.keylesspalace.tusky.interfaces.StatusActionListener

/**
* Placeholder for different timelines.
* Either displays "load more" button or a progress indicator.
**/
*
* Displays a "Load more" button for a particular status ID, or a
* circular progress wheel if the status' page is being loaded.
*
* The user can only have one "Load more" operation in progress at
* a time (determined by the adapter), so the contents of the view
* and the enabled state is driven by that.
*/
class PlaceholderViewHolder(itemView: View) : RecyclerView.ViewHolder(itemView) {
private val loadMoreButton: Button = itemView.findViewById(R.id.button_load_more)
private val progressBar: ProgressBar = itemView.findViewById(R.id.progressBar)
private val loadMoreButton: MaterialButton = itemView.findViewById(R.id.button_load_more)
private val drawable = IndeterminateDrawable.createCircularDrawable(
itemView.context,
CircularProgressIndicatorSpec(itemView.context, null)
)

fun setup(listener: StatusActionListener, loading: Boolean) {
itemView.isEnabled = !loading
loadMoreButton.isEnabled = !loading

if (loading) {
loadMoreButton.text = ""
loadMoreButton.icon = drawable
return
}

loadMoreButton.text = itemView.context.getString(R.string.load_more_placeholder_text)
loadMoreButton.icon = null

fun setup(listener: StatusActionListener, progress: Boolean) {
loadMoreButton.visibility = if (progress) View.GONE else View.VISIBLE
progressBar.visibility = if (progress) View.VISIBLE else View.GONE
loadMoreButton.isEnabled = true
loadMoreButton.setOnClickListener { v: View? ->
// To allow the user to click anywhere in the layout to load more content set the click
// listener on the parent layout instead of loadMoreButton.
//
// See the comments in item_status_placeholder.xml for more details.
itemView.setOnClickListener {
connyduck marked this conversation as resolved.
Show resolved Hide resolved
itemView.isEnabled = false
loadMoreButton.isEnabled = false
loadMoreButton.icon = drawable
loadMoreButton.text = ""
listener.onLoadMore(bindingAdapterPosition)
}
}
Expand Down
30 changes: 30 additions & 0 deletions app/src/main/java/com/keylesspalace/tusky/components/preference/PreferencesFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,26 @@ class PreferencesFragment : PreferenceFragmentCompat(), Injectable {
private val iconSize by lazy { resources.getDimensionPixelSize(R.dimen.preference_icon_size) }
private var httpProxyPref: Preference? = null

enum class ReadingOrder {
/** User scrolls up, reading statuses oldest to newest */
OLDEST_FIRST,
Copy link
Collaborator

Choose a reason for hiding this comment

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

NEWEST_FIRST is the current behavior, right? Can we have it as the default please?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think NEWEST_FIRST is a bug, that this fixes.

From what I can tell, up to and including v16 beta 1 Tusky OLDEST_FIRST behaviour. I verified this by checking out 31da851, adding the DeveloperTools functionality, and experimenting with creating "Load more" gaps of different sizes and seeing how the app behaved when I clicked the button. It kept the position in the list.

I think the NEWEST_FIRST behaviour was introduced in 6d4f5ad with the migration to Paging3.

I can't see anything that suggests that this was a deliberate change, it's just how Paging3 works by default.

This change rapidly started annoying users, and they submitted issues about it. The earliest issue I can see is #2423 from Apr 7th.

I can't find any issues from before the change where users were asking for "Load more" to have its current behaviour. Only issues from after the change with users asking for the behaviour to be reverted.

This is why I think the best thing to do for the users is for the default to be the previous behaviour, which fixes the bug introduced in 6d4f5ad.

Copy link
Collaborator

Choose a reason for hiding this comment

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

But OLDEST_FIRST is loading the oldest statuses in the gap, we never did that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you don't do that (i.e,. use since_id), with this you can get the experience where you click "Load more", the statuses load, but they don't reach the bottom of the gap (i.e., the gap is more than LOAD_AT_ONCE statuses in size).

So then there's a new "Load more" button, and you have to click again. And potentially again.

This code, using min_id (and with OLDEST_FIRST) means the UX is to click "Load more", then scroll up and read the loaded statuses, eventually encounter another "Load more", click that, and repeat, which seems like the right way to do it (and is requested from the filed issues).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Using min_id is also consistent with Mastodon's paging in the link headers in HTTP responses.

Copy link
Collaborator

Choose a reason for hiding this comment

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

The default usage behavior is to read from newest to oldest - that's why the timeline is in reverse chronological order.
In that case, we want to load the newest statuses in the gap by default, so that the user who reads from newest to oldest gets an uninterrupted stream of posts. It's possible that there was a bug or otherwise incorrect behavior here in the past, but I believe that today's behavior is correct for this use case

I get that we want to have better support for the vocal minority of users who want to read every post on their timelines from oldest to newest, but we should absolutely not change such a fundamental default behavior on a whim or due to selection bias.

Copy link
Collaborator

Choose a reason for hiding this comment

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

As a side note, I have felt like I have maybe been coming across as adversarial in some of these discussions, and I've been trying to limit my participation for that reason. However, I also feel like there's an abnormally high amount of talking past each other going on, and I don't understand whether it's because one or more of us are fundamentally misunderstanding what the others are saying, or there's some hidden assumption, or something else.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If it helps, I haven't been interpreting your feedback as adversarial.

To address what I believe are your key points:

"The default usage behavior is to read from newest to oldest"

That's not always true, the app is inconsistent about this behaviour.

For example:

  1. Check out the current tip of develop (eb02d0b at the time I'm writing this)
  2. Launch on a device or emulator
  3. Open the Federated timeline (just because it changes very quickly and you don't have to wait long)
  4. Scroll to the very top, and swipe-to-refresh to make sure you have the most recent statuses
  5. Wait maybe 10 seconds, so that new statuses are available
  6. Swipe-to-refresh to load new statuses

You should observe that your position in the timeline is retained, and the new statuses are loaded above the one you are currently looking at. This is oldest-first behaviour.

This behaviour is the same on at least the Federated, Local, and Home timelines.

If "the default usage behaviour is to read from newest to oldest" was correct, refreshing any of these timelines should take you to the newest of the freshly loaded statuses.

I suspect this is the source of some of the demand for this behaviour from users. They observe that, in some situations the app behaves the way that they expect, but in others it doesn't.

This also gets weird if there were more than LOAD_AT_ONCE statuses loaded and there's also a "Load more" placeholder. Because if the user clicks that then the statuses are loaded below their current position.

So swipe = load above and keep my position, "Load more" = load below and lose my position

"vocal minority"

I agree about "vocal". There are at least 14 issues that reference this, many of which either have comments from other users saying they also want the oldest-first behaviour, or heart / thumbs-up emoji from people who haven't commented but also indicating that they want the oldest-first behaviour.

I don't think the assertion that it's a minority is knowable.

Creating a Github issue or commenting on it has a fairly high barrier to entry -- you have to know where to file the issue, you have to create a Github account if you don't already have one, and you have to take the time to actually write the issue.

Participation inequality is a thing (https://www.nngroup.com/articles/participation-inequality/), and I think it's reasonable to assume that for every 1 user who has taken the time to report this issue, there are N other users who either have the same problem, or have abandoned using the app in favour of a different one.

Unfortunately, we don't know what N is. Could be 10, could be 100, could be 1,000, or more, or less.

This also extends to Play reviews. It doesn't appear to be possible to link to individual reviews, so, from the last month or so:

"Tusky experience (if you try to read from oldest to newest): Scrolling up. Tap "Load more". Scroll down to find where you stopped before it loaded newest posts and lost scrolling position. Tap "Load more" again, rushing to scroll down so that you don't lose scrolling progress. "Load more". "Load more". Repeat 15 times [...]" 2 stars, "3 people found this review helpful"

"Looks pretty good. Nice and clean view. However the load more button defaults to going to the top of your feed losing position. Can this be switched or an option included?", 3 stars, "4 people found this review helpful"

"This App is okay for the most cases. But the main feature is complety insane. The timeline is always at the latest twee...äh..toot(?). You can't continue reading the toots from the place were you stopped reading. You always have to start at the newest toot and must search were you left", 2 stars, "25 people found this review helpful"

My intuition from all this is that N is a large number, and that changing the default approach to oldest-first is a net-win for the majority of the userbase.

You, reasonably, disagree.

We can't both be right.

Proposal

The Tusky Mastodon account has about 15K followers.

Create a poll, run it for 7 days, and boost it once per day for those 7 days. Suggested text:

We are thinking of changing the default "Load more" behaviour (https://github.com/tuskyapp/Tusky/pull/3000)

Which do you prefer when you press the "Load more" button?

We'll support both, this is about what the *default* should be.

1. Jump to the newest post (current behaviour)
2. Stay at the oldest post

Use the result of that to determine the default.

Copy link
Collaborator

Choose a reason for hiding this comment

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

"There are behavioral inconsistencies" is not the same as "the default is secretly the opposite of what you think".

I agree that when the user actively refreshes, the timeline should move to the top (in the default newest-first order).

There's no point in having a poll about the load more behavior, because it should be determined by the reading order, where it doesn't "jump" in either case, but remains at the user's last-read (from the default top or optional bottom) post.

Copy link
Contributor Author

@nikclayton nikclayton Jan 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

"There are behavioral inconsistencies" is not the same as "the default is secretly the opposite of what you think".

That is true.

But that's not what I'm saying.

I'm saying that the default behaviour is inconsistent and is a source (probably a significant one) of user confusion and dissatisfaction with the app.

By pointing out the inconsistency, I intend to suggest that when you say "I believe that today's behavior is correct for this use case" I think you may not have considered all the ways that the app behaves.

In response, you suggested "I agree that when the user actively refreshes, the timeline should move to the top (in the default newest-first order)."

That is something that we could do (not in this PR, and I'm not suggesting we actually do it, I'm using "could" to mean only "This is a technically possible change").

But you've also said (about default reading order) "we should absolutely not change such a fundamental default behavior on a whim or due to selection bias."

I agree with that too -- that's why I suggested the poll, so that it's not a whim, or selection bias.

But I feel this is also inconsistent. On one hand you are suggesting that we should not change fundamental default behaviour like this, on the other hand you suggest we should change the user's reading position when they swipe-to-refresh, which I think would definitely be a change to fundamental behaviour.

I think that may be why some of this discussion is challenging; it feels like you are arguing two different and opposite positions, depending on the behaviour that you want to see, and it's difficult to address your concerns because of that.

Aside: As I've already said, I agree with you when you say "absolutely not change such a fundamental default behaviour on a whim or due to selection bias".

And I absolutely do not want to discuss the specific decision here and derail the conversation, but I'll note that has absolutely happened in the past, probably most recently with #2552.

Finally:

There's no point in having a poll about the load more behavior, because it should be determined by the reading order, where it doesn't "jump" in either case, but remains at the user's last-read (from the default top or optional bottom) post.

That's exactly what this PR does (once I revert 1480c6a, see my other reply #3000 (comment) about that).

[Edit: I have now reverted that change]

The only thing the poll would be for is to decide the default value for the preference, OLDEST_FIRST or NEWEST_FIRST.


/** User scrolls down, reading statuses newest to oldest. Default behaviour. */
NEWEST_FIRST;

companion object {
fun from(s: String?): ReadingOrder {
s ?: return NEWEST_FIRST

return try {
valueOf(s.uppercase())
} catch (_: Throwable) {
NEWEST_FIRST
}
}
}
}

override fun onCreatePreferences(savedInstanceState: Bundle?, rootKey: String?) {
makePreferenceScreen {
preferenceCategory(R.string.pref_title_appearance_settings) {
Expand Down Expand Up @@ -90,6 +110,16 @@ class PreferencesFragment : PreferenceFragmentCompat(), Injectable {
icon = makeIcon(GoogleMaterial.Icon.gmd_format_size)
}

listPreference {
setDefaultValue(ReadingOrder.NEWEST_FIRST.name)
setEntries(R.array.reading_order_names)
setEntryValues(R.array.reading_order_values)
key = PrefKeys.READING_ORDER
setSummaryProvider { entry }
setTitle(R.string.pref_title_reading_order)
icon = makeIcon(GoogleMaterial.Icon.gmd_sort)
}

listPreference {
setDefaultValue("top")
setEntries(R.array.pref_main_nav_position_options)
Expand Down
73 changes: 73 additions & 0 deletions app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ import com.keylesspalace.tusky.adapter.StatusBaseViewHolder
import com.keylesspalace.tusky.appstore.EventHub
import com.keylesspalace.tusky.appstore.PreferenceChangedEvent
import com.keylesspalace.tusky.appstore.StatusComposedEvent
import com.keylesspalace.tusky.components.preference.PreferencesFragment.ReadingOrder
import com.keylesspalace.tusky.components.timeline.viewmodel.CachedTimelineViewModel
import com.keylesspalace.tusky.components.timeline.viewmodel.NetworkTimelineViewModel
import com.keylesspalace.tusky.components.timeline.viewmodel.TimelineViewModel
Expand All @@ -62,6 +63,7 @@ import com.keylesspalace.tusky.util.hide
import com.keylesspalace.tusky.util.show
import com.keylesspalace.tusky.util.viewBinding
import com.keylesspalace.tusky.viewdata.AttachmentViewData
import com.keylesspalace.tusky.viewdata.StatusViewData
import io.reactivex.rxjava3.android.schedulers.AndroidSchedulers
import io.reactivex.rxjava3.core.Observable
import kotlinx.coroutines.flow.collectLatest
Expand Down Expand Up @@ -101,6 +103,38 @@ class TimelineFragment :
private var isSwipeToRefreshEnabled = true
private var hideFab = false

/**
* Adapter position of the placeholder that was most recently clicked to "Load more". If null
* then there is no active "Load more" operation
*/
private var loadMorePosition: Int? = null

/** ID of the status immediately below the most recent "Load more" placeholder click */
// The Paging library assumes that the user will be scrolling down a list of items,
// and if new items are loaded but not visible then it's reasonable to scroll to the top
// of the inserted items. It does not seem to be possible to disable that behaviour.
//
// That behaviour should depend on the user's preferred reading order. If they prefer to
// read oldest first then the list should be scrolled to the bottom of the freshly
// inserted statuses.
//
// To do this:
//
// 1. When "Load more" is clicked (onLoadMore()):
// a. Remember the adapter position of the "Load more" item in loadMorePosition
// b. Remember the ID of the status immediately below the "Load more" item in
// statusIdBelowLoadMore
// 2. After the new items have been inserted, search the adapter for the position of the
// status with id == statusIdBelowLoadMore.
// 3. If this position is still visible on screen then do nothing, otherwise, scroll the view
// so that the status is visible.
//
// The user can then scroll up to read the new statuses.
private var statusIdBelowLoadMore: String? = null

/** The user's preferred reading order */
private lateinit var readingOrder: ReadingOrder

override fun onCreate(savedInstanceState: Bundle?) {
super.onCreate(savedInstanceState)

Expand Down Expand Up @@ -130,6 +164,8 @@ class TimelineFragment :
isSwipeToRefreshEnabled = arguments.getBoolean(ARG_ENABLE_SWIPE_TO_REFRESH, true)

val preferences = PreferenceManager.getDefaultSharedPreferences(requireContext())
readingOrder = ReadingOrder.from(preferences.getString(PrefKeys.READING_ORDER, null))

val statusDisplayOptions = StatusDisplayOptions(
animateAvatars = preferences.getBoolean(PrefKeys.ANIMATE_GIF_AVATARS, false),
mediaPreviewEnabled = accountManager.activeAccount!!.mediaPreviewEnabled,
Expand Down Expand Up @@ -207,6 +243,9 @@ class TimelineFragment :
}
}
}
if (readingOrder == ReadingOrder.OLDEST_FIRST) {
updateReadingPositionForOldestFirst()
}
}
})

Expand Down Expand Up @@ -253,6 +292,33 @@ class TimelineFragment :
}
}

/**
* Set the correct reading position in the timeline after the user clicked "Load more",
* assuming the reading position should be below the freshly-loaded statuses.
*/
// Note: The positionStart parameter to onItemRangeInserted() does not always
// match the adapter position where data was inserted (which is why loadMorePosition
// is tracked manually, see this bug report for another example:
// https://github.com/android/architecture-components-samples/issues/726).
private fun updateReadingPositionForOldestFirst() {
var position = loadMorePosition ?: return
val statusIdBelowLoadMore = statusIdBelowLoadMore ?: return

var status: StatusViewData?
while (adapter.peek(position).let { status = it; it != null }) {
if (status?.id == statusIdBelowLoadMore) {
val lastVisiblePosition =
(binding.recyclerView.layoutManager as LinearLayoutManager).findLastVisibleItemPosition()
if (position > lastVisiblePosition) {
binding.recyclerView.scrollToPosition(position)
}
break
}
position++
}
loadMorePosition = null
}

private fun setupSwipeRefreshLayout() {
binding.swipeRefreshLayout.isEnabled = isSwipeToRefreshEnabled
binding.swipeRefreshLayout.setOnRefreshListener(this)
Expand Down Expand Up @@ -344,6 +410,8 @@ class TimelineFragment :

override fun onLoadMore(position: Int) {
val placeholder = adapter.peek(position)?.asPlaceholderOrNull() ?: return
loadMorePosition = position
statusIdBelowLoadMore = adapter.peek(position + 1)?.id
viewModel.loadMore(placeholder.id)
}

Expand Down Expand Up @@ -404,6 +472,11 @@ class TimelineFragment :
adapter.notifyItemRangeChanged(0, adapter.itemCount)
}
}
PrefKeys.READING_ORDER -> {
readingOrder = ReadingOrder.from(
sharedPreferences.getString(PrefKeys.READING_ORDER, null)
)
}
}
}

Expand Down
6 changes: 5 additions & 1 deletion app/src/main/java/com/keylesspalace/tusky/components/timeline/TimelineTypeMappers.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

package com.keylesspalace.tusky.components.timeline

import android.util.Log
import com.google.gson.Gson
import com.google.gson.reflect.TypeToken
import com.keylesspalace.tusky.db.TimelineAccountEntity
Expand All @@ -30,6 +31,8 @@ import com.keylesspalace.tusky.entity.TimelineAccount
import com.keylesspalace.tusky.viewdata.StatusViewData
import java.util.Date

private const val TAG = "TimelineTypeMappers"

data class Placeholder(
val id: String,
val loading: Boolean
Expand Down Expand Up @@ -150,7 +153,8 @@ fun Status.toEntity(
}

fun TimelineStatusWithAccount.toViewData(gson: Gson, isDetailed: Boolean = false): StatusViewData {
if (this.status.authorServerId == null) {
if (this.status.isPlaceholder) {
Log.d(TAG, "Constructing Placeholder(${this.status.serverId}, ${this.status.expanded})")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Remove?

return StatusViewData.Placeholder(this.status.serverId, this.status.expanded)
}

Expand Down
9 changes: 8 additions & 1 deletion ...ava/com/keylesspalace/tusky/components/timeline/viewmodel/CachedTimelineRemoteMediator.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,14 @@ class CachedTimelineRemoteMediator(
if (oldStatus != null) break
}

val expanded = oldStatus?.expanded ?: activeAccount.alwaysOpenSpoiler
// The "expanded" property for Placeholders determines whether or not they are
// in the "loading" state, and should not be affected by the account's
// "alwaysOpenSpoiler" preference
val expanded = if (oldStatus?.isPlaceholder == true) {
oldStatus.expanded
} else {
oldStatus?.expanded ?: activeAccount.alwaysOpenSpoiler
}
val contentShowing = oldStatus?.contentShowing ?: activeAccount.alwaysShowSensitiveMedia || !status.actionableStatus.sensitive
val contentCollapsed = oldStatus?.contentCollapsed ?: true

Expand Down
Loading