Skip to content

Commit

Permalink
Fix removal logic in widgets
Browse files Browse the repository at this point in the history
We were not removing children if they were more recently synced than we
were. This makes no sense. We should remove all children unless they
were synced this very generation already (in which case they'll be
somewhere else in the tree by now).
  • Loading branch information
Hixie committed Sep 17, 2015
1 parent 7cf7982 commit bb0a7f2
Show file tree
Hide file tree
Showing 2 changed files with 57 additions and 1 deletion.
7 changes: 6 additions & 1 deletion sky/packages/sky/lib/src/widgets/framework.dart
Original file line number Diff line number Diff line change
Expand Up @@ -377,8 +377,13 @@ abstract class Widget {

void remove() {
walkChildren((Widget child) {
if (child._generation <= _generation)
if (child.isFromOldGeneration || !isFromOldGeneration) {
assert(child.parent == this);
child.remove();
} else {
// if it's from the current generation and we're not, it means it's been moved somewhere else in the tree already and isn't really our child anymore
assert(child.parent != this);
}
});
_renderObject = null;
setParent(null);
Expand Down
51 changes: 51 additions & 0 deletions sky/unit/test/widget/duplicate_key_test.dart
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import 'package:sky/src/widgets/framework.dart';
import 'package:sky/src/widgets/basic.dart';
import 'package:test/test.dart';
import 'widget_tester.dart';

class Item {
GlobalKey key1 = new GlobalKey();
GlobalKey key2 = new GlobalKey();
String toString() => "Item($key1, $key2)";
}
List<Item> items = [new Item(), new Item()];

class StatefulLeaf extends StatefulComponent {
StatefulLeaf({ GlobalKey key }) : super(key: key);
void syncConstructorArguments(StatefulLeaf source) { }
void test() { setState(() { }); }
Widget build() => new Text('leaf');
}

class KeyedWrapper extends Component {
KeyedWrapper(this.key1, this.key2);
Key key1, key2;
Widget build() {
return new Container(
key: key1,
child: new StatefulLeaf(
key: key2
)
);
}
}

Widget builder() {
return new Column([
new KeyedWrapper(items[1].key1, items[1].key2),
new KeyedWrapper(items[0].key1, items[0].key2)
]);
}

void main() {
test('duplicate key smoke test', () {
WidgetTester tester = new WidgetTester();
tester.pumpFrame(builder);
tester.findWidget((widget) => widget is StatefulLeaf).test();
tester.pumpFrameWithoutChange();
Item lastItem = items[1];
items.remove(lastItem);
items.insert(0, lastItem);
tester.pumpFrame(builder); // this marks the app dirty and rebuilds it
});
}

0 comments on commit bb0a7f2

Please sign in to comment.