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.

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

Fix broken pagination for lists tab on profile page #6221

Merged
merged 7 commits into from
Nov 23, 2024

Conversation

khuddite
Copy link
Contributor

Fixes: #6204

  1. Summary
    onEndReached is not triggered for the ProfileLists component, and it seems onEndReachedThreshold must be set.
    The issue is specific to the desktop.

  2. Solution
    Set onEndReachedThreshold to 2. I've tested and confirmed it's working across all different platforms (android, iOS and web)

  3. Recording
    https://github.com/user-attachments/assets/f94a2f55-c6d7-4152-bf88-d69e0d38fe20

@gaearon
Copy link
Collaborator

gaearon commented Nov 11, 2024

it seems onEndReachedThreshold must be set.

It's marked as optional, so if it's not working without it, we should figure out why and fix that instead.

@khuddite
Copy link
Contributor Author

khuddite commented Nov 11, 2024

it seems onEndReachedThreshold must be set.

It's marked as optional, so if it's not working without it, we should figure out why and fix that instead.

That's a good question.
You're right - onEndReachedThreshold is optional on FlatList component, but we don't use it for desktop. It's used only for mobile. It seems we built our own list component for desktop - https://github.com/khuddite/social-app/blob/5da3f29498fda9ab1181df19a718e37099cb2cf6/src/view/com/util/List.web.tsx.

On this component, onEndReachedThreshold is optional, but it defaults to zero when missing. On the other hand, it defaults to 2 for mobile when missing - https://reactnative.dev/docs/virtualizedlist#onendreachedthreshold
That's probably why this issue is happening only on desktop.

I think onEndReachedThreshold being zero is a good reason for this issue because it impacts bottomMargin of the tail visibility component. bottomMargin={(onEndReachedThreshold ?? 0) * 100 + '%'}

I came up with the number 2 because it's used in other list tabs that don't have the same problem (like replies, posts, media, etc). onEndReachedThreshold={2} // number of posts left to trigger load more

@gaearon
Copy link
Collaborator

gaearon commented Nov 11, 2024

Hmm, is zero broken as in "it doesn't work even when reaching end directly"? We could maybe just change the default to 2 there (is that RN's default as well?) but I'd like to understand whether it's downright broken with 0 (not registering intersections at all), or whether this particular screen is just somehow harder to scroll exactly to the end.

@khuddite
Copy link
Contributor Author

I investigated a bit more, and here are my findings.

The lists tab has another issue - it's missing a footer component that displays a loading spinner when it reaches the end and pulls the next page. And it's related to the reported issue.

For other lists, when we don't have to display a loading spinner, we still display an empty view that has a certain height. And that seems required for the list to display the entire list without being cut off at the end and onEndReached to be triggered when it should be.

I've updated the ProfilesList component to include a footer and display a loading spinner while fetching the next page. This fixes the reported issue while making it consistent with other lists on the profile tab.

However, I think defaulting onEndReachedThreshold to 2 instead of 0 for the web list is still a good idea because that's how FlatList works on mobile. In the current implementation, if onEndReachedThreshold's missing, it will default to 2 for mobile and 0 for web, which doesn't seem desired.

@gaearon gaearon merged commit f802f81 into bluesky-social:main Nov 23, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Lists tab on the profile page: broken pagination
3 participants