Skip to content

Commit 5233d6d

Browse files
committed
scroll: Avoid overscroll in "scroll to end"
As long as the bottom sliver is size zero (or more generally, as long as maxScrollExtent does not change during the animation), this is nearly NFC: I believe the only changes in behavior would come from differences in rounding. This change handles the case where the end turns out to be closer than it looked at the beginning of the animation. Before this change, the animation would try to scroll past the end in that case. Now it stops at exactly the end -- just like it already did in the case where the end was known exactly in advance, as it currently always is in the actual message list. That case is a possibility as soon as there's a bottom sliver with a message in it: scroll up so the message is offscreen and no longer built; then have the message edited so it becomes shorter; then scroll back down. It's impossible for the viewport to know that the bottom sliver's content has gotten taller until we actually scroll back down and cause the message's widget to get built.
1 parent e738a08 commit 5233d6d

File tree

2 files changed

+63
-0
lines changed

2 files changed

+63
-0
lines changed

lib/widgets/scrolling.dart

+25
Original file line numberDiff line numberDiff line change
@@ -262,6 +262,31 @@ class ScrollToEndActivity extends DrivenScrollActivity {
262262
required super.curve,
263263
required super.vsync,
264264
});
265+
266+
ScrollPosition get _position => delegate as ScrollPosition;
267+
268+
@override
269+
bool applyMoveTo(double value) {
270+
bool done = false;
271+
if (value > _position.maxScrollExtent) {
272+
// The activity has reached the end.
273+
// Stop at exactly the end, rather than causing overscroll.
274+
// Possibly some overscroll would actually be desirable, but:
275+
// TODO(upstream) stretch-overscroll seems busted, inverted:
276+
// Is this formula (from [_StretchController.absorbImpact] really right?
277+
// _stretchSizeTween.end =
278+
// math.min(_stretchIntensity + (_flingFriction / velocity), 1.0);
279+
// Seems to take low velocity to the largest stretch, and high velocity
280+
// to the smallest stretch.
281+
// Specifically, a very slow fling produces a very large stretch,
282+
// while other flings produce small stretches that vary little
283+
// between modest speed (~300 px/s) and top speed (8000 px/s).
284+
value = _position.maxScrollExtent;
285+
done = true;
286+
}
287+
if (!super.applyMoveTo(value)) return false;
288+
return !done;
289+
}
265290
}
266291

267292
/// A version of [ScrollPosition] adapted for the Zulip message list,

test/widgets/scrolling_test.dart

+38
Original file line numberDiff line numberDiff line change
@@ -417,6 +417,44 @@ void main() {
417417

418418
debugDefaultTargetPlatformOverride = null;
419419
});
420+
421+
testWidgets('on overscroll, stop', (tester) async {
422+
debugDefaultTargetPlatformOverride = TargetPlatform.iOS;
423+
await prepare(tester, topHeight: 400, bottomHeight: 1000);
424+
425+
// Scroll up…
426+
position.jumpTo(400);
427+
await tester.pump();
428+
check(position.extentAfter).equals(600);
429+
430+
// … then invoke `scrollToEnd`…
431+
position.scrollToEnd();
432+
await tester.pump();
433+
434+
// … but have the bottom sliver turn out to be shorter than it was.
435+
await prepare(tester, topHeight: 400, bottomHeight: 600,
436+
reuseController: true);
437+
check(position.extentAfter).equals(200);
438+
439+
// Let the scrolling animation proceed until it hits the end.
440+
int steps = 0;
441+
while (position.extentAfter > 0) {
442+
check(++steps).isLessThan(100);
443+
await tester.pump(Duration(milliseconds: 11));
444+
}
445+
446+
// This is the very first frame where the position reached the end.
447+
// It's at exactly the end, no farther…
448+
check(position.pixels - position.maxScrollExtent).equals(0);
449+
450+
// … and the animation is done. Nothing further happens.
451+
check(position.activity).isA<IdleScrollActivity>();
452+
await tester.pump(Duration(milliseconds: 11));
453+
check(position.pixels - position.maxScrollExtent).equals(0);
454+
check(position.activity).isA<IdleScrollActivity>();
455+
456+
debugDefaultTargetPlatformOverride = null;
457+
});
420458
});
421459
});
422460
}

0 commit comments

Comments
 (0)