Skip to content

Commit

Permalink
fix(ngOptions): iterate over the options collection in the same way a…
Browse files Browse the repository at this point in the history
…s `ngRepeat`

In `ngRepeat` if the object to be iterated over is "array-like" then it only iterates
over the numerical indexes rather than every key on the object. This prevents "helper"
methods from being included in the rendered collection.

This commit changes `ngOptions` to iterate in the same way.

BREAKING CHANGE:

Although it is unlikely that anyone is using it in this way, this change does change the
behaviour of `ngOptions` in the following case:

* you are iterating over an array-like object, using the array form of the `ngOptions` syntax
(`item.label for item in items`) and that object contains non-numeric property keys.

In this case these properties with non-numeric keys will be ignored.

** Here array-like is defined by the result of a call to this internal function:
https://github.com/angular/angular.js/blob/v1.4.0-rc.1/src/Angular.js#L198-L211 **

To get the desired behaviour you need to iterate using the object form of the `ngOptions` syntax
(`value.label` for (key, value) in items)`).

Closes angular#11733
  • Loading branch information
petebacondarwin committed May 1, 2015
1 parent cc96188 commit dfa722a
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 5 deletions.
22 changes: 17 additions & 5 deletions src/ng/directive/ngOptions.js
Original file line number Diff line number Diff line change
Expand Up @@ -325,13 +325,25 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
// The option values were already computed in the `getWatchables` fn,
// which must have been called to trigger `getOptions`
var optionValues = valuesFn(scope) || [];
var optionValuesKeys;

var keys = Object.keys(optionValues);
keys.forEach(function getOption(key) {

// Ignore "angular" properties that start with $ or $$
if (key.charAt(0) === '$') return;
if (!keyName && isArrayLike(optionValues)) {
optionValuesKeys = optionValues;
} else {
// if object, extract keys, in enumeration order, unsorted
optionValuesKeys = [];
for (var itemKey in optionValues) {
if (optionValues.hasOwnProperty(itemKey) && itemKey.charAt(0) !== '$') {
optionValuesKeys.push(itemKey);
}
}
}

var optionValuesLength = optionValuesKeys.length;

for (var index = 0; index < optionValuesLength; index++) {
var key = (optionValues === optionValuesKeys) ? index : optionValuesKeys[index];
var value = optionValues[key];
var locals = getLocals(value, key);
var viewValue = viewValueFn(scope, locals);
Expand All @@ -343,7 +355,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {

optionItems.push(optionItem);
selectValueMap[selectValue] = optionItem;
});
}

return {
items: optionItems,
Expand Down
41 changes: 41 additions & 0 deletions test/ng/directive/ngOptionsSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,47 @@ describe('ngOptions', function() {
});


it('should not include properties with non-numeric keys in array-like collections when using array syntax', function() {
createSelect({
'ng-model':'selected',
'ng-options':'value for value in values'
});

scope.$apply(function() {
scope.values = { 0: 'X', 1: 'Y', 2: 'Z', 'a': 'A', length: 3};
scope.selected = scope.values[1];
});

var options = element.find('option');
expect(options.length).toEqual(3);
expect(options.eq(0)).toEqualOption('X');
expect(options.eq(1)).toEqualOption('Y');
expect(options.eq(2)).toEqualOption('Z');

});


it('should include properties with non-numeric keys in array-like collections when using object syntax', function() {
createSelect({
'ng-model':'selected',
'ng-options':'value for (key, value) in values'
});

scope.$apply(function() {
scope.values = { 0: 'X', 1: 'Y', 2: 'Z', 'a': 'A', length: 3};
scope.selected = scope.values[1];
});

var options = element.find('option');
expect(options.length).toEqual(5);
expect(options.eq(0)).toEqualOption('X');
expect(options.eq(1)).toEqualOption('Y');
expect(options.eq(2)).toEqualOption('Z');
expect(options.eq(3)).toEqualOption('A');
expect(options.eq(4)).toEqualOption(3);
});


it('should render an object', function() {
createSelect({
'ng-model': 'selected',
Expand Down

0 comments on commit dfa722a

Please sign in to comment.