Skip to content

Commit 5e5c724

Browse files
IgorMinarIgorMinar
committed
refactor(ngRepeat): add tests and simplify code
This commits adds test for angular#933 and simplifies the implementation of the fix Closes angular#933
1 parent af7e0bd commit 5e5c724

File tree

3 files changed

+49
-53
lines changed

3 files changed

+49
-53
lines changed

‎src/apis.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -98,12 +98,12 @@ HashQueueMap.prototype = {
9898
}
9999
}
100100
},
101-
101+
102102
/**
103103
* return the first item without deleting it
104104
*/
105105
peek: function(key) {
106-
var array = this[key = hashKey(key)];
106+
var array = this[hashKey(key)];
107107
if (array) {
108108
return array[0];
109109
}

‎src/ng/directive/ngRepeat.js

+9-25
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ var ngRepeatDirective = ngDirective({
8888
// We need an array of these objects since the same object can be returned from the iterator.
8989
// We expect this to be a rare case.
9090
var lastOrder = new HashQueueMap();
91-
var indexValues = [];
91+
9292
scope.$watch(function ngRepeatWatch(scope){
9393
var index, length,
9494
collection = scope.$eval(rhs),
@@ -119,18 +119,14 @@ var ngRepeatDirective = ngDirective({
119119
key = (collection === array) ? index : array[index];
120120
value = collection[key];
121121

122-
// if collection is array and value is object, it can be shifted to allow for position change
123-
// if collection is array and value is not object, need to first check whether index is same to
124-
// avoid shifting wrong value
125-
// if collection is not array, need to always check index to avoid shifting wrong value
126-
if (lastOrder.peek(value)) {
127-
last = collection === array ?
128-
((isObject(value)) ? lastOrder.shift(value) :
129-
(index === lastOrder.peek(value).index ? lastOrder.shift(value) : undefined)) :
130-
(index === lastOrder.peek(value).index ? lastOrder.shift(value) : undefined);
131-
} else {
132-
last = undefined;
133-
}
122+
// if value is object, it can be shifted to allow for position change
123+
// if is not object, need to first check whether index is same to avoid shifting wrong val
124+
last = isObject(value)
125+
? lastOrder.shift(value)
126+
: (last = lastOrder.peek(value)) && (index === last.index)
127+
? lastOrder.shift(value)
128+
: undefined;
129+
134130

135131
if (last) {
136132
// if we have already seen this object, then we need to reuse the
@@ -151,12 +147,6 @@ var ngRepeatDirective = ngDirective({
151147
cursor = last.element;
152148
}
153149
} else {
154-
if (indexValues.hasOwnProperty(index) && collection !== array) {
155-
var preValue = indexValues[index];
156-
var v = lastOrder.shift(preValue);
157-
v.element.remove();
158-
v.scope.$destroy();
159-
}
160150
// new item which we don't know about
161151
childScope = scope.$new();
162152
}
@@ -178,16 +168,10 @@ var ngRepeatDirective = ngDirective({
178168
index: index
179169
};
180170
nextOrder.push(value, last);
181-
indexValues[index] = value;
182171
});
183172
}
184173
}
185174

186-
var i, l;
187-
for (i = 0, l = indexValues.length - length; i < l; i++) {
188-
indexValues.pop();
189-
}
190-
191175
//shrink children
192176
for (key in lastOrder) {
193177
if (lastOrder.hasOwnProperty(key)) {

‎test/ng/directive/ngRepeatSpec.js

+38-26
Original file line numberDiff line numberDiff line change
@@ -37,10 +37,10 @@ describe('ngRepeat', function() {
3737
}));
3838

3939

40-
it('should ngRepeat over array of primitive correctly', inject(function($rootScope, $compile) {
40+
it('should ngRepeat over array of primitives', inject(function($rootScope, $compile) {
4141
element = $compile(
4242
'<ul>' +
43-
'<li ng-repeat="item in items" ng-init="suffix = \';\'" ng-bind="item + suffix"></li>' +
43+
'<li ng-repeat="item in items">{{item}};</li>' +
4444
'</ul>')($rootScope);
4545

4646
Array.prototype.extraProperty = "should be ignored";
@@ -91,13 +91,13 @@ describe('ngRepeat', function() {
9191
$rootScope.$digest();
9292
expect(element.find('li').length).toEqual(1);
9393
expect(element.text()).toEqual('test;');
94-
94+
9595
$rootScope.items = ['same', 'value'];
9696
$rootScope.$digest();
9797
expect(element.find('li').length).toEqual(2);
9898
expect(element.text()).toEqual('same;value;');
99-
100-
// number
99+
100+
// number
101101
$rootScope.items = [12, 12, 12];
102102
$rootScope.$digest();
103103
expect(element.find('li').length).toEqual(3);
@@ -130,36 +130,48 @@ describe('ngRepeat', function() {
130130
expect(element.text()).toEqual('misko:swe;shyam:set;');
131131
}));
132132

133-
134-
it('should ngRepeat over object with primitive value correctly', inject(function($rootScope, $compile) {
133+
134+
it('should ngRepeat over object with changing primitive value',
135+
inject(function($rootScope, $compile) {
136+
135137
element = $compile(
136138
'<ul>' +
137-
'<li ng-repeat="(key, value) in items" ng-bind="key + \':\' + value + \';\' "></li>' +
139+
'<li ng-repeat="(key, value) in items">' +
140+
'{{key}}:{{value}};' +
141+
'<input type="checkbox" ng-model="items[key]">' +
142+
'</li>' +
138143
'</ul>')($rootScope);
139-
$rootScope.items = {misko:'true', shyam:'true', zhenbo: 'true'};
144+
//document.body.appendChild(element[0]);
145+
$rootScope.items = {misko: true, shyam: true, zhenbo:true};
140146
$rootScope.$digest();
141147
expect(element.find('li').length).toEqual(3);
142148
expect(element.text()).toEqual('misko:true;shyam:true;zhenbo:true;');
143-
144-
$rootScope.items = {misko:'false', shyam:'true', zhenbo: 'true'};
145-
$rootScope.$digest();
146-
expect(element.find('li').length).toEqual(3);
149+
150+
browserTrigger(element.find('input').eq(0), 'click');
151+
147152
expect(element.text()).toEqual('misko:false;shyam:true;zhenbo:true;');
148-
149-
$rootScope.items = {misko:'false', shyam:'false', zhenbo: 'false'};
150-
$rootScope.$digest();
151-
expect(element.find('li').length).toEqual(3);
152-
expect(element.text()).toEqual('misko:false;shyam:false;zhenbo:false;');
153-
154-
$rootScope.items = {misko:'true'};
155-
$rootScope.$digest();
156-
expect(element.find('li').length).toEqual(1);
157-
expect(element.text()).toEqual('misko:true;');
153+
expect(element.find('input')[0].checked).toBe(false);
154+
expect(element.find('input')[1].checked).toBe(true);
155+
expect(element.find('input')[2].checked).toBe(true);
158156

159-
$rootScope.items = {shyam:'true', zhenbo: 'false'};
157+
browserTrigger(element.find('input').eq(0), 'click');
158+
expect(element.text()).toEqual('misko:true;shyam:true;zhenbo:true;');
159+
expect(element.find('input')[0].checked).toBe(true);
160+
expect(element.find('input')[1].checked).toBe(true);
161+
expect(element.find('input')[2].checked).toBe(true);
162+
163+
browserTrigger(element.find('input').eq(1), 'click');
164+
expect(element.text()).toEqual('misko:true;shyam:false;zhenbo:true;');
165+
expect(element.find('input')[0].checked).toBe(true);
166+
expect(element.find('input')[1].checked).toBe(false);
167+
expect(element.find('input')[2].checked).toBe(true);
168+
169+
$rootScope.items = {misko: false, shyam: true, zhenbo: true};
160170
$rootScope.$digest();
161-
expect(element.find('li').length).toEqual(2);
162-
expect(element.text()).toEqual('shyam:true;zhenbo:false;');
171+
expect(element.text()).toEqual('misko:false;shyam:true;zhenbo:true;');
172+
expect(element.find('input')[0].checked).toBe(false);
173+
expect(element.find('input')[1].checked).toBe(true);
174+
expect(element.find('input')[2].checked).toBe(true);
163175
}));
164176

165177

0 commit comments

Comments
 (0)