Unverified Commit 3f812722 authored by nt4f04uNd's avatar nt4f04uNd Committed by GitHub

RangeMaintainingScrollPhysics remove overscroll maintaining when grow (#90608)

parent 6184888e
...@@ -428,8 +428,12 @@ class ScrollPhysics { ...@@ -428,8 +428,12 @@ class ScrollPhysics {
/// Scroll physics that attempt to keep the scroll position in range when the /// Scroll physics that attempt to keep the scroll position in range when the
/// contents change dimensions suddenly. /// contents change dimensions suddenly.
/// ///
/// If the scroll position is already out of range, this attempts to maintain /// This attempts to maintain the amount of overscroll or underscroll already present,
/// the amount of overscroll or underscroll already present. /// if the scroll position is already out of range _and_ the extents
/// have decreased, meaning that some content was removed. The reason for this
/// condition is that when new content is added, keeping the same overscroll
/// would mean that instead of showing it to the user, all of it is
/// being skipped by jumping right to the max extent.
/// ///
/// If the scroll activity is animating the scroll position, sudden changes to /// If the scroll activity is animating the scroll position, sudden changes to
/// the scroll dimensions are allowed to happen (so as to prevent animations /// the scroll dimensions are allowed to happen (so as to prevent animations
...@@ -513,9 +517,9 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics { ...@@ -513,9 +517,9 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics {
maintainOverscroll = false; maintainOverscroll = false;
if (oldPosition.minScrollExtent.isFinite && oldPosition.maxScrollExtent.isFinite && if (oldPosition.minScrollExtent.isFinite && oldPosition.maxScrollExtent.isFinite &&
newPosition.minScrollExtent.isFinite && newPosition.maxScrollExtent.isFinite) { newPosition.minScrollExtent.isFinite && newPosition.maxScrollExtent.isFinite) {
// In addition, if the position changed then we only enforce // In addition, if the position changed then we don't enforce the new
// the new boundary if the previous boundary was not entirely // boundary if both the new and previous boundaries are entirely finite.
// finite. A common case where the position changes while one // A common case where the position changes while one
// of the extents is infinite is a lazily-loaded list. (If the // of the extents is infinite is a lazily-loaded list. (If the
// boundaries were finite, and the position changed, then we // boundaries were finite, and the position changed, then we
// assume it was intentional.) // assume it was intentional.)
...@@ -529,13 +533,19 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics { ...@@ -529,13 +533,19 @@ class RangeMaintainingScrollPhysics extends ScrollPhysics {
enforceBoundary = false; enforceBoundary = false;
} }
if (maintainOverscroll) { if (maintainOverscroll) {
// Force the new position to be no more out of range // Force the new position to be no more out of range than it was before, if:
// than it was before, if it was overscrolled. // * it was overscrolled, and
if (oldPosition.pixels < oldPosition.minScrollExtent) { // * the extents have decreased, meaning that some content was removed. The
// reason for this condition is that when new content is added, keeping
// the same overscroll would mean that instead of showing it to the user,
// all of it is being skipped by jumping right to the max extent.
if (oldPosition.pixels < oldPosition.minScrollExtent &&
newPosition.minScrollExtent > oldPosition.minScrollExtent) {
final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels; final double oldDelta = oldPosition.minScrollExtent - oldPosition.pixels;
return newPosition.minScrollExtent - oldDelta; return newPosition.minScrollExtent - oldDelta;
} }
if (oldPosition.pixels > oldPosition.maxScrollExtent) { if (oldPosition.pixels > oldPosition.maxScrollExtent &&
newPosition.maxScrollExtent < oldPosition.maxScrollExtent) {
final double oldDelta = oldPosition.pixels - oldPosition.maxScrollExtent; final double oldDelta = oldPosition.pixels - oldPosition.maxScrollExtent;
return newPosition.maxScrollExtent + oldDelta; return newPosition.maxScrollExtent + oldDelta;
} }
......
...@@ -230,7 +230,7 @@ void main() { ...@@ -230,7 +230,7 @@ void main() {
expect(bike2.center, bike1.shift(const Offset(100.0, 0.0)).center); expect(bike2.center, bike1.shift(const Offset(100.0, 0.0)).center);
}); });
testWidgets('Changing the size of the viewport while you are overdragged', (WidgetTester tester) async { testWidgets('changing the size of the viewport when overscrolled', (WidgetTester tester) async {
Widget build(double height) { Widget build(double height) {
return Directionality( return Directionality(
textDirection: TextDirection.rtl, textDirection: TextDirection.rtl,
...@@ -263,6 +263,82 @@ void main() { ...@@ -263,6 +263,82 @@ void main() {
final Rect newPosition = tester.getRect(find.byType(Placeholder)); final Rect newPosition = tester.getRect(find.byType(Placeholder));
expect(oldPosition, newPosition); expect(oldPosition, newPosition);
}); });
testWidgets('inserting and removing an item when overscrolled', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/62890
const double itemExtent = 100.0;
final UniqueKey key = UniqueKey();
final Finder finder = find.byKey(key);
Widget build({required bool twoItems}) {
return Directionality(
textDirection: TextDirection.rtl,
child: ScrollConfiguration(
behavior: const RangeMaintainingTestScrollBehavior(),
child: Align(
child: SizedBox(
width: 100.0,
height: 100.0,
child: ListView(
children: <Widget>[
SizedBox(height: itemExtent, child: Placeholder(key: key)),
if (twoItems)
const SizedBox(height: itemExtent, child: Placeholder()),
],
),
),
),
),
);
}
await tester.pumpWidget(build(twoItems: false));
final ScrollPosition position = tester.state<ScrollableState>(find.byType(Scrollable)).position;
// overscroll bottom
final TestGesture drag1 = await tester.startGesture(tester.getCenter(finder));
await tester.pump();
await drag1.moveBy(const Offset(0.0, -50.0));
await tester.pump();
final double oldOverscroll1 = position.pixels - position.maxScrollExtent;
final Rect oldPosition1 = tester.getRect(finder);
await tester.pumpWidget(build(twoItems: true));
// verify inserting new item didn't change the position of the first one
expect(oldPosition1, tester.getRect(finder));
// verify the overscroll changed by the size of the added item
final double newOverscroll1 = position.pixels - position.maxScrollExtent;
expect(oldOverscroll1, isPositive);
expect(newOverscroll1, isNegative);
expect(newOverscroll1, oldOverscroll1 - itemExtent);
await drag1.up();
// verify there's no ballistic animation, because we weren't overscrolled
expect(await tester.pumpAndSettle(), 1);
// overscroll bottom
final TestGesture drag2 = await tester.startGesture(tester.getCenter(finder));
await tester.pump();
await drag2.moveBy(const Offset(0.0, -100.0));
await tester.pump();
final double oldOverscroll2 = position.pixels - position.maxScrollExtent;
// should find nothing because item is not visible
expect(finder, findsNothing);
await tester.pumpWidget(build(twoItems: false));
// verify removing an item changed the position of the first one, because prior it was not visible
expect(oldPosition1, tester.getRect(finder));
// verify the overscroll was maintained
final double newOverscroll2 = position.pixels - position.maxScrollExtent;
expect(oldOverscroll2, isPositive);
expect(oldOverscroll2, newOverscroll2);
await drag2.up();
// verify there's a ballistic animation from overscroll
expect(await tester.pumpAndSettle(), 9);
});
} }
class TabBarDemo extends StatelessWidget { class TabBarDemo extends StatelessWidget {
......
...@@ -13,7 +13,7 @@ List<Widget> children(int n) { ...@@ -13,7 +13,7 @@ List<Widget> children(int n) {
} }
void main() { void main() {
testWidgets('Scrolling with list view changes', (WidgetTester tester) async { testWidgets('Scrolling with list view changes, leaving the overscroll', (WidgetTester tester) async {
final ScrollController controller = ScrollController(); final ScrollController controller = ScrollController();
await tester.pumpWidget(MaterialApp(home: ListView(controller: controller, children: children(30)))); await tester.pumpWidget(MaterialApp(home: ListView(controller: controller, children: children(30))));
final double thirty = controller.position.maxScrollExtent; final double thirty = controller.position.maxScrollExtent;
...@@ -22,7 +22,21 @@ void main() { ...@@ -22,7 +22,21 @@ void main() {
controller.jumpTo(thirty + 100.0); // past the end controller.jumpTo(thirty + 100.0); // past the end
await tester.pump(); await tester.pump();
await tester.pumpWidget(MaterialApp(home: ListView(controller: controller, children: children(31)))); await tester.pumpWidget(MaterialApp(home: ListView(controller: controller, children: children(31))));
expect(controller.position.pixels, thirty + 200.0); // same distance past the end expect(controller.position.pixels, thirty + 100.0); // has the same position, but no longer overscrolled
expect(await tester.pumpAndSettle(), 1); // doesn't have ballistic animation...
expect(controller.position.pixels, thirty + 100.0); // and ends up at the end
});
testWidgets('Scrolling with list view changes, remaining overscrolled', (WidgetTester tester) async {
final ScrollController controller = ScrollController();
await tester.pumpWidget(MaterialApp(home: ListView(controller: controller, children: children(30))));
final double thirty = controller.position.maxScrollExtent;
controller.jumpTo(thirty);
await tester.pump();
controller.jumpTo(thirty + 200.0); // past the end
await tester.pump();
await tester.pumpWidget(MaterialApp(home: ListView(controller: controller, children: children(31))));
expect(controller.position.pixels, thirty + 200.0); // has the same position, still overscrolled
expect(await tester.pumpAndSettle(), 8); // now it goes ballistic... expect(await tester.pumpAndSettle(), 8); // now it goes ballistic...
expect(controller.position.pixels, thirty + 100.0); // and ends up at the end expect(controller.position.pixels, thirty + 100.0); // and ends up at the end
}); });
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment