From 2331c42d15ad294ba839a499d3e2109816b3e6f8 Mon Sep 17 00:00:00 2001 From: pondermatic Date: Tue, 30 Aug 2016 16:41:08 -0500 Subject: [PATCH] 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. --- src/ng/directive/ngRepeat.js | 154 +++++++++++++++--------------- test/ng/directive/ngRepeatSpec.js | 58 ++++++++++- 2 files changed, 135 insertions(+), 77 deletions(-) diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 0b1240d5c4d4..66df7e1eb64f 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -325,7 +325,6 @@ */ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $animate, $compile) { - var NG_REMOVED = '$$NG_REMOVED'; var ngRepeatMinErr = minErr('ngRepeat'); var updateScope = function(scope, index, valueIdentifier, value, keyIdentifier, key, arrayLength) { @@ -425,126 +424,129 @@ var ngRepeatDirective = ['$parse', '$animate', '$compile', function($parse, $ani //watch props $scope.$watchCollection(rhs, function ngRepeatAction(collection) { - var index, length, - previousNode = $element[0], // node that cloned nodes should be inserted after - // initialized to the comment node anchor - nextNode, - // Same as lastBlockMap but it has the current state. It will become the - // lastBlockMap on the next iteration. - nextBlockMap = createMap(), - collectionLength, - key, value, // key/value of iteration - trackById, - trackByIdFn, - collectionKeys, - block, // last object information {scope, element, id} - nextBlockOrder, - elementsToRemove; + var + block, // last object information {scope, element, id} + collectionKey, + collectionKeys = [], + elementsToRemove, + index, key, value, // key/value of iteration + lastBlockOrder = [], + lastKey, + nextBlockMap = createMap(), + nextBlockOrder = [], + nextKey, nextLength, + previousNode = $element[0], // node that cloned nodes should be inserted after + // initialized to the comment node anchor + trackById, + trackByIdFn; if (aliasAs) { $scope[aliasAs] = collection; } + // get collectionKeys if (isArrayLike(collection)) { collectionKeys = collection; trackByIdFn = trackByIdExpFn || trackByIdArrayFn; } else { trackByIdFn = trackByIdExpFn || trackByIdObjFn; // if object, extract keys, in enumeration order, unsorted - collectionKeys = []; - for (var itemKey in collection) { - if (hasOwnProperty.call(collection, itemKey) && itemKey.charAt(0) !== '$') { - collectionKeys.push(itemKey); + for (collectionKey in collection) { + if (hasOwnProperty.call(collection, collectionKey) && collectionKey.charAt(0) !== '$') { + collectionKeys.push(collectionKey); } } } + nextLength = collectionKeys.length; - collectionLength = collectionKeys.length; - nextBlockOrder = new Array(collectionLength); - - // locate existing items - for (index = 0; index < collectionLength; index++) { + // setup nextBlockMap + for (index = 0; index < nextLength; index++) { key = (collection === collectionKeys) ? index : collectionKeys[index]; value = collection[key]; trackById = trackByIdFn(key, value, index); - if (lastBlockMap[trackById]) { - // found previously seen block - block = lastBlockMap[trackById]; - delete lastBlockMap[trackById]; - nextBlockMap[trackById] = block; - nextBlockOrder[index] = block; - } else if (nextBlockMap[trackById]) { - // if collision detected. restore lastBlockMap and throw an error - forEach(nextBlockOrder, function(block) { - if (block && block.scope) lastBlockMap[block.id] = block; - }); + + if (nextBlockMap[trackById]) { + // if collision detected, throw an error throw ngRepeatMinErr('dupes', - 'Duplicates in a repeater are not allowed. Use \'track by\' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}, Duplicate value: {2}', - expression, trackById, value); - } else { - // new never before seen block - nextBlockOrder[index] = {id: trackById, scope: undefined, clone: undefined}; - nextBlockMap[trackById] = true; + 'Duplicates in a repeater are not allowed. Use \'track by\' expression to specify unique keys. Repeater: {0}, Duplicate key: {1}, Duplicate value: {2}', + expression, trackById, value); } - } - // remove leftover items - for (var blockKey in lastBlockMap) { - block = lastBlockMap[blockKey]; - elementsToRemove = getBlockNodes(block.clone); - $animate.leave(elementsToRemove); - if (elementsToRemove[0].parentNode) { - // if the element was not removed yet because of pending animation, mark it as deleted - // so that we can ignore it later - for (index = 0, length = elementsToRemove.length; index < length; index++) { - elementsToRemove[index][NG_REMOVED] = true; - } - } - block.scope.$destroy(); + nextBlockMap[trackById] = {id: trackById, clone: undefined, scope: undefined, index: index, key: key, value: value}; + nextBlockOrder[index] = trackById; } - // we are not using forEach for perf reasons (trying to avoid #call) - for (index = 0; index < collectionLength; index++) { - key = (collection === collectionKeys) ? index : collectionKeys[index]; - value = collection[key]; - block = nextBlockOrder[index]; + // setup lastBlockOrder, used to determine if block moved + for (lastKey in lastBlockMap) { + lastBlockOrder.push(lastKey); + } - if (block.scope) { - // if we have already seen this object, then we need to reuse the - // associated scope/element + for (index = 0; index < nextLength; index++) { + nextKey = nextBlockOrder[index]; - nextNode = previousNode; + if (lastBlockMap[nextKey]) { + // we have already seen this object and need to reuse the associated scope/element + block = lastBlockMap[nextKey]; - // skip nodes that are already pending removal via leave animation - do { - nextNode = nextNode.nextSibling; - } while (nextNode && nextNode[NG_REMOVED]); + // move + if (lastBlockMap[nextKey].index !== nextBlockMap[nextKey].index) { + // If this block has moved because the last previous block was removed, + // then use the last previous block to set previousNode. + lastKey = lastBlockOrder[lastBlockMap[nextKey].index - 1]; + if (lastKey && !nextBlockMap[lastKey]) { + previousNode = getBlockEnd(lastBlockMap[lastKey]); + } - if (getBlockStart(block) !== nextNode) { - // existing item which got moved $animate.move(getBlockNodes(block.clone), null, previousNode); + block.index = nextBlockMap[nextKey].index; } + + updateScope(block.scope, index, + valueIdentifier, nextBlockMap[nextKey].value, + keyIdentifier, nextBlockMap[nextKey].key, nextLength); + + nextBlockMap[nextKey] = block; previousNode = getBlockEnd(block); - updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength); + } else { + // enter // new item which we don't know about $transclude(function ngRepeatTransclude(clone, scope) { - block.scope = scope; + nextBlockMap[nextKey].scope = scope; // http://jsperf.com/clone-vs-createcomment var endNode = ngRepeatEndComment.cloneNode(false); clone[clone.length++] = endNode; $animate.enter(clone, null, previousNode); previousNode = endNode; + // Note: We only need the first/last node of the cloned nodes. // However, we need to keep the reference to the jqlite wrapper as it might be changed later // by a directive with templateUrl when its template arrives. - block.clone = clone; - nextBlockMap[block.id] = block; - updateScope(block.scope, index, valueIdentifier, value, keyIdentifier, key, collectionLength); + nextBlockMap[nextKey].clone = clone; + updateScope(scope, nextBlockMap[nextKey].index, + valueIdentifier, nextBlockMap[nextKey].value, + keyIdentifier, nextBlockMap[nextKey].key, nextLength); + + delete nextBlockMap[nextKey].key; + delete nextBlockMap[nextKey].value; }); } } + + // leave + // This must go after enter and move because leave prevents getting element's parent. + for (lastKey in lastBlockMap) { + if (nextBlockMap[lastKey]) { + continue; + } + + block = lastBlockMap[lastKey]; + elementsToRemove = getBlockNodes(block.clone); + $animate.leave(elementsToRemove); + block.scope.$destroy(); + } + lastBlockMap = nextBlockMap; }); }; diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 71e5287a5233..8a49ba2e45d1 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -1487,7 +1487,13 @@ describe('ngRepeat animations', function() { $rootScope.items = ['1','3']; $rootScope.$digest(); - item = $animate.queue.shift(); + while ($animate.queue.length) { + item = $animate.queue.shift(); + if (item.event == 'leave') { + break; + } + } + expect(item.event).toBe('leave'); expect(item.element.text()).toBe('2'); })); @@ -1565,6 +1571,56 @@ describe('ngRepeat animations', function() { item = $animate.queue.shift(); expect(item.event).toBe('move'); expect(item.element.text()).toBe('3'); + + item = $animate.queue.shift(); + expect(item.event).toBe('move'); + expect(item.element.text()).toBe('1'); + }) + ); + + it('should fire off the move animation for filtered items', + inject(function($compile, $rootScope, $animate) { + + var item; + + element = $compile(html( + '
' + + '
' + + '{{ item }}' + + '
' + + '
' + ))($rootScope); + + $rootScope.items = ['1','2','3']; + $rootScope.search = ''; + $rootScope.$digest(); + + item = $animate.queue.shift(); + expect(item.event).toBe('enter'); + expect(item.element.text()).toBe('1'); + + item = $animate.queue.shift(); + expect(item.event).toBe('enter'); + expect(item.element.text()).toBe('2'); + + item = $animate.queue.shift(); + expect(item.event).toBe('enter'); + expect(item.element.text()).toBe('3'); + + $rootScope.search = '3'; + $rootScope.$digest(); + + item = $animate.queue.shift(); + expect(item.event).toBe('move'); + expect(item.element.text()).toBe('3'); + + item = $animate.queue.shift(); + expect(item.event).toBe('leave'); + expect(item.element.text()).toBe('1'); + + item = $animate.queue.shift(); + expect(item.event).toBe('leave'); + expect(item.element.text()).toBe('2'); }) ); });