Skip to content

Improve threshold behavior for fetching new batch of messages on scroll #445

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

Closed
sirpengi opened this issue Dec 8, 2023 · 2 comments
Closed
Labels
a-msglist The message-list screen, except what's label:a-content

Comments

@sirpengi
Copy link
Contributor

sirpengi commented Dec 8, 2023

As the user scrolls through the message list, we fetch a new batch of messages before they reach the end of the loaded history. If the user has a good network connection this is an amazing experience given they can continuously fling through history.

Currently we have a static value for the threshold at which we load extra messages (see kFetchMessagesBufferPixels in lib/widgets/message_list.dart) which currently evaluates to 4000. When it was first introduced in bccaead messages always had a recipient header attached, and since then not only are messages more compact (moreso due to work towards #157 ) but messages are grouped together by recipient header. A simple static value by itself is no longer sufficient, and we should probably dynamically adjust this value or change what we rely on to fetch new messages.

@gnprice
Copy link
Member

gnprice commented Dec 21, 2023

Before this will be actionable, we'll need to have an idea of what problem the existing behavior has that we want to solve.

I'm not actually sure the threshold shouldn't ultimately remain a fixed distance in pixels, as it is now. That corresponds to a fixed amount of scrolling, which seems more likely to be relevant in varying circumstances than the number of messages involved. (With #446, it'll actually become quite difficult for the user to even tell how many messages are on their screen.) It's possible that distance would ideally be larger or smaller than the value we have now, but the first step in making a change there would be to identify a problem that it might solve.

I believe the prompt for filing this issue was the test discussed at #446 (comment) , which can be fragile and need adjustment when the message layout changes. I'm not sure different threshold behavior here would actually fix that, though. As written, the test starts a fling-scroll at full speed and then fast-forwards to the end of it. That seems likely to skip right over the point where some other threshold would fire, just as it can skip right over the point where the existing threshold fires. It'd be good to make the test more robust, but that's probably best done by changes to the test.

@gnprice gnprice added the a-msglist The message-list screen, except what's label:a-content label Dec 21, 2023
@gnprice
Copy link
Member

gnprice commented May 9, 2024

Closing as not actionable. We can always open a new issue if there arises a concrete problem in this area that we want to fix.

@gnprice gnprice closed this as not planned Won't fix, can't repro, duplicate, stale May 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a-msglist The message-list screen, except what's label:a-content
Projects
Status: Done
Development

No branches or pull requests

2 participants