From 526498a91d7cc56193c105b8bf6064831879ded4 Mon Sep 17 00:00:00 2001 From: Jeff Cross Date: Thu, 9 Oct 2014 16:48:56 -0700 Subject: [PATCH] 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 --- docs/content/error/ngOptions/trkslct.ngdoc | 3 + src/ng/directive/select.js | 18 +-- test/ng/directive/selectSpec.js | 170 +++++++++++++++++++-- 3 files changed, 170 insertions(+), 21 deletions(-) diff --git a/docs/content/error/ngOptions/trkslct.ngdoc b/docs/content/error/ngOptions/trkslct.ngdoc index 31de2fb593a8..385832200466 100644 --- a/docs/content/error/ngOptions/trkslct.ngdoc +++ b/docs/content/error/ngOptions/trkslct.ngdoc @@ -3,6 +3,9 @@ @fullName Comprehension expression cannot contain both `select as` and `track by` expressions. @description +NOTE: This error was introduced in 1.3.0-rc.5, and was removed for 1.3.0-rc.6 in order to +accommodate existing apps that write complex enough `track by` expressions to work with `select as`. + This error occurs when 'ngOptions' is passed a comprehension expression that contains both a `select as` expression and a `track by` expression. These two expressions are fundamentally incompatible. diff --git a/src/ng/directive/select.js b/src/ng/directive/select.js index 80eaeaba4622..a83647b7e917 100644 --- a/src/ng/directive/select.js +++ b/src/ng/directive/select.js @@ -363,13 +363,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { //re-usable object to represent option's locals locals = {}; - if (trackFn && selectAsFn) { - throw ngOptionsMinErr('trkslct', - "Comprehension expression cannot contain both selectAs '{0}' " + - "and trackBy '{1}' expressions.", - selectAs, track); - } - if (nullOption) { // compile the element since there might be bindings in it $compile(nullOption)(scope); @@ -460,7 +453,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { function createIsSelectedFn(viewValue) { var selectedSet; if (multiple) { - if (!selectAs && trackFn && isArray(viewValue)) { + if (trackFn && isArray(viewValue)) { selectedSet = new HashMap([]); for (var trackIndex = 0; trackIndex < viewValue.length; trackIndex++) { @@ -470,15 +463,16 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) { } else { selectedSet = new HashMap(viewValue); } - } else if (!selectAsFn && trackFn) { + } else if (trackFn) { viewValue = callExpression(trackFn, null, viewValue); } + return function isSelected(key, value) { var compareValueFn; - if (selectAsFn) { - compareValueFn = selectAsFn; - } else if (trackFn) { + if (trackFn) { compareValueFn = trackFn; + } else if (selectAsFn) { + compareValueFn = selectAsFn; } else { compareValueFn = valueFn; } diff --git a/test/ng/directive/selectSpec.js b/test/ng/directive/selectSpec.js index f04e526e3416..3baad2e0f07a 100644 --- a/test/ng/directive/selectSpec.js +++ b/test/ng/directive/selectSpec.js @@ -817,22 +817,174 @@ describe('select', function() { }); + /** + * This behavior is broken and should probably be cleaned up later as track by and select as + * aren't compatible. + */ describe('selectAs+trackBy expression', function() { beforeEach(function() { - scope.arr = [{id: 10, label: 'ten'}, {id:'20', label: 'twenty'}]; - scope.obj = {'10': {score: 10, label: 'ten'}, '20': {score: 20, label: 'twenty'}}; + scope.arr = [{subItem: {label: 'ten', id: 10}}, {subItem: {label: 'twenty', id: 20}}]; + scope.obj = {'10': {subItem: {id: 10, label: 'ten'}}, '20': {subItem: {id: 20, label: 'twenty'}}}; }); - it('should throw a helpful minerr', function() { - expect(function() { + it('It should use the "value" variable to represent items in the array as well as for the ' + + 'selected values in track by expression (single&array)', function() { + createSelect({ + 'ng-model': 'selected', + 'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)' + }); + + // First test model -> view + + scope.$apply(function() { + scope.selected = scope.arr[0].subItem; + }); + expect(element.val()).toEqual('0'); + + scope.$apply(function() { + scope.selected = scope.arr[1].subItem; + }); + expect(element.val()).toEqual('1'); + + // Now test view -> model + + element.val('0'); + browserTrigger(element, 'change'); + expect(scope.selected).toBe(scope.arr[0].subItem); + + // Now reload the array + scope.$apply(function() { + scope.arr = [{ + subItem: {label: 'new ten', id: 10} + },{ + subItem: {label: 'new twenty', id: 20} + }]; + }); + expect(element.val()).toBe('0'); + }); + + + it('It should use the "value" variable to represent items in the array as well as for the ' + + 'selected values in track by expression (multiple&array)', function() { + createSelect({ + 'ng-model': 'selected', + 'multiple': true, + 'ng-options': 'item.subItem as item.subItem.label for item in arr track by (item.id || item.subItem.id)' + }); + + // First test model -> view + + scope.$apply(function() { + scope.selected = [scope.arr[0].subItem]; + }); + expect(element.val()).toEqual(['0']); + + scope.$apply(function() { + scope.selected = [scope.arr[1].subItem]; + }); + expect(element.val()).toEqual(['1']); + + // Now test view -> model + + element.find('option')[0].selected = true; + element.find('option')[1].selected = false; + browserTrigger(element, 'change'); + expect(scope.selected).toEqual([scope.arr[0].subItem]); + + // Now reload the array + scope.$apply(function() { + scope.arr = [{ + subItem: {label: 'new ten', id: 10} + },{ + subItem: {label: 'new twenty', id: 20} + }]; + }); + expect(element.val()).toEqual(['0']); + }); - createSelect({ - 'ng-model': 'selected', - 'ng-options': 'item.id as item.name for item in values track by item.id' - }); - }).toThrowMinErr('ngOptions', 'trkslct', "Comprehension expression cannot contain both selectAs 'item.id' and trackBy 'item.id' expressions."); + it('It should use the "value" variable to represent items in the array as well as for the ' + + 'selected values in track by expression (multiple&object)', function() { + createSelect({ + 'ng-model': 'selected', + 'multiple': true, + 'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)' + }); + + // First test model -> view + + scope.$apply(function() { + scope.selected = [scope.obj['10'].subItem]; + }); + expect(element.val()).toEqual(['10']); + + + scope.$apply(function() { + scope.selected = [scope.obj['10'].subItem]; + }); + expect(element.val()).toEqual(['10']); + + // Now test view -> model + + element.find('option')[0].selected = true; + element.find('option')[1].selected = false; + browserTrigger(element, 'change'); + expect(scope.selected).toEqual([scope.obj['10'].subItem]); + + // Now reload the object + scope.$apply(function() { + scope.obj = { + '10': { + subItem: {label: 'new ten', id: 10} + }, + '20': { + subItem: {label: 'new twenty', id: 20} + } + }; + }); + expect(element.val()).toEqual(['10']); + }); + + + it('It should use the "value" variable to represent items in the array as well as for the ' + + 'selected values in track by expression (single&object)', function() { + createSelect({ + 'ng-model': 'selected', + 'ng-options': 'val.subItem as val.subItem.label for (key, val) in obj track by (val.id || val.subItem.id)' + }); + + // First test model -> view + + scope.$apply(function() { + scope.selected = scope.obj['10'].subItem; + }); + expect(element.val()).toEqual('10'); + + + scope.$apply(function() { + scope.selected = scope.obj['10'].subItem; + }); + expect(element.val()).toEqual('10'); + + // Now test view -> model + + element.find('option')[0].selected = true; + browserTrigger(element, 'change'); + expect(scope.selected).toEqual(scope.obj['10'].subItem); + + // Now reload the object + scope.$apply(function() { + scope.obj = { + '10': { + subItem: {label: 'new ten', id: 10} + }, + '20': { + subItem: {label: 'new twenty', id: 20} + } + }; + }); + expect(element.val()).toEqual('10'); }); });