Skip to content
This repository has been archived by the owner on Feb 22, 2018. It is now read-only.

fix(ng-repeat): handle the ref changing to null and back #1017

Merged
merged 1 commit into from
May 8, 2014
Merged

fix(ng-repeat): handle the ref changing to null and back #1017

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
78 changes: 43 additions & 35 deletions lib/directive/ng_repeat.dart
View file
Open in desktop
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) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be even simpler to move this check at the beginning of this method ?

_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
View file
Open in desktop
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