Unverified Commit 4752b863 authored by yim's avatar yim Committed by GitHub

fix reorderable_list drop animation (#139362)

This PR fixes reorderable_list drop animation.

Fixes #138994
parent 12e883eb
...@@ -802,40 +802,27 @@ class SliverReorderableListState extends State<SliverReorderableList> with Ticke ...@@ -802,40 +802,27 @@ class SliverReorderableListState extends State<SliverReorderableList> with Ticke
} }
void _dragEnd(_DragInfo item) { void _dragEnd(_DragInfo item) {
// No changes required if last child is being inserted into the last position.
if ((_insertIndex! + 1 == _items.length) && _reverse) {
final RenderBox lastItemRenderBox = _items[_items.length - 1]!.context.findRenderObject()! as RenderBox;
final Offset lastItemOffset = lastItemRenderBox.localToGlobal(Offset.zero);
// When drag starts, the corresponding element is removed from
// the list, and moves inside of [ReorderableListState.CustomScrollView],
// which gives [CustomScrollView] a variable height.
//
// So when the element is moved, delta would change accordingly,
// and since it's the last element,
// we animate it back to it's position and add it back to the list.
final double delta = item.itemSize.height;
setState(() {
_finalDropPosition = Offset(lastItemOffset.dx, lastItemOffset.dy - delta);
});
return;
}
setState(() { setState(() {
if (_insertIndex == item.index) { if (_insertIndex == item.index) {
_finalDropPosition = _itemOffsetAt(_insertIndex! + (_reverse ? 1 : 0)); // Although it's at its original position, the original position has been replaced by a zero-size box
} else if (_insertIndex! < widget.itemCount - 1) { // So when reversed, it should offset its own extent
// Find the location of the item we want to insert before _finalDropPosition = _reverse ? _itemOffsetAt(_insertIndex!) - _extentOffset(item.itemExtent, _scrollDirection) : _itemOffsetAt(_insertIndex!);
_finalDropPosition = _itemOffsetAt(_insertIndex!); } else if (_reverse) {
if (_insertIndex! >= _items.length) {
// Drop at the starting position of the last element and offset its own extent
_finalDropPosition = _itemOffsetAt(_items.length - 1) - _extentOffset(item.itemExtent, _scrollDirection);
} else {
// Drop at the end of the current element occupying the insert position
_finalDropPosition = _itemOffsetAt(_insertIndex!) + _extentOffset(_itemExtentAt(_insertIndex!), _scrollDirection);
}
} else { } else {
// Inserting into the last spot on the list. If it's the only spot, put if (_insertIndex! == 0) {
// it back where it was. Otherwise, grab the second to last and move // Drop at the starting position of the first element and offset its own extent
// down by the gap. _finalDropPosition = _itemOffsetAt(0) - _extentOffset(item.itemExtent, _scrollDirection);
final int itemIndex = _items.length > 1 ? _insertIndex! - 1 : _insertIndex!;
if (_reverse) {
_finalDropPosition = _itemOffsetAt(itemIndex) - _extentOffset(item.itemExtent, _scrollDirection);
} else { } else {
_finalDropPosition = _itemOffsetAt(itemIndex) + _extentOffset(item.itemExtent, _scrollDirection); // Drop at the end of the previous element occupying the insert position
final int atIndex = _insertIndex! - 1;
_finalDropPosition = _itemOffsetAt(atIndex) + _extentOffset(_itemExtentAt(atIndex), _scrollDirection);
} }
} }
}); });
...@@ -975,8 +962,11 @@ class SliverReorderableListState extends State<SliverReorderableList> with Ticke ...@@ -975,8 +962,11 @@ class SliverReorderableListState extends State<SliverReorderableList> with Ticke
} }
Offset _itemOffsetAt(int index) { Offset _itemOffsetAt(int index) {
final RenderBox itemRenderBox = _items[index]!.context.findRenderObject()! as RenderBox; return _items[index]!.targetGeometry().topLeft;
return itemRenderBox.localToGlobal(Offset.zero); }
double _itemExtentAt(int index) {
return _sizeExtent(_items[index]!.targetGeometry().size, _scrollDirection);
} }
Widget _itemBuilder(BuildContext context, int index) { Widget _itemBuilder(BuildContext context, int index) {
......
...@@ -2,6 +2,7 @@ ...@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be // Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file. // found in the LICENSE file.
import 'package:collection/collection.dart';
import 'package:flutter/gestures.dart'; import 'package:flutter/gestures.dart';
import 'package:flutter/material.dart'; import 'package:flutter/material.dart';
import 'package:flutter/rendering.dart'; import 'package:flutter/rendering.dart';
...@@ -1405,6 +1406,84 @@ void main() { ...@@ -1405,6 +1406,84 @@ void main() {
await tester.pumpAndSettle(); await tester.pumpAndSettle();
}, },
); );
testWidgetsWithLeakTracking('Tests the correctness of the drop animation in various scenarios', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/138994
late Size screenSize;
final List<double> itemSizes = <double>[20, 50, 30, 80, 100, 30];
Future<void> pumpFor(bool reverse, Axis scrollDirection) async {
await tester.pumpWidget(
MaterialApp(
home: Builder(
builder: (BuildContext context) {
screenSize = MediaQuery.sizeOf(context);
return Scaffold(
body: CustomScrollView(
reverse: reverse,
scrollDirection: scrollDirection,
slivers: <Widget>[
SliverReorderableList(
itemBuilder: (BuildContext context, int index) {
return ReorderableDragStartListener(
key: ValueKey<int>(index),
index: index,
child: Builder(
builder: (BuildContext context) {
return SizedBox(
height: scrollDirection == Axis.vertical ? itemSizes[index] : double.infinity,
width: scrollDirection == Axis.horizontal ? itemSizes[index] : double.infinity,
child: Text('$index'),
);
},
),
);
},
itemCount: itemSizes.length,
onReorder: (int fromIndex, int toIndex) {},
),
],
),
);
}
),
),
);
}
Future<void> testMove(int from, int to, {bool reverse = false, Axis scrollDirection = Axis.vertical}) async {
await pumpFor(reverse, scrollDirection);
final double targetOffset = (List<double>.of(itemSizes)..removeAt(from)).sublist(0, to).sum;
final Offset targetPosition = reverse
? (scrollDirection == Axis.vertical
? Offset(0, screenSize.height - targetOffset - itemSizes[from])
: Offset(screenSize.width - targetOffset - itemSizes[from], 0))
: (scrollDirection == Axis.vertical ? Offset(0, targetOffset) : Offset(targetOffset, 0));
final Offset moveOffset = targetPosition - tester.getTopLeft(find.text('$from'));
await tester.timedDrag(find.text('$from'), moveOffset, const Duration(seconds: 1));
// Before the drop animation starts
final Offset animationBeginOffset = tester.getTopLeft(find.text('$from'));
// Halfway through the animation
await tester.pump(const Duration(milliseconds: 125));
expect(tester.getTopLeft(find.text('$from')), Offset.lerp(animationBeginOffset, targetPosition, 0.5));
// Animation ends
await tester.pump(const Duration(milliseconds: 125));
expect(tester.getTopLeft(find.text('$from')), targetPosition);
await tester.pumpAndSettle();
}
final List<(int,int)> testCases = <(int,int)>[
(3, 1),
(3, 3),
(3, 5),
(0, 5),
(5, 0),
];
for (final (int, int) element in testCases) {
await testMove(element.$1, element.$2);
await testMove(element.$1, element.$2, reverse: true);
await testMove(element.$1, element.$2, scrollDirection: Axis.horizontal);
await testMove(element.$1, element.$2, reverse: true, scrollDirection: Axis.horizontal);
}
});
} }
class TestList extends StatelessWidget { class TestList extends StatelessWidget {
......
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