Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit 9efa46a

Browse files
jankucavojtajina
jankuca
authored and
vojtajina
committed
feat(ngRepeat): use block separator comments
Issue: multi-elements ng-repeat (ng-repeat-start, ng-repeat-end) can contain elements with a trancluding directive. This directive changes content of the row (template) and ng-repeat does not work correctly (when removing/moving rows), because ng-repeat works with the original template (elements). This changes ng-repeat behavior to traverse the DOM to find current elements everytime we are moving/removing rows (if the template has multiple elements). Closes #3104
1 parent 64fd2c4 commit 9efa46a

File tree

5 files changed

+212
-31
lines changed

5 files changed

+212
-31
lines changed

Diff for: ‎src/ng/directive/ngRepeat.js

+26-6
Original file line numberDiff line numberDiff line change
@@ -279,7 +279,8 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
279279
trackByIdFn,
280280
collectionKeys,
281281
block, // last object information {scope, element, id}
282-
nextBlockOrder = [];
282+
nextBlockOrder = [],
283+
elementsToRemove;
283284

284285

285286
if (isArrayLike(collection)) {
@@ -331,8 +332,9 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
331332
// lastBlockMap is our own object so we don't need to use special hasOwnPropertyFn
332333
if (lastBlockMap.hasOwnProperty(key)) {
333334
block = lastBlockMap[key];
334-
$animate.leave(block.elements);
335-
forEach(block.elements, function(element) { element[NG_REMOVED] = true});
335+
elementsToRemove = getBlockElements(block);
336+
$animate.leave(elementsToRemove);
337+
forEach(elementsToRemove, function(element) { element[NG_REMOVED] = true; });
336338
block.scope.$destroy();
337339
}
338340
}
@@ -342,6 +344,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
342344
key = (collection === collectionKeys) ? index : collectionKeys[index];
343345
value = collection[key];
344346
block = nextBlockOrder[index];
347+
if (nextBlockOrder[index - 1]) previousNode = nextBlockOrder[index - 1].endNode;
345348

346349
if (block.startNode) {
347350
// if we have already seen this object, then we need to reuse the
@@ -357,7 +360,7 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
357360
// do nothing
358361
} else {
359362
// existing item which got moved
360-
$animate.move(block.elements, null, jqLite(previousNode));
363+
$animate.move(getBlockElements(block), null, jqLite(previousNode));
361364
}
362365
previousNode = block.endNode;
363366
} else {
@@ -375,11 +378,11 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
375378

376379
if (!block.startNode) {
377380
linker(childScope, function(clone) {
381+
clone[clone.length++] = document.createComment(' end ngRepeat: ' + expression + ' ');
378382
$animate.enter(clone, null, jqLite(previousNode));
379383
previousNode = clone;
380384
block.scope = childScope;
381-
block.startNode = clone[0];
382-
block.elements = clone;
385+
block.startNode = previousNode && previousNode.endNode ? previousNode.endNode : clone[0];
383386
block.endNode = clone[clone.length - 1];
384387
nextBlockMap[block.id] = block;
385388
});
@@ -390,5 +393,22 @@ var ngRepeatDirective = ['$parse', '$animate', function($parse, $animate) {
390393
};
391394
}
392395
};
396+
397+
function getBlockElements(block) {
398+
if (block.startNode === block.endNode) {
399+
return jqLite(block.startNode);
400+
}
401+
402+
var element = block.startNode;
403+
var elements = [element];
404+
405+
do {
406+
element = element.nextSibling;
407+
if (!element) break;
408+
elements.push(element);
409+
} while (element !== block.endNode);
410+
411+
return jqLite(elements);
412+
}
393413
}];
394414

Diff for: ‎test/BinderSpec.js

+23-4
Original file line numberDiff line numberDiff line change
@@ -96,7 +96,9 @@ describe('Binder', function() {
9696
'<ul>' +
9797
'<!-- ngRepeat: item in model.items -->' +
9898
'<li ng-bind="item.a" ng-repeat="item in model.items">A</li>' +
99+
'<!-- end ngRepeat: item in model.items -->' +
99100
'<li ng-bind="item.a" ng-repeat="item in model.items">B</li>' +
101+
'<!-- end ngRepeat: item in model.items -->' +
100102
'</ul>');
101103

102104
items.unshift({a: 'C'});
@@ -105,8 +107,11 @@ describe('Binder', function() {
105107
'<ul>' +
106108
'<!-- ngRepeat: item in model.items -->' +
107109
'<li ng-bind="item.a" ng-repeat="item in model.items">C</li>' +
110+
'<!-- end ngRepeat: item in model.items -->' +
108111
'<li ng-bind="item.a" ng-repeat="item in model.items">A</li>' +
112+
'<!-- end ngRepeat: item in model.items -->' +
109113
'<li ng-bind="item.a" ng-repeat="item in model.items">B</li>' +
114+
'<!-- end ngRepeat: item in model.items -->' +
110115
'</ul>');
111116

112117
items.shift();
@@ -115,7 +120,9 @@ describe('Binder', function() {
115120
'<ul>' +
116121
'<!-- ngRepeat: item in model.items -->' +
117122
'<li ng-bind="item.a" ng-repeat="item in model.items">A</li>' +
123+
'<!-- end ngRepeat: item in model.items -->' +
118124
'<li ng-bind="item.a" ng-repeat="item in model.items">B</li>' +
125+
'<!-- end ngRepeat: item in model.items -->' +
119126
'</ul>');
120127

121128
items.shift();
@@ -134,6 +141,7 @@ describe('Binder', function() {
134141
'<ul>' +
135142
'<!-- ngRepeat: item in model.items -->' +
136143
'<li ng-repeat="item in model.items"><span ng-bind="item.a">A</span></li>' +
144+
'<!-- end ngRepeat: item in model.items -->' +
137145
'</ul>');
138146
}));
139147

@@ -148,15 +156,15 @@ describe('Binder', function() {
148156
$rootScope.items = items;
149157

150158
$rootScope.$apply();
151-
expect(element[0].childNodes.length - 1).toEqual(0);
159+
expect(element[0].childNodes.length).toEqual(1);
152160

153161
items.name = 'misko';
154162
$rootScope.$apply();
155-
expect(element[0].childNodes.length - 1).toEqual(1);
163+
expect(element[0].childNodes.length).toEqual(3);
156164

157165
delete items.name;
158166
$rootScope.$apply();
159-
expect(element[0].childNodes.length - 1).toEqual(0);
167+
expect(element[0].childNodes.length).toEqual(1);
160168
}));
161169

162170
it('IfTextBindingThrowsErrorDecorateTheSpan', function() {
@@ -223,13 +231,19 @@ describe('Binder', function() {
223231
'<div name="a" ng-repeat="m in model">'+
224232
'<!-- ngRepeat: i in m.item -->' +
225233
'<ul name="a1" ng-repeat="i in m.item"></ul>'+
234+
'<!-- end ngRepeat: i in m.item -->' +
226235
'<ul name="a2" ng-repeat="i in m.item"></ul>'+
236+
'<!-- end ngRepeat: i in m.item -->' +
227237
'</div>'+
238+
'<!-- end ngRepeat: m in model -->' +
228239
'<div name="b" ng-repeat="m in model">'+
229240
'<!-- ngRepeat: i in m.item -->' +
230241
'<ul name="b1" ng-repeat="i in m.item"></ul>'+
242+
'<!-- end ngRepeat: i in m.item -->' +
231243
'<ul name="b2" ng-repeat="i in m.item"></ul>'+
244+
'<!-- end ngRepeat: i in m.item -->' +
232245
'</div>' +
246+
'<!-- end ngRepeat: m in model -->' +
233247
'</div>');
234248
}));
235249

@@ -306,15 +320,18 @@ describe('Binder', function() {
306320
'<div ng-repeat="i in [0,1]" ng-class-even="\'e\'" ng-class-odd="\'o\'"></div>' +
307321
'</div>')($rootScope);
308322
$rootScope.$apply();
323+
309324
var d1 = jqLite(element[0].childNodes[1]);
310-
var d2 = jqLite(element[0].childNodes[2]);
325+
var d2 = jqLite(element[0].childNodes[3]);
311326
expect(d1.hasClass('o')).toBeTruthy();
312327
expect(d2.hasClass('e')).toBeTruthy();
313328
expect(sortedHtml(element)).toBe(
314329
'<div>' +
315330
'<!-- ngRepeat: i in [0,1] -->' +
316331
'<div class="o" ng-class-even="\'e\'" ng-class-odd="\'o\'" ng-repeat="i in [0,1]"></div>' +
332+
'<!-- end ngRepeat: i in [0,1] -->' +
317333
'<div class="e" ng-class-even="\'e\'" ng-class-odd="\'o\'" ng-repeat="i in [0,1]"></div>' +
334+
'<!-- end ngRepeat: i in [0,1] -->' +
318335
'</div>');
319336
}));
320337

@@ -420,7 +437,9 @@ describe('Binder', function() {
420437
'<ul>' +
421438
'<!-- ngRepeat: (k,v) in {a:0,b:1} -->' +
422439
'<li ng-bind=\"k + v\" ng-repeat="(k,v) in {a:0,b:1}">a0</li>' +
440+
'<!-- end ngRepeat: (k,v) in {a:0,b:1} -->' +
423441
'<li ng-bind=\"k + v\" ng-repeat="(k,v) in {a:0,b:1}">b1</li>' +
442+
'<!-- end ngRepeat: (k,v) in {a:0,b:1} -->' +
424443
'</ul>');
425444
}));
426445

Diff for: ‎test/helpers/testabilityPatch.js

+11
Original file line numberDiff line numberDiff line change
@@ -226,6 +226,17 @@ function sortedHtml(element, showNgClass) {
226226
}
227227

228228

229+
function childrenTagsOf(element) {
230+
var tags = [];
231+
232+
forEach(jqLite(element).children(), function(child) {
233+
tags.push(child.nodeName.toLowerCase());
234+
});
235+
236+
return tags;
237+
}
238+
239+
229240
// TODO(vojta): migrate these helpers into jasmine matchers
230241
/**a
231242
* This method is a cheap way of testing if css for a given node is not set to 'none'. It doesn't

Diff for: ‎test/ng/directive/ngClassSpec.js

+5-5
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ describe('ngClass', function() {
166166
element = $compile('<ul><li ng-repeat="i in [0,1]" class="existing" ng-class-odd="\'odd\'" ng-class-even="\'even\'"></li><ul>')($rootScope);
167167
$rootScope.$digest();
168168
var e1 = jqLite(element[0].childNodes[1]);
169-
var e2 = jqLite(element[0].childNodes[2]);
169+
var e2 = jqLite(element[0].childNodes[3]);
170170
expect(e1.hasClass('existing')).toBeTruthy();
171171
expect(e1.hasClass('odd')).toBeTruthy();
172172
expect(e2.hasClass('existing')).toBeTruthy();
@@ -181,7 +181,7 @@ describe('ngClass', function() {
181181
'<ul>')($rootScope);
182182
$rootScope.$apply();
183183
var e1 = jqLite(element[0].childNodes[1]);
184-
var e2 = jqLite(element[0].childNodes[2]);
184+
var e2 = jqLite(element[0].childNodes[3]);
185185

186186
expect(e1.hasClass('plainClass')).toBeTruthy();
187187
expect(e1.hasClass('odd')).toBeTruthy();
@@ -199,7 +199,7 @@ describe('ngClass', function() {
199199
'<ul>')($rootScope);
200200
$rootScope.$apply();
201201
var e1 = jqLite(element[0].childNodes[1]);
202-
var e2 = jqLite(element[0].childNodes[2]);
202+
var e2 = jqLite(element[0].childNodes[3]);
203203

204204
expect(e1.hasClass('A')).toBeTruthy();
205205
expect(e1.hasClass('B')).toBeTruthy();
@@ -273,7 +273,7 @@ describe('ngClass', function() {
273273
$rootScope.$digest();
274274

275275
var e1 = jqLite(element[0].childNodes[1]);
276-
var e2 = jqLite(element[0].childNodes[2]);
276+
var e2 = jqLite(element[0].childNodes[3]);
277277

278278
expect(e1.hasClass('odd')).toBeTruthy();
279279
expect(e1.hasClass('even')).toBeFalsy();
@@ -295,7 +295,7 @@ describe('ngClass', function() {
295295
$rootScope.$digest();
296296

297297
var e1 = jqLite(element[0].childNodes[1]);
298-
var e2 = jqLite(element[0].childNodes[2]);
298+
var e2 = jqLite(element[0].childNodes[3]);
299299

300300
expect(e1.hasClass('odd')).toBeTruthy();
301301
expect(e1.hasClass('even')).toBeFalsy();

0 commit comments

Comments
 (0)