Skip to content

Commit 34b68ef

Browse files
committed
fix(select): assign result of track exp to element value
Fixes a regression where the option/select values would always be set to the key or index of a value within the corresponding collection. Prior to some 1.3.0 refactoring, the result of the track expression would be bound to the value, but this behavior was not documented or explicitly tested. This commit adds one explicit test for this behavior, and changes several other trackBy tests to reflect the desired behavior as well. Fixes angular#9592
1 parent 24d00cc commit 34b68ef

File tree

2 files changed

+73
-40
lines changed

2 files changed

+73
-40
lines changed

src/ng/directive/select.js

+17-5
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,8 @@ var ngOptionsMinErr = minErr('ngOptions');
3838
* <div class="alert alert-info">
3939
* **Note:** Using `select as` will bind the result of the `select as` expression to the model, but
4040
* the value of the `<select>` and `<option>` html elements will be either the index (for array data sources)
41-
* or property name (for object data sources) of the value within the collection.
41+
* or property name (for object data sources) of the value within the collection. If a `track by` expression
42+
* is used, the result of that expression will be set as the value of the `option` and `select` elements.
4243
* </div>
4344
*
4445
* **Note:** Using `select as` together with `trackexpr` is not recommended.
@@ -407,24 +408,35 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
407408
if (multiple) {
408409
viewValue = [];
409410
forEach(selectElement.val(), function(selectedKey) {
410-
viewValue.push(getViewValue(selectedKey, collection[selectedKey]));
411+
viewValue.push(getViewValue(selectedKey, collection));
411412
});
412413
} else {
413414
var selectedKey = selectElement.val();
414-
viewValue = getViewValue(selectedKey, collection[selectedKey]);
415+
viewValue = getViewValue(selectedKey, collection);
415416
}
416417
ctrl.$setViewValue(viewValue);
417418
render();
418419
});
419420
}
420421

421-
function getViewValue(key, value) {
422+
function getViewValue(key, collection) {
422423
if (key === '?') {
423424
return undefined;
424425
} else if (key === '') {
425426
return null;
426427
} else {
427428
var viewValueFn = selectAsFn ? selectAsFn : valueFn;
429+
var result;
430+
var value = collection[key];
431+
if (trackFn) {
432+
for (var i in collection) {
433+
result = callExpression(trackFn, key, collection[i]);
434+
if (result.toString() === key) {
435+
value = collection[i];
436+
break;
437+
}
438+
}
439+
}
428440
return callExpression(viewValueFn, key, value);
429441
}
430442
}
@@ -558,7 +570,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
558570
label = isDefined(label) ? label : '';
559571
optionGroup.push({
560572
// either the index into array or key from object
561-
id: (keyName ? keys[index] : index),
573+
id: trackFn? trackFn(scope, locals) : (keyName ? keys[index] : index),
562574
label: label,
563575
selected: selected // determine if we should be selected
564576
});

test/ng/directive/selectSpec.js

+56-35
Original file line numberDiff line numberDiff line change
@@ -722,7 +722,29 @@ describe('select', function() {
722722
describe('trackBy expression', function() {
723723
beforeEach(function() {
724724
scope.arr = [{id: 10, label: 'ten'}, {id:20, label: 'twenty'}];
725-
scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
725+
scope.obj = {'1': {score: 10, label: 'ten'}, '2': {score: 20, label: 'twenty'}};
726+
});
727+
728+
729+
it('should set the result of track by expression to element value', function() {
730+
createSelect({
731+
'ng-model': 'selected',
732+
'ng-options': 'item.label for item in arr track by item.id'
733+
});
734+
735+
scope.$apply(function() {
736+
scope.selected = scope.arr[0];
737+
});
738+
expect(element.val()).toBe('10');
739+
740+
scope.$apply(function() {
741+
scope.arr[0] = {id: 10, label: 'new ten'};
742+
});
743+
expect(element.val()).toBe('10');
744+
745+
element.children()[1].selected = 'selected';
746+
browserTrigger(element, 'change');
747+
expect(scope.selected).toEqual(scope.arr[1]);
726748
});
727749

728750

@@ -735,14 +757,14 @@ describe('select', function() {
735757
scope.$apply(function() {
736758
scope.selected = scope.arr[0];
737759
});
738-
expect(element.val()).toBe('0');
760+
expect(element.val()).toBe('10');
739761

740762
scope.$apply(function() {
741763
scope.arr[0] = {id: 10, label: 'new ten'};
742764
});
743-
expect(element.val()).toBe('0');
765+
expect(element.val()).toBe('10');
744766

745-
element.children()[1].selected = 1;
767+
element.children()[1].selected = 'selected';
746768
browserTrigger(element, 'change');
747769
expect(scope.selected).toEqual(scope.arr[1]);
748770
});
@@ -758,12 +780,12 @@ describe('select', function() {
758780
scope.$apply(function() {
759781
scope.selected = scope.arr;
760782
});
761-
expect(element.val()).toEqual(['0','1']);
783+
expect(element.val()).toEqual(['10','20']);
762784

763785
scope.$apply(function() {
764786
scope.arr[0] = {id: 10, label: 'new ten'};
765787
});
766-
expect(element.val()).toEqual(['0','1']);
788+
expect(element.val()).toEqual(['10','20']);
767789

768790
element.children()[0].selected = false;
769791
browserTrigger(element, 'change');
@@ -778,18 +800,18 @@ describe('select', function() {
778800
});
779801

780802
scope.$apply(function() {
781-
scope.selected = scope.obj['10'];
803+
scope.selected = scope.obj['1'];
782804
});
783805
expect(element.val()).toBe('10');
784806

785807
scope.$apply(function() {
786-
scope.obj['10'] = {score: 10, label: 'ten'};
808+
scope.obj['1'] = {score: 10, label: 'ten'};
787809
});
788810
expect(element.val()).toBe('10');
789811

790812
element.val('20');
791813
browserTrigger(element, 'change');
792-
expect(scope.selected).toBe(scope.obj[20]);
814+
expect(scope.selected).toBe(scope.obj['2']);
793815
});
794816

795817

@@ -801,18 +823,18 @@ describe('select', function() {
801823
});
802824

803825
scope.$apply(function() {
804-
scope.selected = [scope.obj['10']];
826+
scope.selected = [scope.obj['1']];
805827
});
806828
expect(element.val()).toEqual(['10']);
807829

808830
scope.$apply(function() {
809-
scope.obj['10'] = {score: 10, label: 'ten'};
831+
scope.obj['1'] = {score: 10, label: 'ten'};
810832
});
811833
expect(element.val()).toEqual(['10']);
812834

813835
element.children()[1].selected = 'selected';
814836
browserTrigger(element, 'change');
815-
expect(scope.selected).toEqual([scope.obj[10], scope.obj[20]]);
837+
expect(scope.selected).toEqual([scope.obj['1'], scope.obj['2']]);
816838
});
817839
});
818840

@@ -824,7 +846,7 @@ describe('select', function() {
824846
describe('selectAs+trackBy expression', function() {
825847
beforeEach(function() {
826848
scope.arr = [{subItem: {label: 'ten', id: 10}}, {subItem: {label: 'twenty', id: 20}}];
827-
scope.obj = {'10': {subItem: {id: 10, label: 'ten'}}, '20': {subItem: {id: 20, label: 'twenty'}}};
849+
scope.obj = {'1': {subItem: {id: 10, label: 'ten'}}, '2': {subItem: {id: 20, label: 'twenty'}}};
828850
});
829851

830852

@@ -840,16 +862,16 @@ describe('select', function() {
840862
scope.$apply(function() {
841863
scope.selected = scope.arr[0].subItem;
842864
});
843-
expect(element.val()).toEqual('0');
865+
expect(element.val()).toEqual('10');
844866

845867
scope.$apply(function() {
846868
scope.selected = scope.arr[1].subItem;
847869
});
848-
expect(element.val()).toEqual('1');
870+
expect(element.val()).toEqual('20');
849871

850872
// Now test view -> model
851873

852-
element.val('0');
874+
element.val('10');
853875
browserTrigger(element, 'change');
854876
expect(scope.selected).toBe(scope.arr[0].subItem);
855877

@@ -861,7 +883,7 @@ describe('select', function() {
861883
subItem: {label: 'new twenty', id: 20}
862884
}];
863885
});
864-
expect(element.val()).toBe('0');
886+
expect(element.val()).toBe('10');
865887
expect(scope.selected.id).toBe(10);
866888
});
867889

@@ -879,12 +901,12 @@ describe('select', function() {
879901
scope.$apply(function() {
880902
scope.selected = [scope.arr[0].subItem];
881903
});
882-
expect(element.val()).toEqual(['0']);
904+
expect(element.val()).toEqual(['10']);
883905

884906
scope.$apply(function() {
885907
scope.selected = [scope.arr[1].subItem];
886908
});
887-
expect(element.val()).toEqual(['1']);
909+
expect(element.val()).toEqual(['20']);
888910

889911
// Now test view -> model
890912

@@ -901,7 +923,7 @@ describe('select', function() {
901923
subItem: {label: 'new twenty', id: 20}
902924
}];
903925
});
904-
expect(element.val()).toEqual(['0']);
926+
expect(element.val()).toEqual(['10']);
905927
expect(scope.selected[0].id).toEqual(10);
906928
expect(scope.selected.length).toBe(1);
907929
});
@@ -918,13 +940,12 @@ describe('select', function() {
918940
// First test model -> view
919941

920942
scope.$apply(function() {
921-
scope.selected = [scope.obj['10'].subItem];
943+
scope.selected = [scope.obj['1'].subItem];
922944
});
923945
expect(element.val()).toEqual(['10']);
924946

925-
926947
scope.$apply(function() {
927-
scope.selected = [scope.obj['10'].subItem];
948+
scope.selected = [scope.obj['1'].subItem];
928949
});
929950
expect(element.val()).toEqual(['10']);
930951

@@ -933,15 +954,15 @@ describe('select', function() {
933954
element.find('option')[0].selected = true;
934955
element.find('option')[1].selected = false;
935956
browserTrigger(element, 'change');
936-
expect(scope.selected).toEqual([scope.obj['10'].subItem]);
957+
expect(scope.selected).toEqual([scope.obj['1'].subItem]);
937958

938959
// Now reload the object
939960
scope.$apply(function() {
940961
scope.obj = {
941-
'10': {
962+
'1': {
942963
subItem: {label: 'new ten', id: 10}
943964
},
944-
'20': {
965+
'2': {
945966
subItem: {label: 'new twenty', id: 20}
946967
}
947968
};
@@ -962,29 +983,29 @@ describe('select', function() {
962983
// First test model -> view
963984

964985
scope.$apply(function() {
965-
scope.selected = scope.obj['10'].subItem;
986+
scope.selected = scope.obj['1'].subItem;
966987
});
967988
expect(element.val()).toEqual('10');
968989

969990

970991
scope.$apply(function() {
971-
scope.selected = scope.obj['10'].subItem;
992+
scope.selected = scope.obj['1'].subItem;
972993
});
973994
expect(element.val()).toEqual('10');
974995

975996
// Now test view -> model
976997

977998
element.find('option')[0].selected = true;
978999
browserTrigger(element, 'change');
979-
expect(scope.selected).toEqual(scope.obj['10'].subItem);
1000+
expect(scope.selected).toEqual(scope.obj['1'].subItem);
9801001

9811002
// Now reload the object
9821003
scope.$apply(function() {
9831004
scope.obj = {
984-
'10': {
1005+
'1': {
9851006
subItem: {label: 'new ten', id: 10}
9861007
},
987-
'20': {
1008+
'2': {
9881009
subItem: {label: 'new twenty', id: 20}
9891010
}
9901011
};
@@ -1338,20 +1359,20 @@ describe('select', function() {
13381359
scope.selected = scope.values[1];
13391360
});
13401361

1341-
expect(element.val()).toEqual('1');
1362+
expect(element.val()).toEqual('2');
13421363

13431364
var first = jqLite(element.find('option')[0]);
13441365
expect(first.text()).toEqual('first');
1345-
expect(first.attr('value')).toEqual('0');
1366+
expect(first.attr('value')).toEqual('1');
13461367
var forth = jqLite(element.find('option')[3]);
13471368
expect(forth.text()).toEqual('forth');
1348-
expect(forth.attr('value')).toEqual('3');
1369+
expect(forth.attr('value')).toEqual('4');
13491370

13501371
scope.$apply(function() {
13511372
scope.selected = scope.values[3];
13521373
});
13531374

1354-
expect(element.val()).toEqual('3');
1375+
expect(element.val()).toEqual('4');
13551376
});
13561377

13571378

0 commit comments

Comments
 (0)