Skip to content

Commit

Permalink
fix(ng-repeat): handle the ref changing to null and back
Browse files Browse the repository at this point in the history
This was broken in commit 09871cb.

If the ng-repeat expression changed from an iterable to a null,
ng-repeat would not notice it.  This would cause further bugs as it
would not reset its state and the ref became a collection again (i.e.
once out of sync, it was gone forever.)

Closes dart-archive#1015
  • Loading branch information
chirayuk committed May 8, 2014
1 parent 5c6a18e commit e2ac3e0
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 35 deletions.
78 changes: 43 additions & 35 deletions lib/directive/ng_repeat.dart
Original file line number Diff line number Diff line change
Expand Up @@ -132,8 +132,7 @@ class NgRepeat {
_watch = _scope.watch(
_listExpr,
(CollectionChangeRecord changes, _) {
if (changes is! CollectionChangeRecord) return;
_onChange(changes);
_onChange((changes is CollectionChangeRecord) ? changes : null);
},
collection: true,
formatters: formatters
Expand All @@ -142,7 +141,8 @@ class NgRepeat {

// Computes and executes DOM changes when the item list changes
void _onChange(CollectionChangeRecord changes) {
final int length = changes.length;
final iterable = (changes == null) ? const [] : changes.iterable;
final int length = (changes == null) ? 0 : changes.length;
final rows = new List<_Row>(length);
final changeFunctions = new List<Function>(length);
final removedIndexes = <int>[];
Expand Down Expand Up @@ -170,43 +170,51 @@ class NgRepeat {
_rows = new List<_Row>(length);
for (var i = 0; i < length; i++) {
changeFunctions[i] = (index, previousView) {
addRow(index, changes.iterable.elementAt(i), previousView);
addRow(index, iterable.elementAt(i), previousView);
};
}
} else {
changes.forEachRemoval((CollectionChangeItem removal) {
var index = removal.previousIndex;
var row = _rows[index];
row.scope.destroy();
_viewPort.remove(row.view);
leftInDom.removeAt(domLength - 1 - index);
});
if (changes == null) {
_rows.forEach((row) {
row.scope.destroy();
_viewPort.remove(row.view);
});
leftInDom.clear();
} else {
changes.forEachRemoval((CollectionChangeItem removal) {
var index = removal.previousIndex;
var row = _rows[index];
row.scope.destroy();
_viewPort.remove(row.view);
leftInDom.removeAt(domLength - 1 - index);
});

changes.forEachAddition((CollectionChangeItem addition) {
changeFunctions[addition.currentIndex] = (index, previousView) {
addRow(index, addition.item, previousView);
};
});
changes.forEachAddition((CollectionChangeItem addition) {
changeFunctions[addition.currentIndex] = (index, previousView) {
addRow(index, addition.item, previousView);
};
});

changes.forEachMove((CollectionChangeItem move) {
var previousIndex = move.previousIndex;
var value = move.item;
changeFunctions[move.currentIndex] = (index, previousView) {
var previousRow = _rows[previousIndex];
var childScope = previousRow.scope;
var childContext = _updateContext(childScope.context, index, length);
if (!identical(childScope.context[_valueIdentifier], value)) {
childContext[_valueIdentifier] = value;
}
rows[index] = _rows[previousIndex];
// Only move the DOM node when required
if (domIndex < 0 || leftInDom[domIndex] != previousIndex) {
_viewPort.move(previousRow.view, moveAfter: previousView);
leftInDom.remove(previousIndex);
}
domIndex--;
};
});
changes.forEachMove((CollectionChangeItem move) {
var previousIndex = move.previousIndex;
var value = move.item;
changeFunctions[move.currentIndex] = (index, previousView) {
var previousRow = _rows[previousIndex];
var childScope = previousRow.scope;
var childContext = _updateContext(childScope.context, index, length);
if (!identical(childScope.context[_valueIdentifier], value)) {
childContext[_valueIdentifier] = value;
}
rows[index] = _rows[previousIndex];
// Only move the DOM node when required
if (domIndex < 0 || leftInDom[domIndex] != previousIndex) {
_viewPort.move(previousRow.view, moveAfter: previousView);
leftInDom.remove(previousIndex);
}
domIndex--;
};
});
}
}

var previousView = null;
Expand Down
27 changes: 27 additions & 0 deletions test/directive/ng_repeat_spec.dart
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,33 @@ main() {
});


it('should gracefully handle ref changing to null and back', () {
scope.context['items'] = ['odin', 'dva',];
element = compile(
'<div>'
'<ul>'
'<li ng-repeat="item in items">{{item}};</li>'
'</ul>'
'</div>');
scope.apply();
expect(element.querySelectorAll('ul').length).toEqual(1);
expect(element.querySelectorAll('li').length).toEqual(2);
expect(element.text).toEqual('odin;dva;');

scope.context['items'] = null;
scope.apply();
expect(element.querySelectorAll('ul').length).toEqual(1);
expect(element.querySelectorAll('li').length).toEqual(0);
expect(element.text).toEqual('');

scope.context['items'] = ['odin', 'dva', 'tri'];
scope.apply();
expect(element.querySelectorAll('ul').length).toEqual(1);
expect(element.querySelectorAll('li').length).toEqual(3);
expect(element.text).toEqual('odin;dva;tri;');
});


it('should support formatters', () {
element = compile(
'<div><span ng-repeat="item in items | filter:myFilter">{{item}}</span></div>');
Expand Down

0 comments on commit e2ac3e0

Please sign in to comment.