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. A
cache was added in order to improve performance getting the associated
value for a given track expression.

This commit adds one explicit test for this behavior, and changes several
other trackBy tests to reflect the desired behavior as well.

Closes #9718
Fixes #9592
This commit is contained in:
Juan Gabriel Jimenez Campos
2014-10-21 16:08:21 +02:00
committed by Jeff Cross
parent c3b3b90bc9
commit 4b4098bfca
2 changed files with 70 additions and 25 deletions

View File

@@ -355,6 +355,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
valuesFn = $parse(match[7]),
track = match[8],
trackFn = track ? $parse(match[8]) : null,
trackKeysCache = {},
// This is an array of array of existing option groups in DOM.
// We try to reuse these if possible
// - optionGroupsCache[0] is the options with no option group
@@ -405,10 +406,11 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
if (multiple) {
viewValue = [];
forEach(selectElement.val(), function(selectedKey) {
selectedKey = trackFn ? trackKeysCache[selectedKey] : selectedKey;
viewValue.push(getViewValue(selectedKey, collection[selectedKey]));
});
} else {
var selectedKey = selectElement.val();
var selectedKey = trackFn ? trackKeysCache[selectElement.val()] : selectElement.val();
viewValue = getViewValue(selectedKey, collection[selectedKey]);
}
ctrl.$setViewValue(viewValue);
@@ -530,7 +532,10 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
anySelected = false,
lastElement,
element,
label;
label,
optionId;
trackKeysCache = {};
// We now build up the list of options we need (we merge later)
for (index = 0; length = keys.length, index < length; index++) {
@@ -554,9 +559,14 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
// doing displayFn(scope, locals) || '' overwrites zero values
label = isDefined(label) ? label : '';
optionId = trackFn ? trackFn(scope, locals) : (keyName ? keys[index] : index);
if (trackFn) {
trackKeysCache[optionId] = key;
}
optionGroup.push({
// either the index into array or key from object
id: (keyName ? keys[index] : index),
id: optionId,
label: label,
selected: selected // determine if we should be selected
});

View File

@@ -722,10 +722,45 @@ describe('select', function() {
describe('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.obj = {'1': {score: 10, label: 'ten'}, '2': {score: 20, label: 'twenty'}};
});
it('should set the result of track by expression to element value', function() {
createSelect({
'ng-model': 'selected',
'ng-options': 'item.label for item in arr track by item.id'
});
scope.$apply(function() {
scope.selected = scope.arr[0];
});
expect(element.val()).toBe('10');
scope.$apply(function() {
scope.arr[0] = {id: 10, label: 'new ten'};
});
expect(element.val()).toBe('10');
element.children()[1].selected = 'selected';
browserTrigger(element, 'change');
expect(scope.selected).toEqual(scope.arr[1]);
});
it('should use the tracked expression as option value', function() {
createSelect({
'ng-model': 'selected',
'ng-options': 'item.label for item in arr track by item.id'
});
var options = element.find('option');
expect(options.length).toEqual(3);
expect(sortedHtml(options[0])).toEqual('<option value="?"></option>');
expect(sortedHtml(options[1])).toEqual('<option value="10">ten</option>');
expect(sortedHtml(options[2])).toEqual('<option value="20">twenty</option>');
});
it('should preserve value even when reference has changed (single&array)', function() {
createSelect({
'ng-model': 'selected',
@@ -735,12 +770,12 @@ describe('select', function() {
scope.$apply(function() {
scope.selected = scope.arr[0];
});
expect(element.val()).toBe('0');
expect(element.val()).toBe('10');
scope.$apply(function() {
scope.arr[0] = {id: 10, label: 'new ten'};
});
expect(element.val()).toBe('0');
expect(element.val()).toBe('10');
element.children()[1].selected = 1;
browserTrigger(element, 'change');
@@ -758,12 +793,12 @@ describe('select', function() {
scope.$apply(function() {
scope.selected = scope.arr;
});
expect(element.val()).toEqual(['0','1']);
expect(element.val()).toEqual(['10','20']);
scope.$apply(function() {
scope.arr[0] = {id: 10, label: 'new ten'};
});
expect(element.val()).toEqual(['0','1']);
expect(element.val()).toEqual(['10','20']);
element.children()[0].selected = false;
browserTrigger(element, 'change');
@@ -778,18 +813,18 @@ describe('select', function() {
});
scope.$apply(function() {
scope.selected = scope.obj['10'];
scope.selected = scope.obj['1'];
});
expect(element.val()).toBe('10');
scope.$apply(function() {
scope.obj['10'] = {score: 10, label: 'ten'};
scope.obj['1'] = {score: 10, label: 'ten'};
});
expect(element.val()).toBe('10');
element.val('20');
browserTrigger(element, 'change');
expect(scope.selected).toBe(scope.obj[20]);
expect(scope.selected).toBe(scope.obj['2']);
});
@@ -801,18 +836,18 @@ describe('select', function() {
});
scope.$apply(function() {
scope.selected = [scope.obj['10']];
scope.selected = [scope.obj['1']];
});
expect(element.val()).toEqual(['10']);
scope.$apply(function() {
scope.obj['10'] = {score: 10, label: 'ten'};
scope.obj['1'] = {score: 10, label: 'ten'};
});
expect(element.val()).toEqual(['10']);
element.children()[1].selected = 'selected';
browserTrigger(element, 'change');
expect(scope.selected).toEqual([scope.obj[10], scope.obj[20]]);
expect(scope.selected).toEqual([scope.obj['1'], scope.obj['2']]);
});
});
@@ -840,16 +875,16 @@ describe('select', function() {
scope.$apply(function() {
scope.selected = scope.arr[0].subItem;
});
expect(element.val()).toEqual('0');
expect(element.val()).toEqual('10');
scope.$apply(function() {
scope.selected = scope.arr[1].subItem;
});
expect(element.val()).toEqual('1');
expect(element.val()).toEqual('20');
// Now test view -> model
element.val('0');
element.val('10');
browserTrigger(element, 'change');
expect(scope.selected).toBe(scope.arr[0].subItem);
@@ -861,7 +896,7 @@ describe('select', function() {
subItem: {label: 'new twenty', id: 20}
}];
});
expect(element.val()).toBe('0');
expect(element.val()).toBe('10');
expect(scope.selected.id).toBe(10);
});
@@ -879,12 +914,12 @@ describe('select', function() {
scope.$apply(function() {
scope.selected = [scope.arr[0].subItem];
});
expect(element.val()).toEqual(['0']);
expect(element.val()).toEqual(['10']);
scope.$apply(function() {
scope.selected = [scope.arr[1].subItem];
});
expect(element.val()).toEqual(['1']);
expect(element.val()).toEqual(['20']);
// Now test view -> model
@@ -901,7 +936,7 @@ describe('select', function() {
subItem: {label: 'new twenty', id: 20}
}];
});
expect(element.val()).toEqual(['0']);
expect(element.val()).toEqual(['10']);
expect(scope.selected[0].id).toEqual(10);
expect(scope.selected.length).toBe(1);
});
@@ -1338,20 +1373,20 @@ describe('select', function() {
scope.selected = scope.values[1];
});
expect(element.val()).toEqual('1');
expect(element.val()).toEqual('2');
var first = jqLite(element.find('option')[0]);
expect(first.text()).toEqual('first');
expect(first.attr('value')).toEqual('0');
expect(first.attr('value')).toEqual('1');
var forth = jqLite(element.find('option')[3]);
expect(forth.text()).toEqual('forth');
expect(forth.attr('value')).toEqual('3');
expect(forth.attr('value')).toEqual('4');
scope.$apply(function() {
scope.selected = scope.values[3];
});
expect(element.val()).toEqual('3');
expect(element.val()).toEqual('4');
});