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

fix(select): assign result of track exp to element value #9726

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 13 additions & 3 deletions src/ng/directive/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -407,10 +408,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);
Expand Down Expand Up @@ -532,7 +534,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++) {
Expand All @@ -556,9 +561,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){
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space after (trackFn) before {

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
});
Expand Down
43 changes: 28 additions & 15 deletions test/ng/directive/selectSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -726,6 +726,19 @@ describe('select', function() {
});


it('should use the tracked expression as option value', function () {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

space between function and ()

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',
Expand All @@ -735,12 +748,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');
Expand All @@ -758,12 +771,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');
Expand Down Expand Up @@ -840,16 +853,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);

Expand All @@ -861,7 +874,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);
});

Expand All @@ -879,12 +892,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

Expand All @@ -901,7 +914,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);
});
Expand Down Expand Up @@ -1338,20 +1351,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');
});


Expand Down