Unverified Commit 484c2268 authored by Kate Lovett's avatar Kate Lovett Committed by GitHub

Fix error message for unbounded viewports (#123035)

Fix error message for unbounded viewports
parent 76ee8ad2
......@@ -2,6 +2,7 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.
import 'box.dart';
import 'object.dart';
export 'package:flutter/foundation.dart' show debugPrint;
......@@ -311,3 +312,80 @@ bool debugAssertAllRenderVarsUnset(String reason, { bool debugCheckIntrinsicSize
}());
return true;
}
/// Returns true if the given [Axis] is bounded within the given
/// [BoxConstraints] in both the main and cross axis, throwing an exception
/// otherwise.
///
/// This is used by viewports during `performLayout` and `computeDryLayout`
/// because bounded constraints are required in order to layout their children.
bool debugCheckHasBoundedAxis(Axis axis, BoxConstraints constraints) {
assert(() {
if (!constraints.hasBoundedHeight || !constraints.hasBoundedWidth) {
switch (axis) {
case Axis.vertical:
if (!constraints.hasBoundedHeight) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Vertical viewport was given unbounded height.'),
ErrorDescription(
'Viewports expand in the scrolling direction to fill their container. '
'In this case, a vertical viewport was given an unlimited amount of '
'vertical space in which to expand. This situation typically happens '
'when a scrollable widget is nested inside another scrollable widget.',
),
ErrorHint(
'If this widget is always nested in a scrollable widget there '
'is no need to use a viewport because there will always be enough '
'vertical space for the children. In this case, consider using a '
'Column or Wrap instead. Otherwise, consider using a '
'CustomScrollView to concatenate arbitrary slivers into a '
'single scrollable.',
),
]);
}
if (!constraints.hasBoundedWidth) {
throw FlutterError(
'Vertical viewport was given unbounded width.\n'
'Viewports expand in the cross axis to fill their container and '
'constrain their children to match their extent in the cross axis. '
'In this case, a vertical viewport was given an unlimited amount of '
'horizontal space in which to expand.',
);
}
break;
case Axis.horizontal:
if (!constraints.hasBoundedWidth) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Horizontal viewport was given unbounded width.'),
ErrorDescription(
'Viewports expand in the scrolling direction to fill their container. '
'In this case, a horizontal viewport was given an unlimited amount of '
'horizontal space in which to expand. This situation typically happens '
'when a scrollable widget is nested inside another scrollable widget.',
),
ErrorHint(
'If this widget is always nested in a scrollable widget there '
'is no need to use a viewport because there will always be enough '
'horizontal space for the children. In this case, consider using a '
'Row or Wrap instead. Otherwise, consider using a '
'CustomScrollView to concatenate arbitrary slivers into a '
'single scrollable.',
),
]);
}
if (!constraints.hasBoundedHeight) {
throw FlutterError(
'Horizontal viewport was given unbounded height.\n'
'Viewports expand in the cross axis to fill their container and '
'constrain their children to match their extent in the cross axis. '
'In this case, a horizontal viewport was given an unlimited amount of '
'vertical space in which to expand.',
);
}
break;
}
}
return true;
}());
return true;
}
......@@ -9,6 +9,7 @@ import 'package:flutter/foundation.dart';
import 'package:flutter/semantics.dart';
import 'box.dart';
import 'debug.dart';
import 'layer.dart';
import 'object.dart';
import 'sliver.dart';
......@@ -1388,73 +1389,7 @@ class RenderViewport extends RenderViewportBase<SliverPhysicalContainerParentDat
@override
Size computeDryLayout(BoxConstraints constraints) {
assert(() {
if (!constraints.hasBoundedHeight || !constraints.hasBoundedWidth) {
switch (axis) {
case Axis.vertical:
if (!constraints.hasBoundedHeight) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Vertical viewport was given unbounded height.'),
ErrorDescription(
'Viewports expand in the scrolling direction to fill their container. '
'In this case, a vertical viewport was given an unlimited amount of '
'vertical space in which to expand. This situation typically happens '
'when a scrollable widget is nested inside another scrollable widget.',
),
ErrorHint(
'If this widget is always nested in a scrollable widget there '
'is no need to use a viewport because there will always be enough '
'vertical space for the children. In this case, consider using a '
'Column or Wrap instead. Otherwise, consider using a '
'CustomScrollView to concatenate arbitrary slivers into a '
'single scrollable.',
),
]);
}
if (!constraints.hasBoundedWidth) {
throw FlutterError(
'Vertical viewport was given unbounded width.\n'
'Viewports expand in the cross axis to fill their container and '
'constrain their children to match their extent in the cross axis. '
'In this case, a vertical viewport was given an unlimited amount of '
'horizontal space in which to expand.',
);
}
break;
case Axis.horizontal:
if (!constraints.hasBoundedWidth) {
throw FlutterError.fromParts(<DiagnosticsNode>[
ErrorSummary('Horizontal viewport was given unbounded width.'),
ErrorDescription(
'Viewports expand in the scrolling direction to fill their container. '
'In this case, a horizontal viewport was given an unlimited amount of '
'horizontal space in which to expand. This situation typically happens '
'when a scrollable widget is nested inside another scrollable widget.',
),
ErrorHint(
'If this widget is always nested in a scrollable widget there '
'is no need to use a viewport because there will always be enough '
'horizontal space for the children. In this case, consider using a '
'Row or Wrap instead. Otherwise, consider using a '
'CustomScrollView to concatenate arbitrary slivers into a '
'single scrollable.',
),
]);
}
if (!constraints.hasBoundedHeight) {
throw FlutterError(
'Horizontal viewport was given unbounded height.\n'
'Viewports expand in the cross axis to fill their container and '
'constrain their children to match their extent in the cross axis. '
'In this case, a horizontal viewport was given an unlimited amount of '
'vertical space in which to expand.',
);
}
break;
}
}
return true;
}());
assert(debugCheckHasBoundedAxis(axis, constraints));
return constraints.biggest;
}
......@@ -1858,17 +1793,48 @@ class RenderShrinkWrappingViewport extends RenderViewportBase<SliverLogicalConta
late double _shrinkWrapExtent;
bool _hasVisualOverflow = false;
bool _debugCheckHasBoundedCrossAxis() {
assert(() {
switch (axis) {
case Axis.vertical:
if (!constraints.hasBoundedWidth) {
throw FlutterError(
'Vertical viewport was given unbounded width.\n'
'Viewports expand in the cross axis to fill their container and '
'constrain their children to match their extent in the cross axis. '
'In this case, a vertical shrinkwrapping viewport was given an '
'unlimited amount of horizontal space in which to expand.',
);
}
break;
case Axis.horizontal:
if (!constraints.hasBoundedHeight) {
throw FlutterError(
'Horizontal viewport was given unbounded height.\n'
'Viewports expand in the cross axis to fill their container and '
'constrain their children to match their extent in the cross axis. '
'In this case, a horizontal shrinkwrapping viewport was given an '
'unlimited amount of vertical space in which to expand.',
);
}
break;
}
return true;
}());
return true;
}
@override
void performLayout() {
final BoxConstraints constraints = this.constraints;
if (firstChild == null) {
// Shrinkwrapping viewport only requires the cross axis to be bounded.
assert(_debugCheckHasBoundedCrossAxis());
switch (axis) {
case Axis.vertical:
assert(constraints.hasBoundedWidth);
size = Size(constraints.maxWidth, constraints.minHeight);
break;
case Axis.horizontal:
assert(constraints.hasBoundedHeight);
size = Size(constraints.minWidth, constraints.maxHeight);
break;
}
......@@ -1882,14 +1848,14 @@ class RenderShrinkWrappingViewport extends RenderViewportBase<SliverLogicalConta
final double mainAxisExtent;
final double crossAxisExtent;
// Shrinkwrapping viewport only requires the cross axis to be bounded.
assert(_debugCheckHasBoundedCrossAxis());
switch (axis) {
case Axis.vertical:
assert(constraints.hasBoundedWidth);
mainAxisExtent = constraints.maxHeight;
crossAxisExtent = constraints.maxWidth;
break;
case Axis.horizontal:
assert(constraints.hasBoundedHeight);
mainAxisExtent = constraints.maxWidth;
crossAxisExtent = constraints.maxHeight;
break;
......
......@@ -261,4 +261,15 @@ void main() {
expect(() => debugAssertAllRenderVarsUnset('ERROR'), throwsFlutterError);
debugDisableOpacityLayers = false;
});
test('debugCheckHasBoundedAxis warns for vertical and horizontal axis', () {
expect(
() => debugCheckHasBoundedAxis(Axis.vertical, const BoxConstraints()),
throwsFlutterError,
);
expect(
() => debugCheckHasBoundedAxis(Axis.horizontal, const BoxConstraints()),
throwsFlutterError,
);
});
}
......@@ -2188,4 +2188,76 @@ void main() {
});
expect(visited, true);
});
testWidgets('Shrinkwrapping viewport asserts bounded cross axis', (WidgetTester tester) async {
final List<FlutterErrorDetails> errors = <FlutterErrorDetails>[];
FlutterError.onError = (FlutterErrorDetails error) => errors.add(error);
// Vertical
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: ListView(
scrollDirection: Axis.horizontal,
children: <Widget>[
ListView(
shrinkWrap: true,
children: const <Widget>[ SizedBox.square(dimension: 500) ],
),
],
),
));
expect(errors, isNotEmpty);
expect(errors.first.exception, isFlutterError);
FlutterError error = errors.first.exception as FlutterError;
expect(
error.toString(),
contains('Viewports expand in the cross axis to fill their container'),
);
errors.clear();
// Horizontal
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: ListView(
children: <Widget>[
ListView(
scrollDirection: Axis.horizontal,
shrinkWrap: true,
children: const <Widget>[ SizedBox.square(dimension: 500) ],
),
],
),
));
expect(errors, isNotEmpty);
expect(errors.first.exception, isFlutterError);
error = errors.first.exception as FlutterError;
expect(
error.toString(),
contains('Viewports expand in the cross axis to fill their container'),
);
errors.clear();
// No children
await tester.pumpWidget(Directionality(
textDirection: TextDirection.ltr,
child: ListView(
scrollDirection: Axis.horizontal,
children: <Widget>[
ListView(
shrinkWrap: true,
),
],
),
));
expect(errors, isNotEmpty);
expect(errors.first.exception, isFlutterError);
error = errors.first.exception as FlutterError;
expect(
error.toString(),
contains('Viewports expand in the cross axis to fill their container'),
);
errors.clear();
});
}
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