Skip to content
This repository was archived by the owner on Apr 12, 2024. It is now read-only.

Commit addfff3

Browse files
jeffbcrossjeffbcross
committed
fix(select): add basic track by and select as support
Instead of throwing an error when using "track by" and "select as" expressions, ngOptions will assume that the track by expression is valid, and will use it to compare values. Closes #6564
1 parent 6e4955a commit addfff3

File tree

3 files changed

+176
-21
lines changed

3 files changed

+176
-21
lines changed

‎docs/content/error/ngOptions/trkslct.ngdoc

+3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,9 @@
33
@fullName Comprehension expression cannot contain both `select as` and `track by` expressions.
44
@description
55

6+
NOTE: This error was introduced in 1.3.0-rc.5, and was removed for 1.3.0-rc.6 in order to
7+
not break existing apps.
8+
69
This error occurs when 'ngOptions' is passed a comprehension expression that contains both a
710
`select as` expression and a `track by` expression. These two expressions are fundamentally
811
incompatible.

‎src/ng/directive/select.js

+6-12
Original file line numberDiff line numberDiff line change
@@ -363,13 +363,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
363363
//re-usable object to represent option's locals
364364
locals = {};
365365

366-
if (trackFn && selectAsFn) {
367-
throw ngOptionsMinErr('trkslct',
368-
"Comprehension expression cannot contain both selectAs '{0}' " +
369-
"and trackBy '{1}' expressions.",
370-
selectAs, track);
371-
}
372-
373366
if (nullOption) {
374367
// compile the element since there might be bindings in it
375368
$compile(nullOption)(scope);
@@ -460,7 +453,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
460453
function createIsSelectedFn(viewValue) {
461454
var selectedSet;
462455
if (multiple) {
463-
if (!selectAs && trackFn && isArray(viewValue)) {
456+
if (trackFn && isArray(viewValue)) {
464457

465458
selectedSet = new HashMap([]);
466459
for (var trackIndex = 0; trackIndex < viewValue.length; trackIndex++) {
@@ -470,15 +463,16 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
470463
} else {
471464
selectedSet = new HashMap(viewValue);
472465
}
473-
} else if (!selectAsFn && trackFn) {
466+
} else if (trackFn) {
474467
viewValue = callExpression(trackFn, null, viewValue);
475468
}
469+
476470
return function isSelected(key, value) {
477471
var compareValueFn;
478-
if (selectAsFn) {
479-
compareValueFn = selectAsFn;
480-
} else if (trackFn) {
472+
if (trackFn) {
481473
compareValueFn = trackFn;
474+
} else if (selectAsFn) {
475+
compareValueFn = selectAsFn;
482476
} else {
483477
compareValueFn = valueFn;
484478
}

‎test/ng/directive/selectSpec.js

+167-9
Original file line numberDiff line numberDiff line change
@@ -817,22 +817,180 @@ describe('select', function() {
817817
});
818818

819819

820+
/**
821+
* This behavior is broken and should probably be cleaned up later as track by and select as
822+
* aren't compatible.
823+
*/
820824
describe('selectAs+trackBy expression', function() {
821825
beforeEach(function() {
822-
scope.arr = [{id: 10, label: 'ten'}, {id:'20', label: 'twenty'}];
823-
scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}};
826+
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'}}};
824828
});
825829

826830

827-
it('should throw a helpful minerr', function() {
828-
expect(function() {
831+
it('It should use the "value" variable to represent items in the array as well as for the ' +
832+
'selected values in track by expression (single&array)', function() {
833+
createSelect({
834+
'ng-model': 'selected',
835+
'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)'
836+
});
837+
838+
// First test model -> view
839+
840+
scope.$apply(function() {
841+
scope.selected = scope.arr[0].subItem;
842+
});
843+
expect(element.val()).toEqual('0');
844+
845+
scope.$apply(function() {
846+
scope.selected = scope.arr[1].subItem;
847+
});
848+
expect(element.val()).toEqual('1');
849+
850+
// Now test view -> model
851+
852+
element.val('0');
853+
browserTrigger(element, 'change');
854+
expect(scope.selected).toBe(scope.arr[0].subItem);
855+
856+
// Now reload the array
857+
scope.$apply(function() {
858+
scope.arr = [{
859+
subItem: {label: 'new ten', id: 10}
860+
},{
861+
subItem: {label: 'new twenty', id: 20}
862+
}];
863+
});
864+
expect(element.val()).toBe('0');
865+
expect(scope.selected.id).toBe(10);
866+
});
867+
868+
869+
it('It should use the "value" variable to represent items in the array as well as for the ' +
870+
'selected values in track by expression (multiple&array)', function() {
871+
createSelect({
872+
'ng-model': 'selected',
873+
'multiple': true,
874+
'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)'
875+
});
876+
877+
// First test model -> view
878+
879+
scope.$apply(function() {
880+
scope.selected = [scope.arr[0].subItem];
881+
});
882+
expect(element.val()).toEqual(['0']);
883+
884+
scope.$apply(function() {
885+
scope.selected = [scope.arr[1].subItem];
886+
});
887+
expect(element.val()).toEqual(['1']);
888+
889+
// Now test view -> model
890+
891+
element.find('option')[0].selected = true;
892+
element.find('option')[1].selected = false;
893+
browserTrigger(element, 'change');
894+
expect(scope.selected).toEqual([scope.arr[0].subItem]);
895+
896+
// Now reload the array
897+
scope.$apply(function() {
898+
scope.arr = [{
899+
subItem: {label: 'new ten', id: 10}
900+
},{
901+
subItem: {label: 'new twenty', id: 20}
902+
}];
903+
});
904+
expect(element.val()).toEqual(['0']);
905+
expect(scope.selected[0].id).toEqual(10);
906+
expect(scope.selected.length).toBe(1);
907+
});
829908

830-
createSelect({
831-
'ng-model': 'selected',
832-
'ng-options': 'item.id as item.name for item in values track by item.id'
833-
});
834909

835-
}).toThrowMinErr('ngOptions', 'trkslct', "Comprehension expression cannot contain both selectAs 'item.id' and trackBy 'item.id' expressions.");
910+
it('It should use the "value" variable to represent items in the array as well as for the ' +
911+
'selected values in track by expression (multiple&object)', function() {
912+
createSelect({
913+
'ng-model': 'selected',
914+
'multiple': true,
915+
'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)'
916+
});
917+
918+
// First test model -> view
919+
920+
scope.$apply(function() {
921+
scope.selected = [scope.obj['10'].subItem];
922+
});
923+
expect(element.val()).toEqual(['10']);
924+
925+
926+
scope.$apply(function() {
927+
scope.selected = [scope.obj['10'].subItem];
928+
});
929+
expect(element.val()).toEqual(['10']);
930+
931+
// Now test view -> model
932+
933+
element.find('option')[0].selected = true;
934+
element.find('option')[1].selected = false;
935+
browserTrigger(element, 'change');
936+
expect(scope.selected).toEqual([scope.obj['10'].subItem]);
937+
938+
// Now reload the object
939+
scope.$apply(function() {
940+
scope.obj = {
941+
'10': {
942+
subItem: {label: 'new ten', id: 10}
943+
},
944+
'20': {
945+
subItem: {label: 'new twenty', id: 20}
946+
}
947+
};
948+
});
949+
expect(element.val()).toEqual(['10']);
950+
expect(scope.selected[0].id).toBe(10);
951+
expect(scope.selected.length).toBe(1);
952+
});
953+
954+
955+
it('It should use the "value" variable to represent items in the array as well as for the ' +
956+
'selected values in track by expression (single&object)', function() {
957+
createSelect({
958+
'ng-model': 'selected',
959+
'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)'
960+
});
961+
962+
// First test model -> view
963+
964+
scope.$apply(function() {
965+
scope.selected = scope.obj['10'].subItem;
966+
});
967+
expect(element.val()).toEqual('10');
968+
969+
970+
scope.$apply(function() {
971+
scope.selected = scope.obj['10'].subItem;
972+
});
973+
expect(element.val()).toEqual('10');
974+
975+
// Now test view -> model
976+
977+
element.find('option')[0].selected = true;
978+
browserTrigger(element, 'change');
979+
expect(scope.selected).toEqual(scope.obj['10'].subItem);
980+
981+
// Now reload the object
982+
scope.$apply(function() {
983+
scope.obj = {
984+
'10': {
985+
subItem: {label: 'new ten', id: 10}
986+
},
987+
'20': {
988+
subItem: {label: 'new twenty', id: 20}
989+
}
990+
};
991+
});
992+
expect(element.val()).toEqual('10');
993+
expect(scope.selected.id).toBe(10);
836994
});
837995
});
838996

0 commit comments

Comments
 (0)