From 5e5c724590623cbecdd03396f77705ccf3700b57 Mon Sep 17 00:00:00 2001 From: Igor Minar Date: Thu, 22 Nov 2012 23:18:31 +0100 Subject: [PATCH] refactor(ngRepeat): add tests and simplify code This commits adds test for #933 and simplifies the implementation of the fix Closes #933 --- src/apis.js | 4 +- src/ng/directive/ngRepeat.js | 34 +++++----------- test/ng/directive/ngRepeatSpec.js | 64 ++++++++++++++++++------------- 3 files changed, 49 insertions(+), 53 deletions(-) diff --git a/src/apis.js b/src/apis.js index 098d9cdd6be8..0e94e2a55ce2 100644 --- a/src/apis.js +++ b/src/apis.js @@ -98,12 +98,12 @@ HashQueueMap.prototype = { } } }, - + /** * return the first item without deleting it */ peek: function(key) { - var array = this[key = hashKey(key)]; + var array = this[hashKey(key)]; if (array) { return array[0]; } diff --git a/src/ng/directive/ngRepeat.js b/src/ng/directive/ngRepeat.js index 021845fae0ad..8c934b767210 100644 --- a/src/ng/directive/ngRepeat.js +++ b/src/ng/directive/ngRepeat.js @@ -88,7 +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 ngRepeatWatch(scope){ var index, length, collection = scope.$eval(rhs), @@ -119,18 +119,14 @@ var ngRepeatDirective = ngDirective({ key = (collection === array) ? index : array[index]; value = collection[key]; - // 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 value is object, it can be shifted to allow for position change + // if is not object, need to first check whether index is same to avoid shifting wrong val + last = isObject(value) + ? lastOrder.shift(value) + : (last = lastOrder.peek(value)) && (index === last.index) + ? lastOrder.shift(value) + : undefined; + if (last) { // if we have already seen this object, then we need to reuse the @@ -151,12 +147,6 @@ 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(); } @@ -178,16 +168,10 @@ 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/ng/directive/ngRepeatSpec.js b/test/ng/directive/ngRepeatSpec.js index 83f23cec370d..3a2e2bb35969 100644 --- a/test/ng/directive/ngRepeatSpec.js +++ b/test/ng/directive/ngRepeatSpec.js @@ -37,10 +37,10 @@ describe('ngRepeat', function() { })); - it('should ngRepeat over array of primitive correctly', inject(function($rootScope, $compile) { + it('should ngRepeat over array of primitives', inject(function($rootScope, $compile) { element = $compile( '')($rootScope); Array.prototype.extraProperty = "should be ignored"; @@ -91,13 +91,13 @@ describe('ngRepeat', function() { $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 + + // number $rootScope.items = [12, 12, 12]; $rootScope.$digest(); expect(element.find('li').length).toEqual(3); @@ -130,36 +130,48 @@ describe('ngRepeat', function() { expect(element.text()).toEqual('misko:swe;shyam:set;'); })); - - it('should ngRepeat over object with primitive value correctly', inject(function($rootScope, $compile) { + + it('should ngRepeat over object with changing primitive value', + inject(function($rootScope, $compile) { + element = $compile( '')($rootScope); - $rootScope.items = {misko:'true', shyam:'true', zhenbo: 'true'}; + //document.body.appendChild(element[0]); + $rootScope.items = {misko: true, shyam: true, zhenbo:true}; $rootScope.$digest(); expect(element.find('li').length).toEqual(3); expect(element.text()).toEqual('misko:true;shyam:true;zhenbo:true;'); - - $rootScope.items = {misko:'false', shyam:'true', zhenbo: 'true'}; - $rootScope.$digest(); - expect(element.find('li').length).toEqual(3); + + browserTrigger(element.find('input').eq(0), 'click'); + expect(element.text()).toEqual('misko:false;shyam:true;zhenbo:true;'); - - $rootScope.items = {misko:'false', shyam:'false', zhenbo: 'false'}; - $rootScope.$digest(); - expect(element.find('li').length).toEqual(3); - expect(element.text()).toEqual('misko:false;shyam:false;zhenbo:false;'); - - $rootScope.items = {misko:'true'}; - $rootScope.$digest(); - expect(element.find('li').length).toEqual(1); - expect(element.text()).toEqual('misko:true;'); + expect(element.find('input')[0].checked).toBe(false); + expect(element.find('input')[1].checked).toBe(true); + expect(element.find('input')[2].checked).toBe(true); - $rootScope.items = {shyam:'true', zhenbo: 'false'}; + browserTrigger(element.find('input').eq(0), 'click'); + expect(element.text()).toEqual('misko:true;shyam:true;zhenbo:true;'); + expect(element.find('input')[0].checked).toBe(true); + expect(element.find('input')[1].checked).toBe(true); + expect(element.find('input')[2].checked).toBe(true); + + browserTrigger(element.find('input').eq(1), 'click'); + expect(element.text()).toEqual('misko:true;shyam:false;zhenbo:true;'); + expect(element.find('input')[0].checked).toBe(true); + expect(element.find('input')[1].checked).toBe(false); + expect(element.find('input')[2].checked).toBe(true); + + $rootScope.items = {misko: false, shyam: true, zhenbo: true}; $rootScope.$digest(); - expect(element.find('li').length).toEqual(2); - expect(element.text()).toEqual('shyam:true;zhenbo:false;'); + expect(element.text()).toEqual('misko:false;shyam:true;zhenbo:true;'); + expect(element.find('input')[0].checked).toBe(false); + expect(element.find('input')[1].checked).toBe(true); + expect(element.find('input')[2].checked).toBe(true); }));