Commit a8a32a97 authored by Hixie's avatar Hixie

Fix crash when removing a card in card_collection

MixedViewport didn't use the building:true flag when locking itself, so
when it caused a rebuild of its children, we assumed that nobody was
allowed to mark things dirty below the list, and things crashed when
Inherited people did in fact rebuild.

Also:
 - default offset for MixedViewport
 - don't bother rebuilding if the underlying RenderObject is going to
   rebuild anyway for some reason
 - better docs for the "items must have keys" assert
 - keep the FlipComponent stuff together in test_widgets.dart
parent af790303
......@@ -301,7 +301,7 @@ class CardCollectionState extends State<CardCollection> {
)
: new DefaultTextStyle(
style: DefaultTextStyle.of(context).merge(cardLabelStyle).merge(_textStyle).copyWith(
fontSize: _varyFontSizes ? 5.0 + _cardModels.length.toDouble() : null
fontSize: _varyFontSizes ? _cardModels.length.toDouble() : null
),
child: new Column(<Widget>[
new Text(cardModel.label)
......
......@@ -140,7 +140,7 @@ class _HomogeneousViewportElement extends RenderObjectElement<HomogeneousViewpor
renderObject.minExtent = getTotalExtent(null);
renderObject.startOffset = offset;
renderObject.overlayPainter = widget.overlayPainter;
});
}, building: true);
}
void _updateChildren() {
......
......@@ -17,7 +17,7 @@ enum _ChangeDescription { none, scrolled, resized }
class MixedViewport extends RenderObjectWidget {
MixedViewport({
Key key,
this.startOffset,
this.startOffset: 0.0,
this.direction: ScrollDirection.vertical,
this.builder,
this.token,
......@@ -28,9 +28,9 @@ class MixedViewport extends RenderObjectWidget {
final double startOffset;
final ScrollDirection direction;
final IndexedBuilder builder;
final Object token;
final Object token; // change this if the list changed (i.e. there are added, removed, or resorted items)
final ExtentsUpdateCallback onExtentsUpdate;
final InvalidatorAvailableCallback onInvalidatorAvailable;
final InvalidatorAvailableCallback onInvalidatorAvailable; // call the callback this gives to invalidate sizes
_MixedViewportElement createElement() => new _MixedViewportElement(this);
......@@ -171,7 +171,16 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
if (changes != _ChangeDescription.none || !isValid) {
renderObject.markNeedsLayout();
} else {
reinvokeBuilders();
// We have to reinvoke our builders because they might return new data.
// Consider a stateful component that owns us. The builder it gives us
// includes some of the state from that component. The component calls
// setState() on itself. It rebuilds. Part of that involves rebuilding
// us, but now what? If we don't reinvoke the builders. then they will
// not be rebuilt, and so the new state won't be used.
// Note that if the builders are to change so much that the _sizes_ of
// the children would change, then the parent must change the 'token'.
if (!renderObject.needsLayout)
reinvokeBuilders();
}
}
......@@ -207,7 +216,7 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
}
BuildableElement.lockState(() {
_doLayout(constraints);
});
}, building: true);
if (widget.onExtentsUpdate != null) {
final double newExtents = _didReachLastChild ? _childOffsets.last : null;
if (newExtents != _lastReportedExtents) {
......@@ -239,7 +248,10 @@ class _MixedViewportElement extends RenderObjectElement<MixedViewport> {
if (widget.builder == null)
return null;
final Widget newWidget = widget.builder(this, index);
assert(newWidget == null || newWidget.key != null); // every widget in a list must have a list-unique key
assert(() {
'Every widget in a list must have a list-unique key.';
return newWidget == null || newWidget.key != null;
});
return newWidget;
}
......
import 'package:flutter/widgets.dart';
import 'package:test/test.dart';
import 'widget_tester.dart';
List<int> items = <int>[0, 1, 2, 3, 4, 5];
Widget buildCard(BuildContext context, int index) {
if (index >= items.length)
return null;
return new Container(
key: new ValueKey<int>(items[index]),
height: 100.0,
child: new DefaultTextStyle(
style: new TextStyle(fontSize: 2.0 + items.length.toDouble()),
child: new Text('${items[index]}')
)
);
}
InvalidatorCallback invalidator;
Widget buildFrame() {
return new ScrollableMixedWidgetList(
builder: buildCard,
token: items.length,
onInvalidatorAvailable: (InvalidatorCallback callback) { invalidator = callback; }
);
}
void main() {
test('MixedViewport is a build function (smoketest)', () {
testWidgets((WidgetTester tester) {
tester.pumpWidget(buildFrame());
expect(tester.findText('0'), isNotNull);
expect(tester.findText('1'), isNotNull);
expect(tester.findText('2'), isNotNull);
expect(tester.findText('3'), isNotNull);
items.removeAt(2);
tester.pumpWidget(buildFrame());
expect(tester.findText('0'), isNotNull);
expect(tester.findText('1'), isNotNull);
expect(tester.findText('2'), isNull);
expect(tester.findText('3'), isNotNull);
});
});
}
......@@ -15,6 +15,16 @@ final BoxDecoration kBoxDecorationC = new BoxDecoration(
backgroundColor: const Color(0xFF0000FF)
);
class TestBuildCounter extends StatelessComponent {
static int buildCount = 0;
Widget build(BuildContext context) {
++buildCount;
return new DecoratedBox(decoration: kBoxDecorationA);
}
}
class FlipComponent extends StatefulComponent {
FlipComponent({ Key key, this.left, this.right }) : super(key: key);
......@@ -38,15 +48,6 @@ class FlipComponentState extends State<FlipComponent> {
}
}
class TestBuildCounter extends StatelessComponent {
static int buildCount = 0;
Widget build(BuildContext context) {
++buildCount;
return new DecoratedBox(decoration: kBoxDecorationA);
}
}
void flipStatefulComponent(WidgetTester tester) {
StatefulComponentElement stateElement =
tester.findElement((Element element) => element is StatefulComponentElement);
......
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