From f2b7fffdc0b0ad80cebb24f5fea743e9e4a439d5 Mon Sep 17 00:00:00 2001 From: Zhenbo Zhang Date: Wed, 9 May 2012 00:00:38 +0300 Subject: [PATCH] fix(ngRepeat): now works with primitive types closes #933 --- src/apis.js | 10 +++ src/ng/directive/ngRepeat.js | 28 +++++++- test/ApiSpecs.js | 4 ++ test/ng/directive/ngRepeatSpec.js | 115 ++++++++++++++++++++++++++++++ 4 files changed, 156 insertions(+), 1 deletion(-) diff --git a/src/apis.js b/src/apis.js index 7b4708020f3a..098d9cdd6be8 100644 --- a/src/apis.js +++ b/src/apis.js @@ -97,5 +97,15 @@ HashQueueMap.prototype = { return array.shift(); } } + }, + + /** + * return the first item without deleting it + */ + peek: function(key) { + var array = this[key = hashKey(key)]; + if (array) { + return array[0]; + } } }; diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 6dd74ef53798..a47b7abc68db 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -88,6 +88,7 @@ var ngRepeatDirective = ngDirective({ // We need an array of these objects since the same object can be returned from the iterator. // We expect this to be a rare case. var lastOrder = new HashQueueMap(); + var indexValues = []; scope.$watch(function(scope){ var index, length, collection = scope.$eval(rhs), @@ -117,7 +118,20 @@ var ngRepeatDirective = ngDirective({ for (index = 0, length = array.length; index < length; index++) { key = (collection === array) ? index : array[index]; value = collection[key]; - last = lastOrder.shift(value); + + // if collection is array and value is object, it can be shifted to allow for position change + // if collection is array and value is not object, need to first check whether index is same to + // avoid shifting wrong value + // if collection is not array, need to always check index to avoid shifting wrong value + if (lastOrder.peek(value)) { + last = collection === array ? + ((isObject(value)) ? lastOrder.shift(value) : + (index === lastOrder.peek(value).index ? lastOrder.shift(value) : undefined)) : + (index === lastOrder.peek(value).index ? lastOrder.shift(value) : undefined); + } else { + last = undefined; + } + if (last) { // if we have already seen this object, then we need to reuse the // associated scope/element @@ -137,6 +151,12 @@ var ngRepeatDirective = ngDirective({ cursor = last.element; } } else { + if (indexValues.hasOwnProperty(index) && collection !== array) { + var preValue = indexValues[index]; + var v = lastOrder.shift(preValue); + v.element.remove(); + v.scope.$destroy(); + } // new item which we don't know about childScope = scope.$new(); } @@ -158,10 +178,16 @@ var ngRepeatDirective = ngDirective({ index: index }; nextOrder.push(value, last); + indexValues[index] = value; }); } } + var i, l; + for (i = 0, l = indexValues.length - length; i < l; i++) { + indexValues.pop(); + } + //shrink children for (key in lastOrder) { if (lastOrder.hasOwnProperty(key)) { diff --git a/test/ApiSpecs.js b/test/ApiSpecs.js index 35a85bd4e2a2..1e52cf442b7a 100644 --- a/test/ApiSpecs.js +++ b/test/ApiSpecs.js @@ -31,7 +31,11 @@ describe('api', function() { map.push('key', 'a'); map.push('key', 'b'); expect(map[hashKey('key')]).toEqual(['a', 'b']); + expect(map.peek('key')).toEqual('a'); + expect(map[hashKey('key')]).toEqual(['a', 'b']); expect(map.shift('key')).toEqual('a'); + expect(map.peek('key')).toEqual('b'); + expect(map[hashKey('key')]).toEqual(['b']); expect(map.shift('key')).toEqual('b'); expect(map.shift('key')).toEqual(undefined); expect(map[hashKey('key')]).toEqual(undefined); diff --git a/test/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 72382591b970..83f23cec370d 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -37,6 +37,89 @@ describe('ngRepeat', function() { })); + it('should ngRepeat over array of primitive correctly', inject(function($rootScope, $compile) { + element = $compile( + '')($rootScope); + + Array.prototype.extraProperty = "should be ignored"; + // INIT + $rootScope.items = [true, true, true]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('true;true;true;'); + delete Array.prototype.extraProperty; + + $rootScope.items = [false, true, true]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('false;true;true;'); + + $rootScope.items = [false, true, false]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('false;true;false;'); + + $rootScope.items = [true]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(1); + expect(element.text()).toEqual('true;'); + + $rootScope.items = [true, true, false]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('true;true;false;'); + + $rootScope.items = [true, false, false]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('true;false;false;'); + + // string + $rootScope.items = ['a', 'a', 'a']; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('a;a;a;'); + + $rootScope.items = ['ab', 'a', 'a']; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('ab;a;a;'); + + $rootScope.items = ['test']; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(1); + expect(element.text()).toEqual('test;'); + + $rootScope.items = ['same', 'value']; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(2); + expect(element.text()).toEqual('same;value;'); + + // number + $rootScope.items = [12, 12, 12]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('12;12;12;'); + + $rootScope.items = [53, 12, 27]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(3); + expect(element.text()).toEqual('53;12;27;'); + + $rootScope.items = [89]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(1); + expect(element.text()).toEqual('89;'); + + $rootScope.items = [89, 23]; + $rootScope.$digest(); + expect(element.find('li').length).toEqual(2); + expect(element.text()).toEqual('89;23;'); + })); + + it('should ngRepeat over object', inject(function($rootScope, $compile) { element = $compile( '