Commit cdc0f15f authored by Adam Barth's avatar Adam Barth Committed by GitHub

SingleChildScrollView can assert in LayoutBuilder (#8162)

We can't read our size in the offset setter because we might be in the middle
of layout. This issue also occured with the sliver-based viewport.
parent 70d7fe3a
...@@ -157,7 +157,7 @@ class Scrollable2State extends State<Scrollable2> with TickerProviderStateMixin ...@@ -157,7 +157,7 @@ class Scrollable2State extends State<Scrollable2> with TickerProviderStateMixin
bool _shouldUpdatePosition(Scrollable2 oldConfig) { bool _shouldUpdatePosition(Scrollable2 oldConfig) {
return config.physics?.runtimeType != oldConfig.physics?.runtimeType return config.physics?.runtimeType != oldConfig.physics?.runtimeType
|| config.controller?.runtimeType != config.controller?.runtimeType; || config.controller?.runtimeType != oldConfig.controller?.runtimeType;
} }
@override @override
......
...@@ -154,24 +154,11 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix ...@@ -154,24 +154,11 @@ class _RenderSingleChildViewport extends RenderBox with RenderObjectWithChildMix
if (value == _offset) if (value == _offset)
return; return;
if (attached) if (attached)
_offset.removeListener(markNeedsLayout); _offset.removeListener(markNeedsPaint);
if (_offset.pixels != value.pixels)
markNeedsLayout();
_offset = value; _offset = value;
if (attached) if (attached)
_offset.addListener(markNeedsLayout); _offset.addListener(markNeedsPaint);
// If we already have a size, then we should re-report the dimensions markNeedsLayout();
// to the new ViewportOffset. If we don't then we'll report them when
// we establish the dimensions later, so don't worry about it now.
if (hasSize) {
assert(_minScrollExtent != null);
assert(_maxScrollExtent != null);
assert(_effectiveExtent != null);
final bool didAcceptViewportDimensions = offset.applyViewportDimension(_effectiveExtent);
final bool didAcceptContentDimensions = offset.applyContentDimensions(_minScrollExtent, _maxScrollExtent);
if (didAcceptViewportDimensions || didAcceptContentDimensions)
markNeedsPaint();
}
} }
@override @override
......
...@@ -5,6 +5,32 @@ ...@@ -5,6 +5,32 @@
import 'package:flutter_test/flutter_test.dart'; import 'package:flutter_test/flutter_test.dart';
import 'package:flutter/widgets.dart'; import 'package:flutter/widgets.dart';
class TestScrollPosition extends ScrollPosition {
TestScrollPosition({
ScrollPhysics physics,
AbstractScrollState state,
double initialPixels: 0.0,
ScrollPosition oldPosition,
}) : super(
physics: physics,
state: state,
initialPixels: initialPixels,
oldPosition: oldPosition,
);
}
class TestScrollController extends ScrollController {
@override
ScrollPosition createScrollPosition(ScrollPhysics physics, AbstractScrollState state, ScrollPosition oldPosition) {
return new TestScrollPosition(
physics: physics,
state: state,
initialPixels: initialScrollOffset,
oldPosition: oldPosition,
);
}
}
void main() { void main() {
testWidgets('SingleChildScrollView control test', (WidgetTester tester) async { testWidgets('SingleChildScrollView control test', (WidgetTester tester) async {
await tester.pumpWidget(new SingleChildScrollView( await tester.pumpWidget(new SingleChildScrollView(
...@@ -23,4 +49,71 @@ void main() { ...@@ -23,4 +49,71 @@ void main() {
expect(box.localToGlobal(Point.origin), equals(const Point(0.0, -200.0))); expect(box.localToGlobal(Point.origin), equals(const Point(0.0, -200.0)));
}); });
testWidgets('Changing controllers changes scroll position', (WidgetTester tester) async {
TestScrollController controller = new TestScrollController();
await tester.pumpWidget(new SingleChildScrollView(
child: new Container(
height: 2000.0,
decoration: const BoxDecoration(
backgroundColor: const Color(0xFF00FF00),
),
),
));
await tester.pumpWidget(new SingleChildScrollView(
controller: controller,
child: new Container(
height: 2000.0,
decoration: const BoxDecoration(
backgroundColor: const Color(0xFF00FF00),
),
),
));
Scrollable2State scrollable = tester.state(find.byType(Scrollable2));
expect(scrollable.position, const isInstanceOf<TestScrollPosition>());
});
testWidgets('Changing scroll controller inside dirty layout builder does not assert', (WidgetTester tester) async {
ScrollController controller = new ScrollController();
await tester.pumpWidget(new Center(
child: new SizedBox(
width: 750.0,
child: new LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return new SingleChildScrollView(
child: new Container(
height: 2000.0,
decoration: const BoxDecoration(
backgroundColor: const Color(0xFF00FF00),
),
),
);
},
),
),
));
await tester.pumpWidget(new Center(
child: new SizedBox(
width: 700.0,
child: new LayoutBuilder(
builder: (BuildContext context, BoxConstraints constraints) {
return new SingleChildScrollView(
controller: controller,
child: new Container(
height: 2000.0,
decoration: const BoxDecoration(
backgroundColor: const Color(0xFF00FF00),
),
),
);
},
),
),
));
});
} }
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