Unverified Commit 31191dd2 authored by xubaolin's avatar xubaolin Committed by GitHub

Fixes some widgets(`ListView.builder`, `GridView.builder` etc.) state-lose issue (#100547)

* ++

* ++

* ++

* ++

* codereview feedback

* ++
parent c920ac2b
...@@ -339,6 +339,7 @@ class SliverAnimatedList extends StatefulWidget { ...@@ -339,6 +339,7 @@ class SliverAnimatedList extends StatefulWidget {
const SliverAnimatedList({ const SliverAnimatedList({
Key? key, Key? key,
required this.itemBuilder, required this.itemBuilder,
this.findChildIndexCallback,
this.initialItemCount = 0, this.initialItemCount = 0,
}) : assert(itemBuilder != null), }) : assert(itemBuilder != null),
assert(initialItemCount != null && initialItemCount >= 0), assert(initialItemCount != null && initialItemCount >= 0),
...@@ -359,6 +360,9 @@ class SliverAnimatedList extends StatefulWidget { ...@@ -359,6 +360,9 @@ class SliverAnimatedList extends StatefulWidget {
/// [SliverAnimatedListState.removeItem] removes an item immediately. /// [SliverAnimatedListState.removeItem] removes an item immediately.
final AnimatedListItemBuilder itemBuilder; final AnimatedListItemBuilder itemBuilder;
/// {@macro flutter.widgets.SliverChildBuilderDelegate.findChildIndexCallback}
final ChildIndexGetter? findChildIndexCallback;
/// {@macro flutter.widgets.animatedList.initialItemCount} /// {@macro flutter.widgets.animatedList.initialItemCount}
final int initialItemCount; final int initialItemCount;
...@@ -507,7 +511,11 @@ class SliverAnimatedListState extends State<SliverAnimatedList> with TickerProvi ...@@ -507,7 +511,11 @@ class SliverAnimatedListState extends State<SliverAnimatedList> with TickerProvi
} }
SliverChildDelegate _createDelegate() { SliverChildDelegate _createDelegate() {
return SliverChildBuilderDelegate(_itemBuilder, childCount: _itemsCount); return SliverChildBuilderDelegate(
_itemBuilder,
childCount: _itemsCount,
findChildIndexCallback: widget.findChildIndexCallback,
);
} }
/// Insert an item at [index] and start an animation that will be passed to /// Insert an item at [index] and start an animation that will be passed to
......
...@@ -665,9 +665,14 @@ class PageView extends StatefulWidget { ...@@ -665,9 +665,14 @@ class PageView extends StatefulWidget {
/// [itemBuilder] will be called only with indices greater than or equal to /// [itemBuilder] will be called only with indices greater than or equal to
/// zero and less than [itemCount]. /// zero and less than [itemCount].
/// ///
/// [PageView.builder] by default does not support child reordering. If /// {@template flutter.widgets.PageView.findChildIndexCallback}
/// you are planning to change child order at a later time, consider using /// The [findChildIndexCallback] corresponds to the
/// [PageView] or [PageView.custom]. /// [SliverChildBuilderDelegate.findChildIndexCallback] property. If null,
/// a child widget may not map to its existing [RenderObject] when the order
/// of children returned from the children builder changes.
/// This may result in state-loss. This callback needs to be implemented if
/// the order of the children may change at a later time.
/// {@endtemplate}
/// ///
/// {@macro flutter.widgets.PageView.allowImplicitScrolling} /// {@macro flutter.widgets.PageView.allowImplicitScrolling}
PageView.builder({ PageView.builder({
...@@ -679,6 +684,7 @@ class PageView extends StatefulWidget { ...@@ -679,6 +684,7 @@ class PageView extends StatefulWidget {
this.pageSnapping = true, this.pageSnapping = true,
this.onPageChanged, this.onPageChanged,
required IndexedWidgetBuilder itemBuilder, required IndexedWidgetBuilder itemBuilder,
ChildIndexGetter? findChildIndexCallback,
int? itemCount, int? itemCount,
this.dragStartBehavior = DragStartBehavior.start, this.dragStartBehavior = DragStartBehavior.start,
this.allowImplicitScrolling = false, this.allowImplicitScrolling = false,
...@@ -689,7 +695,11 @@ class PageView extends StatefulWidget { ...@@ -689,7 +695,11 @@ class PageView extends StatefulWidget {
}) : assert(allowImplicitScrolling != null), }) : assert(allowImplicitScrolling != null),
assert(clipBehavior != null), assert(clipBehavior != null),
controller = controller ?? _defaultPageController, controller = controller ?? _defaultPageController,
childrenDelegate = SliverChildBuilderDelegate(itemBuilder, childCount: itemCount), childrenDelegate = SliverChildBuilderDelegate(
itemBuilder,
findChildIndexCallback: findChildIndexCallback,
childCount: itemCount,
),
super(key: key); super(key: key);
/// Creates a scrollable list that works page by page with a custom child /// Creates a scrollable list that works page by page with a custom child
......
...@@ -430,6 +430,7 @@ class SliverReorderableList extends StatefulWidget { ...@@ -430,6 +430,7 @@ class SliverReorderableList extends StatefulWidget {
const SliverReorderableList({ const SliverReorderableList({
Key? key, Key? key,
required this.itemBuilder, required this.itemBuilder,
this.findChildIndexCallback,
required this.itemCount, required this.itemCount,
required this.onReorder, required this.onReorder,
this.onReorderStart, this.onReorderStart,
...@@ -447,6 +448,9 @@ class SliverReorderableList extends StatefulWidget { ...@@ -447,6 +448,9 @@ class SliverReorderableList extends StatefulWidget {
/// {@macro flutter.widgets.reorderable_list.itemBuilder} /// {@macro flutter.widgets.reorderable_list.itemBuilder}
final IndexedWidgetBuilder itemBuilder; final IndexedWidgetBuilder itemBuilder;
/// {@macro flutter.widgets.SliverChildBuilderDelegate.findChildIndexCallback}
final ChildIndexGetter? findChildIndexCallback;
/// {@macro flutter.widgets.reorderable_list.itemCount} /// {@macro flutter.widgets.reorderable_list.itemCount}
final int itemCount; final int itemCount;
...@@ -908,6 +912,7 @@ class SliverReorderableListState extends State<SliverReorderableList> with Ticke ...@@ -908,6 +912,7 @@ class SliverReorderableListState extends State<SliverReorderableList> with Ticke
// by a zero height SizedBox, so that the gap can move around. To make the // by a zero height SizedBox, so that the gap can move around. To make the
// list extent stable we add a dummy entry to the end. // list extent stable we add a dummy entry to the end.
childCount: widget.itemCount + (_dragInfo != null ? 1 : 0), childCount: widget.itemCount + (_dragInfo != null ? 1 : 0),
findChildIndexCallback: widget.findChildIndexCallback,
); );
if (widget.itemExtent != null) { if (widget.itemExtent != null) {
return SliverFixedExtentList( return SliverFixedExtentList(
......
...@@ -1126,6 +1126,8 @@ class ListView extends BoxScrollView { ...@@ -1126,6 +1126,8 @@ class ListView extends BoxScrollView {
/// efficient, however, is to create the instances on demand using this /// efficient, however, is to create the instances on demand using this
/// constructor's `itemBuilder` callback. /// constructor's `itemBuilder` callback.
/// ///
/// {@macro flutter.widgets.PageView.findChildIndexCallback}
///
/// The `addAutomaticKeepAlives` argument corresponds to the /// The `addAutomaticKeepAlives` argument corresponds to the
/// [SliverChildBuilderDelegate.addAutomaticKeepAlives] property. The /// [SliverChildBuilderDelegate.addAutomaticKeepAlives] property. The
/// `addRepaintBoundaries` argument corresponds to the /// `addRepaintBoundaries` argument corresponds to the
...@@ -1133,10 +1135,6 @@ class ListView extends BoxScrollView { ...@@ -1133,10 +1135,6 @@ class ListView extends BoxScrollView {
/// `addSemanticIndexes` argument corresponds to the /// `addSemanticIndexes` argument corresponds to the
/// [SliverChildBuilderDelegate.addSemanticIndexes] property. None may be /// [SliverChildBuilderDelegate.addSemanticIndexes] property. None may be
/// null. /// null.
///
/// [ListView.builder] by default does not support child reordering. If
/// you are planning to change child order at a later time, consider using
/// [ListView] or [ListView.custom].
ListView.builder({ ListView.builder({
Key? key, Key? key,
Axis scrollDirection = Axis.vertical, Axis scrollDirection = Axis.vertical,
...@@ -1149,6 +1147,7 @@ class ListView extends BoxScrollView { ...@@ -1149,6 +1147,7 @@ class ListView extends BoxScrollView {
this.itemExtent, this.itemExtent,
this.prototypeItem, this.prototypeItem,
required IndexedWidgetBuilder itemBuilder, required IndexedWidgetBuilder itemBuilder,
ChildIndexGetter? findChildIndexCallback,
int? itemCount, int? itemCount,
bool addAutomaticKeepAlives = true, bool addAutomaticKeepAlives = true,
bool addRepaintBoundaries = true, bool addRepaintBoundaries = true,
...@@ -1167,6 +1166,7 @@ class ListView extends BoxScrollView { ...@@ -1167,6 +1166,7 @@ class ListView extends BoxScrollView {
), ),
childrenDelegate = SliverChildBuilderDelegate( childrenDelegate = SliverChildBuilderDelegate(
itemBuilder, itemBuilder,
findChildIndexCallback: findChildIndexCallback,
childCount: itemCount, childCount: itemCount,
addAutomaticKeepAlives: addAutomaticKeepAlives, addAutomaticKeepAlives: addAutomaticKeepAlives,
addRepaintBoundaries: addRepaintBoundaries, addRepaintBoundaries: addRepaintBoundaries,
...@@ -1211,6 +1211,8 @@ class ListView extends BoxScrollView { ...@@ -1211,6 +1211,8 @@ class ListView extends BoxScrollView {
/// view's children are created in advance, or all at once when the [ListView] /// view's children are created in advance, or all at once when the [ListView]
/// itself is created, it is more efficient to use the [ListView] constructor. /// itself is created, it is more efficient to use the [ListView] constructor.
/// ///
/// {@macro flutter.widgets.PageView.findChildIndexCallback}
///
/// {@tool snippet} /// {@tool snippet}
/// ///
/// This example shows how to create [ListView] whose [ListTile] list items /// This example shows how to create [ListView] whose [ListTile] list items
...@@ -1246,6 +1248,7 @@ class ListView extends BoxScrollView { ...@@ -1246,6 +1248,7 @@ class ListView extends BoxScrollView {
bool shrinkWrap = false, bool shrinkWrap = false,
EdgeInsetsGeometry? padding, EdgeInsetsGeometry? padding,
required IndexedWidgetBuilder itemBuilder, required IndexedWidgetBuilder itemBuilder,
ChildIndexGetter? findChildIndexCallback,
required IndexedWidgetBuilder separatorBuilder, required IndexedWidgetBuilder separatorBuilder,
required int itemCount, required int itemCount,
bool addAutomaticKeepAlives = true, bool addAutomaticKeepAlives = true,
...@@ -1278,6 +1281,7 @@ class ListView extends BoxScrollView { ...@@ -1278,6 +1281,7 @@ class ListView extends BoxScrollView {
} }
return widget; return widget;
}, },
findChildIndexCallback: findChildIndexCallback,
childCount: _computeActualChildCount(itemCount), childCount: _computeActualChildCount(itemCount),
addAutomaticKeepAlives: addAutomaticKeepAlives, addAutomaticKeepAlives: addAutomaticKeepAlives,
addRepaintBoundaries: addRepaintBoundaries, addRepaintBoundaries: addRepaintBoundaries,
...@@ -1797,6 +1801,8 @@ class GridView extends BoxScrollView { ...@@ -1797,6 +1801,8 @@ class GridView extends BoxScrollView {
/// `itemBuilder` will be called only with indices greater than or equal to /// `itemBuilder` will be called only with indices greater than or equal to
/// zero and less than `itemCount`. /// zero and less than `itemCount`.
/// ///
/// {@macro flutter.widgets.PageView.findChildIndexCallback}
///
/// The [gridDelegate] argument must not be null. /// The [gridDelegate] argument must not be null.
/// ///
/// The `addAutomaticKeepAlives` argument corresponds to the /// The `addAutomaticKeepAlives` argument corresponds to the
...@@ -1815,6 +1821,7 @@ class GridView extends BoxScrollView { ...@@ -1815,6 +1821,7 @@ class GridView extends BoxScrollView {
EdgeInsetsGeometry? padding, EdgeInsetsGeometry? padding,
required this.gridDelegate, required this.gridDelegate,
required IndexedWidgetBuilder itemBuilder, required IndexedWidgetBuilder itemBuilder,
ChildIndexGetter? findChildIndexCallback,
int? itemCount, int? itemCount,
bool addAutomaticKeepAlives = true, bool addAutomaticKeepAlives = true,
bool addRepaintBoundaries = true, bool addRepaintBoundaries = true,
...@@ -1828,6 +1835,7 @@ class GridView extends BoxScrollView { ...@@ -1828,6 +1835,7 @@ class GridView extends BoxScrollView {
}) : assert(gridDelegate != null), }) : assert(gridDelegate != null),
childrenDelegate = SliverChildBuilderDelegate( childrenDelegate = SliverChildBuilderDelegate(
itemBuilder, itemBuilder,
findChildIndexCallback: findChildIndexCallback,
childCount: itemCount, childCount: itemCount,
addAutomaticKeepAlives: addAutomaticKeepAlives, addAutomaticKeepAlives: addAutomaticKeepAlives,
addRepaintBoundaries: addRepaintBoundaries, addRepaintBoundaries: addRepaintBoundaries,
......
...@@ -194,6 +194,10 @@ abstract class SliverChildDelegate { ...@@ -194,6 +194,10 @@ abstract class SliverChildDelegate {
/// This will be called during `performRebuild` in [SliverMultiBoxAdaptorElement] /// This will be called during `performRebuild` in [SliverMultiBoxAdaptorElement]
/// to check if a child has moved to a different position. It should return the /// to check if a child has moved to a different position. It should return the
/// index of the child element with associated key, null if not found. /// index of the child element with associated key, null if not found.
///
/// If not provided, a child widget may not map to its existing [RenderObject]
/// when the order of children returned from the children builder changes.
/// This may result in state-loss.
int? findIndexByKey(Key key) => null; int? findIndexByKey(Key key) => null;
@override @override
...@@ -429,14 +433,16 @@ class SliverChildBuilderDelegate extends SliverChildDelegate { ...@@ -429,14 +433,16 @@ class SliverChildBuilderDelegate extends SliverChildDelegate {
/// Defaults to providing an index for each widget. /// Defaults to providing an index for each widget.
final SemanticIndexCallback semanticIndexCallback; final SemanticIndexCallback semanticIndexCallback;
/// {@template flutter.widgets.SliverChildBuilderDelegate.findChildIndexCallback}
/// Called to find the new index of a child based on its key in case of reordering. /// Called to find the new index of a child based on its key in case of reordering.
/// ///
/// If not provided, a child widget may not map to its existing [RenderObject] /// If not provided, a child widget may not map to its existing [RenderObject]
/// when the order in which children are returned from [builder] changes. /// when the order of children returned from the children builder changes.
/// This may result in state-loss. /// This may result in state-loss.
/// ///
/// This callback should take an input [Key], and it should return the /// This callback should take an input [Key], and it should return the
/// index of the child element with that associated key, or null if not found. /// index of the child element with that associated key, or null if not found.
/// {@endtemplate}
final ChildIndexGetter? findChildIndexCallback; final ChildIndexGetter? findChildIndexCallback;
@override @override
......
...@@ -7,6 +7,46 @@ import 'package:flutter/widgets.dart'; ...@@ -7,6 +7,46 @@ import 'package:flutter/widgets.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
void main() { void main() {
// Regression test for https://github.com/flutter/flutter/issues/100451
testWidgets('SliverAnimatedList.builder respects findChildIndexCallback', (WidgetTester tester) async {
bool finderCalled = false;
int itemCount = 7;
late StateSetter stateSetter;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
stateSetter = setState;
return CustomScrollView(
slivers: <Widget>[
SliverAnimatedList(
initialItemCount: itemCount,
itemBuilder: (BuildContext context, int index, Animation<double> animation) => Container(
key: Key('$index'),
height: 2000.0,
),
findChildIndexCallback: (Key key) {
finderCalled = true;
return null;
},
),
],
);
},
),
)
);
expect(finderCalled, false);
// Trigger update.
stateSetter(() => itemCount = 77);
await tester.pump();
expect(finderCalled, true);
});
testWidgets('AnimatedList', (WidgetTester tester) async { testWidgets('AnimatedList', (WidgetTester tester) async {
Widget builder(BuildContext context, int index, Animation<double> animation) { Widget builder(BuildContext context, int index, Animation<double> animation) {
return SizedBox( return SizedBox(
......
...@@ -12,6 +12,45 @@ import '../rendering/rendering_tester.dart' show TestClipPaintingContext; ...@@ -12,6 +12,45 @@ import '../rendering/rendering_tester.dart' show TestClipPaintingContext;
import 'states.dart'; import 'states.dart';
void main() { void main() {
// Regression test for https://github.com/flutter/flutter/issues/100451
testWidgets('GridView.builder respects findChildIndexCallback', (WidgetTester tester) async {
bool finderCalled = false;
int itemCount = 7;
late StateSetter stateSetter;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
stateSetter = setState;
return GridView.builder(
itemCount: itemCount,
itemBuilder: (BuildContext _, int index) => Container(
key: Key('$index'),
height: 2000.0,
),
findChildIndexCallback: (Key key) {
finderCalled = true;
return null;
},
gridDelegate: const SliverGridDelegateWithFixedCrossAxisCount(
crossAxisCount: 4,
),
);
},
),
)
);
expect(finderCalled, false);
// Trigger update.
stateSetter(() => itemCount = 77);
await tester.pump();
expect(finderCalled, true);
});
testWidgets('Empty GridView', (WidgetTester tester) async { testWidgets('Empty GridView', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
Directionality( Directionality(
......
...@@ -76,6 +76,79 @@ class _StatefulListViewState extends State<_StatefulListView> { ...@@ -76,6 +76,79 @@ class _StatefulListViewState extends State<_StatefulListView> {
} }
void main() { void main() {
// Regression test for https://github.com/flutter/flutter/issues/100451
testWidgets('ListView.builder respects findChildIndexCallback', (WidgetTester tester) async {
bool finderCalled = false;
int itemCount = 7;
late StateSetter stateSetter;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
stateSetter = setState;
return ListView.builder(
itemCount: itemCount,
itemBuilder: (BuildContext _, int index) => Container(
key: Key('$index'),
height: 2000.0,
),
findChildIndexCallback: (Key key) {
finderCalled = true;
return null;
},
);
},
),
)
);
expect(finderCalled, false);
// Trigger update.
stateSetter(() => itemCount = 77);
await tester.pump();
expect(finderCalled, true);
});
// Regression test for https://github.com/flutter/flutter/issues/100451
testWidgets('ListView.separator respects findChildIndexCallback', (WidgetTester tester) async {
bool finderCalled = false;
int itemCount = 7;
late StateSetter stateSetter;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
stateSetter = setState;
return ListView.separated(
itemCount: itemCount,
itemBuilder: (BuildContext _, int index) => Container(
key: Key('$index'),
height: 2000.0,
),
findChildIndexCallback: (Key key) {
finderCalled = true;
return null;
},
separatorBuilder: (BuildContext _, int __) => const Divider(),
);
},
),
)
);
expect(finderCalled, false);
// Trigger update.
stateSetter(() => itemCount = 77);
await tester.pump();
expect(finderCalled, true);
});
testWidgets('ListView default control', (WidgetTester tester) async { testWidgets('ListView default control', (WidgetTester tester) async {
await tester.pumpWidget( await tester.pumpWidget(
Directionality( Directionality(
......
...@@ -12,6 +12,42 @@ import 'semantics_tester.dart'; ...@@ -12,6 +12,42 @@ import 'semantics_tester.dart';
import 'states.dart'; import 'states.dart';
void main() { void main() {
// Regression test for https://github.com/flutter/flutter/issues/100451
testWidgets('PageView.builder respects findChildIndexCallback', (WidgetTester tester) async {
bool finderCalled = false;
int itemCount = 7;
late StateSetter stateSetter;
await tester.pumpWidget(
Directionality(
textDirection: TextDirection.ltr,
child: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
stateSetter = setState;
return PageView.builder(
itemCount: itemCount,
itemBuilder: (BuildContext _, int index) => Container(
key: Key('$index'),
height: 2000.0,
),
findChildIndexCallback: (Key key) {
finderCalled = true;
return null;
},
);
},
),
)
);
expect(finderCalled, false);
// Trigger update.
stateSetter(() => itemCount = 77);
await tester.pump();
expect(finderCalled, true);
});
testWidgets('PageView resize from zero-size viewport should not lose state', (WidgetTester tester) async { testWidgets('PageView resize from zero-size viewport should not lose state', (WidgetTester tester) async {
// Regression test for https://github.com/flutter/flutter/issues/88956 // Regression test for https://github.com/flutter/flutter/issues/88956
final PageController controller = PageController( final PageController controller = PageController(
......
...@@ -7,6 +7,46 @@ import 'package:flutter/material.dart'; ...@@ -7,6 +7,46 @@ import 'package:flutter/material.dart';
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
void main() { void main() {
// Regression test for https://github.com/flutter/flutter/issues/100451
testWidgets('SliverReorderableList.builder respects findChildIndexCallback', (WidgetTester tester) async {
bool finderCalled = false;
int itemCount = 7;
late StateSetter stateSetter;
await tester.pumpWidget(
MaterialApp(
home: StatefulBuilder(
builder: (BuildContext context, StateSetter setState) {
stateSetter = setState;
return CustomScrollView(
slivers: <Widget>[
SliverReorderableList(
itemCount: itemCount,
itemBuilder: (BuildContext _, int index) => Container(
key: Key('$index'),
height: 2000.0,
),
findChildIndexCallback: (Key key) {
finderCalled = true;
return null;
},
onReorder: (int oldIndex, int newIndex) { },
),
],
);
},
),
)
);
expect(finderCalled, false);
// Trigger update.
stateSetter(() => itemCount = 77);
await tester.pump();
expect(finderCalled, true);
});
// Regression test for https://github.com/flutter/flutter/issues/88191 // Regression test for https://github.com/flutter/flutter/issues/88191
testWidgets('Do not crash when dragging with two fingers simultaneously', (WidgetTester tester) async { testWidgets('Do not crash when dragging with two fingers simultaneously', (WidgetTester tester) async {
final List<int> items = List<int>.generate(3, (int index) => index); final List<int> items = List<int>.generate(3, (int index) => index);
......
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