Skip to content

Commit ab52f44

Browse files
gnpricechrisbobbe
authored andcommitted
msglist: Fix speed calculations in scroll to bottom
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.)
1 parent 8a2f774 commit ab52f44

File tree

2 files changed

+37
-2
lines changed

2 files changed

+37
-2
lines changed

lib/widgets/message_list.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -696,11 +696,12 @@ class ScrollToBottomButton extends StatelessWidget {
696696
final ScrollController scrollController;
697697

698698
Future<void> _scrollToBottom() {
699-
final distance = scrollController.position.pixels;
699+
final target = 0.0;
700+
final distance = target - scrollController.position.pixels;
700701
final durationMsAtSpeedLimit = (1000 * distance / 8000).ceil();
701702
final durationMs = max(300, durationMsAtSpeedLimit);
702703
return scrollController.animateTo(
703-
0,
704+
target,
704705
duration: Duration(milliseconds: durationMs),
705706
curve: Curves.ease);
706707
}

test/widgets/message_list_test.dart

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -510,6 +510,40 @@ void main() {
510510
// … and for good measure confirm the button disappeared.
511511
check(isButtonVisible(tester)).equals(false);
512512
});
513+
514+
testWidgets('scrolls at reasonable speed', (tester) async {
515+
const referenceSpeed = 8000.0;
516+
const distance = 40000.0;
517+
await setupMessageListPage(tester, messageCount: 1000);
518+
final controller = findMessageListScrollController(tester)!;
519+
520+
// Scroll a long distance up, many screenfuls.
521+
controller.jumpTo(-distance);
522+
await tester.pump();
523+
check(controller.position.pixels).equals(-distance);
524+
525+
// Tap button.
526+
await tester.tap(find.byType(ScrollToBottomButton));
527+
await tester.pump();
528+
529+
// Measure speed.
530+
final log = <double>[];
531+
double pos = controller.position.pixels;
532+
while (pos < 0) {
533+
check(log.length).isLessThan(30);
534+
await tester.pump(const Duration(seconds: 1));
535+
final lastPos = pos;
536+
pos = controller.position.pixels;
537+
log.add(pos - lastPos);
538+
}
539+
// Check the main question: the speed stayed in range throughout.
540+
const maxSpeed = 2 * referenceSpeed;
541+
check(log).every((it) => it..isGreaterThan(0)..isLessThan(maxSpeed));
542+
// Also check the test's assumptions: the scroll reached the end…
543+
check(pos).equals(0);
544+
// … and scrolled far enough to effectively test the max speed.
545+
check(log.sum).isGreaterThan(2 * maxSpeed);
546+
});
513547
});
514548

515549
group('TypingStatusWidget', () {

0 commit comments

Comments
 (0)