-
Notifications
You must be signed in to change notification settings - Fork 306
Scroll-to-bottom button is abrupt and janks when scrolling a long way #1485
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
Labels
a-msglist
The message-list screen, except what's label:a-content
Milestone
Comments
gnprice
added a commit
to gnprice/zulip-flutter
that referenced
this issue
Apr 25, 2025
Fixes zulip#1485. This logic for deciding how long the scroll-to-bottom animation should take, introduced in 5abeb88 (zulip#223), didn't have any tests. As a result, when its unstated assumption about the message list -- that scrolling up from the end was represented by positive scroll positions -- was broken a few months later (in bdac26f), nothing alerted us to that. I did notice a few times over the past year or so that the effect of the scroll-to-bottom button seemed jerky, as if it were trying to move much farther in each frame than it should. Now I know why. (Discovered this in the course of revisiting this code in order to adapt it to the more radical change to the message list's scroll positions which is coming up: "zero" won't be the end, but somewhere in the middle.)
gnprice
added a commit
to gnprice/zulip-flutter
that referenced
this issue
Apr 28, 2025
Fixes zulip#1485. This logic for deciding how long the scroll-to-bottom animation should take, introduced in 5abeb88 (zulip#223), didn't have any tests. As a result, when its unstated assumption about the message list -- that scrolling up from the end was represented by positive scroll positions -- was broken a few months later (in bdac26f), nothing alerted us to that. I did notice a few times over the past year or so that the effect of the scroll-to-bottom button seemed jerky, as if it were trying to move much farther in each frame than it should. Now I know why. (Discovered this in the course of revisiting this code in order to adapt it to the more radical change to the message list's scroll positions which is coming up: "zero" won't be the end, but somewhere in the middle.)
gnprice
added a commit
to gnprice/zulip-flutter
that referenced
this issue
Apr 30, 2025
Fixes zulip#1485. This logic for deciding how long the scroll-to-bottom animation should take, introduced in 5abeb88 (zulip#223), didn't have any tests. As a result, when its unstated assumption about the message list -- that scrolling up from the end was represented by positive scroll positions -- was broken a few months later (in bdac26f), nothing alerted us to that. I did notice a few times over the past year or so that the effect of the scroll-to-bottom button seemed jerky, as if it were trying to move much farther in each frame than it should. Now I know why. (Discovered this in the course of revisiting this code in order to adapt it to the more radical change to the message list's scroll positions which is coming up: "zero" won't be the end, but somewhere in the middle.)
chrisbobbe
pushed a commit
that referenced
this issue
May 1, 2025
Fixes #1485. This logic for deciding how long the scroll-to-bottom animation should take, introduced in 5abeb88 (#223), didn't have any tests. As a result, when its unstated assumption about the message list -- that scrolling up from the end was represented by positive scroll positions -- was broken a few months later (in bdac26f), nothing alerted us to that. I did notice a few times over the past year or so that the effect of the scroll-to-bottom button seemed jerky, as if it were trying to move much farther in each frame than it should. Now I know why. (Discovered this in the course of revisiting this code in order to adapt it to the more radical change to the message list's scroll positions which is coming up: "zero" won't be the end, but somewhere in the middle.)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Background
In the message list, if you hit the scroll-to-bottom button, the intended behavior is that it scrolls to the bottom at an average speed of at most 8000 px/sec, the same as the top speed of a fling-scroll, and slower if necessary to make the animation take at least 300 ms.
So e.g. if you've scrolled up by 16000 logical pixels — roughly 20 screenfuls, since 160px has a nominal value of one inch — then the scroll to the bottom will take (16000 px)/(8000 px/sec) = 2 seconds. If you've scrolled up by only 1200 px (roughly 1.5 screenfuls), then the scroll to the bottom will be slower, at 4000 px/sec, in order to stretch it over 300 ms.
Bug
The logic to do that, though — introduced in 5abeb88 (#223) — didn't have any tests. As a result, when an unstated assumption it made got broken a few months later (in bdac26f), nothing alerted us to that. It's been broken since then.
The current behavior is instead that the animation is always set up to take just 300 ms, no matter how far it needs to travel. So for example if you're scrolled 16,000 px up (again ~20 screenfuls), then it will try to go a little over 53,000 px per second, which at 60 fps is 889 px/frame, i.e. likely more than a whole screenful per frame. (And in fact faster than that for part of the time: that's the average over the whole animation, but it uses an easing curve, so it goes faster early on and slower later.)
Scrolling at such ludicrous speed looks very flickery at best, i.e. if the app does successfully render each of those frames on time. It's also quite a strain on performance: the app needs to build, lay out, and render an entire new screenful of messages each frame, and if going by more than a screenful it also builds and lays out all the unseen messages in between in order to know how tall they are. So the actual behavior I usually see is that a lot of frames get dropped, i.e. jank.
Plan
I've spotted this behavior as a user a few times over the past year or so. I don't think we've heard anyone complain about it, though, and I think this button isn't central to the UX, so we haven't prioritized it. But I discovered the cause of the bug while reading the scroll-to-bottom button's code in the course of revising it to work at all in a world of #82, and I'll fix the bug as part of that work.
The text was updated successfully, but these errors were encountered: