mirror of
https://github.com/flutter/flutter.git
synced 2025-06-03 00:51:18 +00:00
Fix CarouselView.weighted crashes when initlal viewportDimension is 0.0 (#167628)
## Description This PR fixes `CarouselView.weigthed` crashes due to given constraints being zero (which happen, for instance, when viewportDimension is 0.0). At startup, a warm-up frame can be produced before the Flutter engine has reported the initial view metrics. As a result, the first frame can be produced with a size of zero. In the context of CarouselView this leads to some problems mainly related to division by zero. Similar to https://github.com/flutter/flutter/pull/167271 which addressed the same problem for `CarouselView`. ## Related Issue Fixes https://github.com/flutter/flutter/issues/167621 ## Tests Adds 3 tests.
This commit is contained in:
parent
a7bb66b675
commit
0e595cc86d
@ -844,6 +844,11 @@ class _RenderSliverWeightedCarousel extends RenderSliverFixedExtentBoxAdaptor {
|
|||||||
// The given `index` is compared with `_firstVisibleItemIndex` to know how
|
// The given `index` is compared with `_firstVisibleItemIndex` to know how
|
||||||
// many items are placed before the current one in the view.
|
// many items are placed before the current one in the view.
|
||||||
double _buildItemExtent(int index, SliverLayoutDimensions currentLayoutDimensions) {
|
double _buildItemExtent(int index, SliverLayoutDimensions currentLayoutDimensions) {
|
||||||
|
// If constraints.viewportMainAxisExtent is 0, firstChildExtent will be 0 and cause division error.
|
||||||
|
if (constraints.viewportMainAxisExtent == 0) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
|
|
||||||
double extent;
|
double extent;
|
||||||
if (index == _firstVisibleItemIndex) {
|
if (index == _firstVisibleItemIndex) {
|
||||||
extent = math.max(_distanceToLeadingEdge, effectiveShrinkExtent);
|
extent = math.max(_distanceToLeadingEdge, effectiveShrinkExtent);
|
||||||
@ -905,6 +910,10 @@ class _RenderSliverWeightedCarousel extends RenderSliverFixedExtentBoxAdaptor {
|
|||||||
// (with weight 7), we leave some space before item 0 assuming there is another
|
// (with weight 7), we leave some space before item 0 assuming there is another
|
||||||
// item -1 as the first visible item.
|
// item -1 as the first visible item.
|
||||||
int get _firstVisibleItemIndex {
|
int get _firstVisibleItemIndex {
|
||||||
|
// If constraints.viewportMainAxisExtent is 0, firstChildExtent will be 0 and cause division error.
|
||||||
|
if (constraints.viewportMainAxisExtent == 0.0) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
int smallerWeightCount = 0;
|
int smallerWeightCount = 0;
|
||||||
for (final int weight in weights) {
|
for (final int weight in weights) {
|
||||||
if (weight == weights.max) {
|
if (weight == weights.max) {
|
||||||
@ -928,6 +937,10 @@ class _RenderSliverWeightedCarousel extends RenderSliverFixedExtentBoxAdaptor {
|
|||||||
// item. It informs them how much the first item has moved off-screen,
|
// item. It informs them how much the first item has moved off-screen,
|
||||||
// enabling them to adjust their sizes (grow or shrink) accordingly.
|
// enabling them to adjust their sizes (grow or shrink) accordingly.
|
||||||
double get _firstVisibleItemOffscreenExtent {
|
double get _firstVisibleItemOffscreenExtent {
|
||||||
|
// If constraints.viewportMainAxisExtent is 0, firstChildExtent will be 0 and cause division error.
|
||||||
|
if (constraints.viewportMainAxisExtent == 0.0) {
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
int index;
|
int index;
|
||||||
final double actual = constraints.scrollOffset / firstChildExtent;
|
final double actual = constraints.scrollOffset / firstChildExtent;
|
||||||
final int round = (constraints.scrollOffset / firstChildExtent).round();
|
final int round = (constraints.scrollOffset / firstChildExtent).round();
|
||||||
|
@ -1589,6 +1589,108 @@ void main() {
|
|||||||
expect(logoSize.width, itemExtent - itemHorizontalPadding);
|
expect(logoSize.width, itemExtent - itemHorizontalPadding);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
// Regression test for https://github.com/flutter/flutter/issues/167621.
|
||||||
|
testWidgets('CarouselView.weighted does not crash when parent size is zero', (
|
||||||
|
WidgetTester tester,
|
||||||
|
) async {
|
||||||
|
await tester.pumpWidget(
|
||||||
|
const MaterialApp(
|
||||||
|
home: Scaffold(
|
||||||
|
body: SizedBox(
|
||||||
|
width: 0,
|
||||||
|
child: CarouselView.weighted(
|
||||||
|
flexWeights: <int>[1, 2],
|
||||||
|
children: <Widget>[FlutterLogo(), FlutterLogo()],
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(tester.takeException(), isNull);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Regression test for https://github.com/flutter/flutter/issues/167621.
|
||||||
|
testWidgets('CarouselView.weighted does not crash when initial viewport dimension is zero', (
|
||||||
|
WidgetTester tester,
|
||||||
|
) async {
|
||||||
|
await tester.binding.setSurfaceSize(Size.zero);
|
||||||
|
addTearDown(() => tester.binding.setSurfaceSize(null));
|
||||||
|
|
||||||
|
await tester.pumpWidget(
|
||||||
|
const MaterialApp(
|
||||||
|
home: Scaffold(
|
||||||
|
body: CarouselView.weighted(
|
||||||
|
flexWeights: <int>[1, 2],
|
||||||
|
children: <Widget>[FlutterLogo(), FlutterLogo()],
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
|
||||||
|
expect(tester.takeException(), isNull);
|
||||||
|
});
|
||||||
|
|
||||||
|
// Regression test for https://github.com/flutter/flutter/issues/167621.
|
||||||
|
testWidgets('CarouselView.weigted weigths are applied when viewport dimension is updated', (
|
||||||
|
WidgetTester tester,
|
||||||
|
) async {
|
||||||
|
addTearDown(() => tester.binding.setSurfaceSize(null));
|
||||||
|
final CarouselController controller = CarouselController(initialItem: 1);
|
||||||
|
addTearDown(controller.dispose);
|
||||||
|
|
||||||
|
const int firstWeight = 2;
|
||||||
|
const int secondWeight = 3;
|
||||||
|
bool showScrollbars = false;
|
||||||
|
|
||||||
|
Future<void> updateSurfaceSizeAndPump(Size size) async {
|
||||||
|
await tester.binding.setSurfaceSize(size);
|
||||||
|
|
||||||
|
// At startup, a warm-up frame can be produced before the Flutter engine has reported the
|
||||||
|
// initial view metrics. As a result, the first frame can be produced with a size of zero.
|
||||||
|
// This leads to several instances of _CarouselPosition being created and
|
||||||
|
// _CarouselPosition.absorb to be called.
|
||||||
|
// To correctly simulate this behavior in the test environment, one solution is to
|
||||||
|
// update the ScrollConfiguration. For instance by changing the ScrollBehavior.scrollbars
|
||||||
|
// value on each build.
|
||||||
|
showScrollbars = !showScrollbars;
|
||||||
|
|
||||||
|
await tester.pumpWidget(
|
||||||
|
MaterialApp(
|
||||||
|
home: Scaffold(
|
||||||
|
body: Center(
|
||||||
|
child: ScrollConfiguration(
|
||||||
|
behavior: const ScrollBehavior().copyWith(scrollbars: showScrollbars),
|
||||||
|
child: CarouselView.weighted(
|
||||||
|
controller: controller,
|
||||||
|
flexWeights: const <int>[firstWeight, secondWeight],
|
||||||
|
children: List<Widget>.generate(20, (int index) {
|
||||||
|
return Center(child: Text('Item $index'));
|
||||||
|
}),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
),
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// Simulate an initial zero viewport dimension.
|
||||||
|
await updateSurfaceSizeAndPump(Size.zero);
|
||||||
|
const double surfaceWidth = 500;
|
||||||
|
await updateSurfaceSizeAndPump(const Size(surfaceWidth, 400));
|
||||||
|
|
||||||
|
const int totalWeight = firstWeight + secondWeight;
|
||||||
|
|
||||||
|
expect(find.text('Item 0'), findsOne);
|
||||||
|
expect(find.text('Item 1'), findsOne);
|
||||||
|
|
||||||
|
final double firstItemWidth = tester.getRect(getItem(0)).width;
|
||||||
|
expect(firstItemWidth, surfaceWidth * firstWeight / totalWeight);
|
||||||
|
final double secondItemWidth = tester.getRect(getItem(1)).width;
|
||||||
|
expect(secondItemWidth, surfaceWidth * secondWeight / totalWeight);
|
||||||
|
});
|
||||||
|
|
||||||
group('CarouselController.animateToItem', () {
|
group('CarouselController.animateToItem', () {
|
||||||
testWidgets('CarouselView.weighted horizontal, not reversed, flexWeights [7,1]', (
|
testWidgets('CarouselView.weighted horizontal, not reversed, flexWeights [7,1]', (
|
||||||
WidgetTester tester,
|
WidgetTester tester,
|
||||||
|
Loading…
Reference in New Issue
Block a user