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 notifications pagination on web #4400

Merged
merged 2 commits into from
Jun 6, 2024
Merged

Conversation

pfrazee
Copy link
Collaborator

@pfrazee pfrazee commented Jun 6, 2024

Reverts #4235

It looks like the issue is that fetching the next page doesn't reliably add 2x list height to the scrollview. The intersection observer only tells us when the boundary is crossed -- it doesn't trigger if we're still in the boundary -- and so the next page successfully fires but then doesn't retrigger.

It's entirely possible, with client-side filtering, that even dropping the threshold down to 60% doesn't totally fix the issue.

  • We could possibly solve this by calculating the number of new list items getting added post-grouping and, if it's too low, queue another fetch. That feels brittle though.
  • We could look for some way to fire an event post-layout, perhaps by running checkVisiblity on the bottom of the screen.

Copy link

render bot commented Jun 6, 2024

@haileyok haileyok self-requested a review June 6, 2024 19:03
Copy link

github-actions bot commented Jun 6, 2024

Old size New size Diff
7.39 MB 7.39 MB 6 B (0.00%)

@pfrazee pfrazee merged commit fef16e0 into main Jun 6, 2024
6 checks passed
@pfrazee pfrazee deleted the paul/fix-notifs-pagination branch June 6, 2024 19:10
@gaearon
Copy link
Collaborator

gaearon commented Jun 6, 2024

We could maybe change the logic to keep trying until we either run out of pages or the tail runs out of viewport again.

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.

3 participants