From 6707f5efbe59a60ffd0f2c678f6f4e5d9d87d443 Mon Sep 17 00:00:00 2001 From: xubaolin Date: Wed, 21 Feb 2024 06:14:00 +0800 Subject: [PATCH] Change `ItemExtentBuilder`'s return value nullable (#142428) Fixes https://github.com/flutter/flutter/issues/138912 Change `ItemExtentBuilder`'s return value nullable, it should return null if asked to build an item extent with a greater index than exists. --- .../flutter/lib/src/rendering/sliver.dart | 5 ++- .../rendering/sliver_fixed_extent_list.dart | 31 ++++++++++++++++--- .../rendering/sliver_multi_box_adaptor.dart | 11 +++++++ .../flutter/lib/src/widgets/scroll_view.dart | 3 ++ packages/flutter/lib/src/widgets/sliver.dart | 10 +----- .../widgets/sliver_varied_extent_list.dart | 3 ++ .../flutter/test/widgets/list_view_test.dart | 31 +++++++++++++++++++ 7 files changed, 80 insertions(+), 14 deletions(-) diff --git a/packages/flutter/lib/src/rendering/sliver.dart b/packages/flutter/lib/src/rendering/sliver.dart index a060d75af6..2ace312372 100644 --- a/packages/flutter/lib/src/rendering/sliver.dart +++ b/packages/flutter/lib/src/rendering/sliver.dart @@ -18,8 +18,11 @@ import 'viewport_offset.dart'; /// Called to get the item extent by the index of item. /// +/// Should return null if asked to build an item extent with a greater index than +/// exists. +/// /// Used by [ListView.itemExtentBuilder] and [SliverVariedExtentList.itemExtentBuilder]. -typedef ItemExtentBuilder = double Function(int index, SliverLayoutDimensions dimensions); +typedef ItemExtentBuilder = double? Function(int index, SliverLayoutDimensions dimensions); /// Relates the dimensions of the [RenderSliver] during layout. /// diff --git a/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart b/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart index 5b775f779a..b91aad6dc7 100644 --- a/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart +++ b/packages/flutter/lib/src/rendering/sliver_fixed_extent_list.dart @@ -67,8 +67,17 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda return itemExtent * index; } else { double offset = 0.0; + double? itemExtent; for (int i = 0; i < index; i++) { - offset += itemExtentBuilder!(i, _currentLayoutDimensions); + final int? childCount = childManager.estimatedChildCount; + if (childCount != null && i > childCount - 1) { + break; + } + itemExtent = itemExtentBuilder!(i, _currentLayoutDimensions); + if (itemExtent == null) { + break; + } + offset += itemExtent; } return offset; } @@ -179,8 +188,13 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda return childManager.childCount * itemExtent; } else { double offset = 0.0; + double? itemExtent; for (int i = 0; i < childManager.childCount; i++) { - offset += itemExtentBuilder!(i, _currentLayoutDimensions); + itemExtent = itemExtentBuilder!(i, _currentLayoutDimensions); + if (itemExtent == null) { + break; + } + offset += itemExtent; } return offset; } @@ -212,8 +226,17 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda } double position = 0.0; int index = 0; + double? itemExtent; while (position < scrollOffset) { - position += callback(index, _currentLayoutDimensions); + final int? childCount = childManager.estimatedChildCount; + if (childCount != null && index > childCount - 1) { + break; + } + itemExtent = callback(index, _currentLayoutDimensions); + if (itemExtent == null) { + break; + } + position += itemExtent; ++index; } return index - 1; @@ -224,7 +247,7 @@ abstract class RenderSliverFixedExtentBoxAdaptor extends RenderSliverMultiBoxAda if (itemExtentBuilder == null) { extent = itemExtent!; } else { - extent = itemExtentBuilder!(index, _currentLayoutDimensions); + extent = itemExtentBuilder!(index, _currentLayoutDimensions)!; } return constraints.asBoxConstraints( minExtent: extent, diff --git a/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart b/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart index 76188fcbca..fc99fe68f4 100644 --- a/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart +++ b/packages/flutter/lib/src/rendering/sliver_multi_box_adaptor.dart @@ -79,6 +79,17 @@ abstract class RenderSliverBoxChildManager { /// list). int get childCount; + /// The best available estimate of [childCount], or null if no estimate is available. + /// + /// This differs from [childCount] in that [childCount] never returns null (and must + /// not be accessed if the child count is not yet available, meaning the [createChild] + /// method has not been provided an index that does not create a child). + /// + /// See also: + /// + /// * [SliverChildDelegate.estimatedChildCount], to which this getter defers. + int? get estimatedChildCount => null; + /// Called during [RenderSliverMultiBoxAdaptor.adoptChild] or /// [RenderSliverMultiBoxAdaptor.move]. /// diff --git a/packages/flutter/lib/src/widgets/scroll_view.dart b/packages/flutter/lib/src/widgets/scroll_view.dart index 067c69877f..ad8dcd813d 100644 --- a/packages/flutter/lib/src/widgets/scroll_view.dart +++ b/packages/flutter/lib/src/widgets/scroll_view.dart @@ -1506,6 +1506,9 @@ class ListView extends BoxScrollView { /// This will be called multiple times during the layout phase of a frame to find /// the items that should be loaded by the lazy loading process. /// + /// Should return null if asked to build an item extent with a greater index than + /// exists. + /// /// Unlike [itemExtent] or [prototypeItem], this allows children to have /// different extents. /// diff --git a/packages/flutter/lib/src/widgets/sliver.dart b/packages/flutter/lib/src/widgets/sliver.dart index 235b81a20c..a2dac000aa 100644 --- a/packages/flutter/lib/src/widgets/sliver.dart +++ b/packages/flutter/lib/src/widgets/sliver.dart @@ -937,15 +937,7 @@ class SliverMultiBoxAdaptorElement extends RenderObjectElement implements Render ); } - /// The best available estimate of [childCount], or null if no estimate is available. - /// - /// This differs from [childCount] in that [childCount] never returns null (and must - /// not be accessed if the child count is not yet available, meaning the [createChild] - /// method has not been provided an index that does not create a child). - /// - /// See also: - /// - /// * [SliverChildDelegate.estimatedChildCount], to which this getter defers. + @override int? get estimatedChildCount => (widget as SliverMultiBoxAdaptorWidget).delegate.estimatedChildCount; @override diff --git a/packages/flutter/lib/src/widgets/sliver_varied_extent_list.dart b/packages/flutter/lib/src/widgets/sliver_varied_extent_list.dart index aa85723041..c6a4306f67 100644 --- a/packages/flutter/lib/src/widgets/sliver_varied_extent_list.dart +++ b/packages/flutter/lib/src/widgets/sliver_varied_extent_list.dart @@ -98,6 +98,9 @@ class SliverVariedExtentList extends SliverMultiBoxAdaptorWidget { )); /// The children extent builder. + /// + /// Should return null if asked to build an item extent with a greater index than + /// exists. final ItemExtentBuilder itemExtentBuilder; @override diff --git a/packages/flutter/test/widgets/list_view_test.dart b/packages/flutter/test/widgets/list_view_test.dart index 19341c7745..f8329726ad 100644 --- a/packages/flutter/test/widgets/list_view_test.dart +++ b/packages/flutter/test/widgets/list_view_test.dart @@ -780,6 +780,37 @@ void main() { expect(renderObject.clipBehavior, equals(Clip.antiAlias)); }); + // Regression test for https://github.com/flutter/flutter/pull/138912 + testWidgets('itemExtentBuilder should respect item count', (WidgetTester tester) async { + final ScrollController controller = ScrollController(); + addTearDown(controller.dispose); + final List numbers = [ + 10, 20, 30, 40, 50, + ]; + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: ListView.builder( + controller: controller, + itemCount: numbers.length, + itemExtentBuilder: (int index, SliverLayoutDimensions dimensions) { + return numbers[index]; + }, + itemBuilder: (BuildContext context, int index) { + return SizedBox( + height: numbers[index], + child: Text('Item $index'), + ); + }, + ), + ), + ); + + expect(find.text('Item 0'), findsOneWidget); + expect(find.text('Item 4'), findsOneWidget); + expect(find.text('Item 5'), findsNothing); + }); + // Regression test for https://github.com/flutter/flutter/pull/131393 testWidgets('itemExtentBuilder test', (WidgetTester tester) async { final ScrollController controller = ScrollController();