Skip to content

Commit e17b506

Browse files
pondermaticNarretz
pondermatic
authored and
Narretz
committed
fix(ngRepeat): trigger move animation
ngRepeat does not trigger a move animation for items that come after inserted or deleted items. The documentation says that a move animation occurs "when an adjacent item is filtered out causing a reorder or when the item contents are reordered". With this fix, items that are moved can use an animation, such as changing the border or background color, to show a change. This will fix issue angular#15068.
1 parent 26e0dfb commit e17b506

File tree

2 files changed

+135
-77
lines changed

2 files changed

+135
-77
lines changed

‎src/ng/directive/ngRepeat.js

+78-76
Original file line numberDiff line numberDiff line change
@@ -338,7 +338,6 @@
338338
</example>
339339
*/
340340
var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $animate, $compile) {
341-
var NG_REMOVED = '$$NG_REMOVED';
342341
var ngRepeatMinErr = minErr('ngRepeat');
343342

344343
var updateScope = function(scope, index, valueIdentifier, value, keyIdentifier, key, arrayLength) {
@@ -438,126 +437,129 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani
438437

439438
//watch props
440439
$scope.$watchCollection(rhs, function ngRepeatAction(collection) {
441-
var index, length,
442-
previousNode = $element[0], // node that cloned nodes should be inserted after
443-
// initialized to the comment node anchor
444-
nextNode,
445-
// Same as lastBlockMap but it has the current state. It will become the
446-
// lastBlockMap on the next iteration.
447-
nextBlockMap = createMap(),
448-
collectionLength,
449-
key, value, // key/value of iteration
450-
trackById,
451-
trackByIdFn,
452-
collectionKeys,
453-
block, // last object information {scope, element, id}
454-
nextBlockOrder,
455-
elementsToRemove;
440+
var
441+
block, // last object information {scope, element, id}
442+
collectionKey,
443+
collectionKeys = [],
444+
elementsToRemove,
445+
index, key, value, // key/value of iteration
446+
lastBlockOrder = [],
447+
lastKey,
448+
nextBlockMap = createMap(),
449+
nextBlockOrder = [],
450+
nextKey, nextLength,
451+
previousNode = $element[0], // node that cloned nodes should be inserted after
452+
// initialized to the comment node anchor
453+
trackById,
454+
trackByIdFn;
456455

457456
if (aliasAs) {
458457
$scope[aliasAs] = collection;
459458
}
460459

460+
// get collectionKeys
461461
if (isArrayLike(collection)) {
462462
collectionKeys = collection;
463463
trackByIdFn = trackByIdExpFn || trackByIdArrayFn;
464464
} else {
465465
trackByIdFn = trackByIdExpFn || trackByIdObjFn;
466466
// if object, extract keys, in enumeration order, unsorted
467-
collectionKeys = [];
468-
for (var itemKey in collection) {
469-
if (hasOwnProperty.call(collection, itemKey) && itemKey.charAt(0) !== '$') {
470-
collectionKeys.push(itemKey);
467+
for (collectionKey in collection) {
468+
if (hasOwnProperty.call(collection, collectionKey) && collectionKey.charAt(0) !== '$') {
469+
collectionKeys.push(collectionKey);
471470
}
472471
}
473472
}
473+
nextLength = collectionKeys.length;
474474

475-
collectionLength = collectionKeys.length;
476-
nextBlockOrder = new Array(collectionLength);
477-
478-
// locate existing items
479-
for (index = 0; index < collectionLength; index++) {
475+
// setup nextBlockMap
476+
for (index = 0; index < nextLength; index++) {
480477
key = (collection === collectionKeys) ? index : collectionKeys[index];
481478
value = collection[key];
482479
trackById = trackByIdFn(key, value, index);
483-
if (lastBlockMap[trackById]) {
484-
// found previously seen block
485-
block = lastBlockMap[trackById];
486-
delete lastBlockMap[trackById];
487-
nextBlockMap[trackById] = block;
488-
nextBlockOrder[index] = block;
489-
} else if (nextBlockMap[trackById]) {
490-
// if collision detected. restore lastBlockMap and throw an error
491-
forEach(nextBlockOrder, function(block) {
492-
if (block && block.scope) lastBlockMap[block.id] = block;
493-
});
480+
481+
if (nextBlockMap[trackById]) {
482+
// if collision detected, throw an error
494483
throw ngRepeatMinErr('dupes',
495-
'Duplicates in a repeater are not allowed. Use \'track by\' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}, Duplicate value: {2}',
496-
expression, trackById, value);
497-
} else {
498-
// new never before seen block
499-
nextBlockOrder[index] = {id: trackById, scope: undefined, clone: undefined};
500-
nextBlockMap[trackById] = true;
484+
'Duplicates in a repeater are not allowed. Use \'track by\' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}, Duplicate value: {2}',
485+
expression, trackById, value);
501486
}
502-
}
503487

504-
// remove leftover items
505-
for (var blockKey in lastBlockMap) {
506-
block = lastBlockMap[blockKey];
507-
elementsToRemove = getBlockNodes(block.clone);
508-
$animate.leave(elementsToRemove);
509-
if (elementsToRemove[0].parentNode) {
510-
// if the element was not removed yet because of pending animation, mark it as deleted
511-
// so that we can ignore it later
512-
for (index = 0, length = elementsToRemove.length; index < length; index++) {
513-
elementsToRemove[index][NG_REMOVED] = true;
514-
}
515-
}
516-
block.scope.$destroy();
488+
nextBlockMap[trackById] = {id: trackById, clone: undefined, scope: undefined, index: index, key: key, value: value};
489+
nextBlockOrder[index] = trackById;
517490
}
518491

519-
// we are not using forEach for perf reasons (trying to avoid #call)
520-
for (index = 0; index < collectionLength; index++) {
521-
key = (collection === collectionKeys) ? index : collectionKeys[index];
522-
value = collection[key];
523-
block = nextBlockOrder[index];
492+
// setup lastBlockOrder, used to determine if block moved
493+
for (lastKey in lastBlockMap) {
494+
lastBlockOrder.push(lastKey);
495+
}
524496

525-
if (block.scope) {
526-
// if we have already seen this object, then we need to reuse the
527-
// associated scope/element
497+
for (index = 0; index < nextLength; index++) {
498+
nextKey = nextBlockOrder[index];
528499

529-
nextNode = previousNode;
500+
if (lastBlockMap[nextKey]) {
501+
// we have already seen this object and need to reuse the associated scope/element
502+
block = lastBlockMap[nextKey];
530503

531-
// skip nodes that are already pending removal via leave animation
532-
do {
533-
nextNode = nextNode.nextSibling;
534-
} while (nextNode && nextNode[NG_REMOVED]);
504+
// move
505+
if (lastBlockMap[nextKey].index !== nextBlockMap[nextKey].index) {
506+
// If this block has moved because the last previous block was removed,
507+
// then use the last previous block to set previousNode.
508+
lastKey = lastBlockOrder[lastBlockMap[nextKey].index - 1];
509+
if (lastKey && !nextBlockMap[lastKey]) {
510+
previousNode = getBlockEnd(lastBlockMap[lastKey]);
511+
}
535512

536-
if (getBlockStart(block) !== nextNode) {
537-
// existing item which got moved
538513
$animate.move(getBlockNodes(block.clone), null, previousNode);
514+
block.index = nextBlockMap[nextKey].index;
539515
}
516+
517+
updateScope(block.scope, index,
518+
valueIdentifier, nextBlockMap[nextKey].value,
519+
keyIdentifier, nextBlockMap[nextKey].key, nextLength);
520+
521+
nextBlockMap[nextKey] = block;
540522
previousNode = getBlockEnd(block);
541-
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength);
523+
542524
} else {
525+
// enter
543526
// new item which we don't know about
544527
$transclude(function ngRepeatTransclude(clone, scope) {
545-
block.scope = scope;
528+
nextBlockMap[nextKey].scope = scope;
546529
// http://jsperf.com/clone-vs-createcomment
547530
var endNode = ngRepeatEndComment.cloneNode(false);
548531
clone[clone.length++] = endNode;
549532

550533
$animate.enter(clone, null, previousNode);
551534
previousNode = endNode;
535+
552536
// Note: We only need the first/last node of the cloned nodes.
553537
// However, we need to keep the reference to the jqlite wrapper as it might be changed later
554538
// by a directive with templateUrl when its template arrives.
555-
block.clone = clone;
556-
nextBlockMap[block.id] = block;
557-
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength);
539+
nextBlockMap[nextKey].clone = clone;
540+
updateScope(scope, nextBlockMap[nextKey].index,
541+
valueIdentifier, nextBlockMap[nextKey].value,
542+
keyIdentifier, nextBlockMap[nextKey].key, nextLength);
543+
544+
delete nextBlockMap[nextKey].key;
545+
delete nextBlockMap[nextKey].value;
558546
});
559547
}
560548
}
549+
550+
// leave
551+
// This must go after enter and move because leave prevents getting element's parent.
552+
for (lastKey in lastBlockMap) {
553+
if (nextBlockMap[lastKey]) {
554+
continue;
555+
}
556+
557+
block = lastBlockMap[lastKey];
558+
elementsToRemove = getBlockNodes(block.clone);
559+
$animate.leave(elementsToRemove);
560+
block.scope.$destroy();
561+
}
562+
561563
lastBlockMap = nextBlockMap;
562564
});
563565
};

‎test/ng/directive/ngRepeatSpec.js

+57-1
Original file line numberDiff line numberDiff line change
@@ -1488,7 +1488,13 @@ describe('ngRepeat animations', function() {
14881488
$rootScope.items = ['1','3'];
14891489
$rootScope.$digest();
14901490

1491-
item = $animate.queue.shift();
1491+
while ($animate.queue.length) {
1492+
item = $animate.queue.shift();
1493+
if (item.event == 'leave') {
1494+
break;
1495+
}
1496+
}
1497+
14921498
expect(item.event).toBe('leave');
14931499
expect(item.element.text()).toBe('2');
14941500
}));
@@ -1566,6 +1572,56 @@ describe('ngRepeat animations', function() {
15661572
item = $animate.queue.shift();
15671573
expect(item.event).toBe('move');
15681574
expect(item.element.text()).toBe('3');
1575+
1576+
item = $animate.queue.shift();
1577+
expect(item.event).toBe('move');
1578+
expect(item.element.text()).toBe('1');
1579+
})
1580+
);
1581+
1582+
it('should fire off the move animation for filtered items',
1583+
inject(function($compile, $rootScope, $animate) {
1584+
1585+
var item;
1586+
1587+
element = $compile(html(
1588+
'<div>' +
1589+
'<div ng-repeat="item in items | filter:search">' +
1590+
'{{ item }}' +
1591+
'</div>' +
1592+
'</div>'
1593+
))($rootScope);
1594+
1595+
$rootScope.items = ['1','2','3'];
1596+
$rootScope.search = '';
1597+
$rootScope.$digest();
1598+
1599+
item = $animate.queue.shift();
1600+
expect(item.event).toBe('enter');
1601+
expect(item.element.text()).toBe('1');
1602+
1603+
item = $animate.queue.shift();
1604+
expect(item.event).toBe('enter');
1605+
expect(item.element.text()).toBe('2');
1606+
1607+
item = $animate.queue.shift();
1608+
expect(item.event).toBe('enter');
1609+
expect(item.element.text()).toBe('3');
1610+
1611+
$rootScope.search = '3';
1612+
$rootScope.$digest();
1613+
1614+
item = $animate.queue.shift();
1615+
expect(item.event).toBe('move');
1616+
expect(item.element.text()).toBe('3');
1617+
1618+
item = $animate.queue.shift();
1619+
expect(item.event).toBe('leave');
1620+
expect(item.element.text()).toBe('1');
1621+
1622+
item = $animate.queue.shift();
1623+
expect(item.event).toBe('leave');
1624+
expect(item.element.text()).toBe('2');
15691625
})
15701626
);
15711627
});

0 commit comments

Comments
 (0)