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

Commit 2331c42

Browse files
pondermaticpondermatic
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 #15068.
1 parent 53a3bf6 commit 2331c42

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
@@ -325,7 +325,6 @@
325325
</example>
326326
*/
327327
var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $animate, $compile) {
328-
var NG_REMOVED = '$$NG_REMOVED';
329328
var ngRepeatMinErr = minErr('ngRepeat');
330329

331330
var updateScope = function(scope, index, valueIdentifier, value, keyIdentifier, key, arrayLength) {
@@ -425,126 +424,129 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani
425424

426425
//watch props
427426
$scope.$watchCollection(rhs, function ngRepeatAction(collection) {
428-
var index, length,
429-
previousNode = $element[0], // node that cloned nodes should be inserted after
430-
// initialized to the comment node anchor
431-
nextNode,
432-
// Same as lastBlockMap but it has the current state. It will become the
433-
// lastBlockMap on the next iteration.
434-
nextBlockMap = createMap(),
435-
collectionLength,
436-
key, value, // key/value of iteration
437-
trackById,
438-
trackByIdFn,
439-
collectionKeys,
440-
block, // last object information {scope, element, id}
441-
nextBlockOrder,
442-
elementsToRemove;
427+
var
428+
block, // last object information {scope, element, id}
429+
collectionKey,
430+
collectionKeys = [],
431+
elementsToRemove,
432+
index, key, value, // key/value of iteration
433+
lastBlockOrder = [],
434+
lastKey,
435+
nextBlockMap = createMap(),
436+
nextBlockOrder = [],
437+
nextKey, nextLength,
438+
previousNode = $element[0], // node that cloned nodes should be inserted after
439+
// initialized to the comment node anchor
440+
trackById,
441+
trackByIdFn;
443442

444443
if (aliasAs) {
445444
$scope[aliasAs] = collection;
446445
}
447446

447+
// get collectionKeys
448448
if (isArrayLike(collection)) {
449449
collectionKeys = collection;
450450
trackByIdFn = trackByIdExpFn || trackByIdArrayFn;
451451
} else {
452452
trackByIdFn = trackByIdExpFn || trackByIdObjFn;
453453
// if object, extract keys, in enumeration order, unsorted
454-
collectionKeys = [];
455-
for (var itemKey in collection) {
456-
if (hasOwnProperty.call(collection, itemKey) && itemKey.charAt(0) !== '$') {
457-
collectionKeys.push(itemKey);
454+
for (collectionKey in collection) {
455+
if (hasOwnProperty.call(collection, collectionKey) && collectionKey.charAt(0) !== '$') {
456+
collectionKeys.push(collectionKey);
458457
}
459458
}
460459
}
460+
nextLength = collectionKeys.length;
461461

462-
collectionLength = collectionKeys.length;
463-
nextBlockOrder = new Array(collectionLength);
464-
465-
// locate existing items
466-
for (index = 0; index < collectionLength; index++) {
462+
// setup nextBlockMap
463+
for (index = 0; index < nextLength; index++) {
467464
key = (collection === collectionKeys) ? index : collectionKeys[index];
468465
value = collection[key];
469466
trackById = trackByIdFn(key, value, index);
470-
if (lastBlockMap[trackById]) {
471-
// found previously seen block
472-
block = lastBlockMap[trackById];
473-
delete lastBlockMap[trackById];
474-
nextBlockMap[trackById] = block;
475-
nextBlockOrder[index] = block;
476-
} else if (nextBlockMap[trackById]) {
477-
// if collision detected. restore lastBlockMap and throw an error
478-
forEach(nextBlockOrder, function(block) {
479-
if (block && block.scope) lastBlockMap[block.id] = block;
480-
});
467+
468+
if (nextBlockMap[trackById]) {
469+
// if collision detected, throw an error
481470
throw ngRepeatMinErr('dupes',
482-
'Duplicates in a repeater are not allowed. Use \'track by\' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}, Duplicate value: {2}',
483-
expression, trackById, value);
484-
} else {
485-
// new never before seen block
486-
nextBlockOrder[index] = {id: trackById, scope: undefined, clone: undefined};
487-
nextBlockMap[trackById] = true;
471+
'Duplicates in a repeater are not allowed. Use \'track by\' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}, Duplicate value: {2}',
472+
expression, trackById, value);
488473
}
489-
}
490474

491-
// remove leftover items
492-
for (var blockKey in lastBlockMap) {
493-
block = lastBlockMap[blockKey];
494-
elementsToRemove = getBlockNodes(block.clone);
495-
$animate.leave(elementsToRemove);
496-
if (elementsToRemove[0].parentNode) {
497-
// if the element was not removed yet because of pending animation, mark it as deleted
498-
// so that we can ignore it later
499-
for (index = 0, length = elementsToRemove.length; index < length; index++) {
500-
elementsToRemove[index][NG_REMOVED] = true;
501-
}
502-
}
503-
block.scope.$destroy();
475+
nextBlockMap[trackById] = {id: trackById, clone: undefined, scope: undefined, index: index, key: key, value: value};
476+
nextBlockOrder[index] = trackById;
504477
}
505478

506-
// we are not using forEach for perf reasons (trying to avoid #call)
507-
for (index = 0; index < collectionLength; index++) {
508-
key = (collection === collectionKeys) ? index : collectionKeys[index];
509-
value = collection[key];
510-
block = nextBlockOrder[index];
479+
// setup lastBlockOrder, used to determine if block moved
480+
for (lastKey in lastBlockMap) {
481+
lastBlockOrder.push(lastKey);
482+
}
511483

512-
if (block.scope) {
513-
// if we have already seen this object, then we need to reuse the
514-
// associated scope/element
484+
for (index = 0; index < nextLength; index++) {
485+
nextKey = nextBlockOrder[index];
515486

516-
nextNode = previousNode;
487+
if (lastBlockMap[nextKey]) {
488+
// we have already seen this object and need to reuse the associated scope/element
489+
block = lastBlockMap[nextKey];
517490

518-
// skip nodes that are already pending removal via leave animation
519-
do {
520-
nextNode = nextNode.nextSibling;
521-
} while (nextNode && nextNode[NG_REMOVED]);
491+
// move
492+
if (lastBlockMap[nextKey].index !== nextBlockMap[nextKey].index) {
493+
// If this block has moved because the last previous block was removed,
494+
// then use the last previous block to set previousNode.
495+
lastKey = lastBlockOrder[lastBlockMap[nextKey].index - 1];
496+
if (lastKey && !nextBlockMap[lastKey]) {
497+
previousNode = getBlockEnd(lastBlockMap[lastKey]);
498+
}
522499

523-
if (getBlockStart(block) !== nextNode) {
524-
// existing item which got moved
525500
$animate.move(getBlockNodes(block.clone), null, previousNode);
501+
block.index = nextBlockMap[nextKey].index;
526502
}
503+
504+
updateScope(block.scope, index,
505+
valueIdentifier, nextBlockMap[nextKey].value,
506+
keyIdentifier, nextBlockMap[nextKey].key, nextLength);
507+
508+
nextBlockMap[nextKey] = block;
527509
previousNode = getBlockEnd(block);
528-
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength);
510+
529511
} else {
512+
// enter
530513
// new item which we don't know about
531514
$transclude(function ngRepeatTransclude(clone, scope) {
532-
block.scope = scope;
515+
nextBlockMap[nextKey].scope = scope;
533516
// http://jsperf.com/clone-vs-createcomment
534517
var endNode = ngRepeatEndComment.cloneNode(false);
535518
clone[clone.length++] = endNode;
536519

537520
$animate.enter(clone, null, previousNode);
538521
previousNode = endNode;
522+
539523
// Note: We only need the first/last node of the cloned nodes.
540524
// However, we need to keep the reference to the jqlite wrapper as it might be changed later
541525
// by a directive with templateUrl when its template arrives.
542-
block.clone = clone;
543-
nextBlockMap[block.id] = block;
544-
updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength);
526+
nextBlockMap[nextKey].clone = clone;
527+
updateScope(scope, nextBlockMap[nextKey].index,
528+
valueIdentifier, nextBlockMap[nextKey].value,
529+
keyIdentifier, nextBlockMap[nextKey].key, nextLength);
530+
531+
delete nextBlockMap[nextKey].key;
532+
delete nextBlockMap[nextKey].value;
545533
});
546534
}
547535
}
536+
537+
// leave
538+
// This must go after enter and move because leave prevents getting element's parent.
539+
for (lastKey in lastBlockMap) {
540+
if (nextBlockMap[lastKey]) {
541+
continue;
542+
}
543+
544+
block = lastBlockMap[lastKey];
545+
elementsToRemove = getBlockNodes(block.clone);
546+
$animate.leave(elementsToRemove);
547+
block.scope.$destroy();
548+
}
549+
548550
lastBlockMap = nextBlockMap;
549551
});
550552
};

‎test/ng/directive/ngRepeatSpec.js

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

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

0 commit comments

Comments
 (0)