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

Commit 35da770

Browse files
committed
fix(select): make ngOptions support selectAs and trackBy together
This commit implements two functions, "isSelected()" and "getViewValue()" to properly compute an option's selected state and the model controller's viewValue respectively. These functions give proper precedence to "track by" and "select as" parts of the ngOptions comprehension expression, which were previously inconsistent and incompatible. Fixes #6564
1 parent 613d0a3 commit 35da770

File tree

2 files changed

+352
-54
lines changed

2 files changed

+352
-54
lines changed

src/ng/directive/select.js

+102-49
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,12 @@ var ngOptionsMinErr = minErr('ngOptions');
3535
* be bound to string values at present.
3636
* </div>
3737
*
38+
* <div class="alert alert-info">
39+
* **Note:** Using `selectAs` will bind the result of the `selectAs` expression to the model, but
40+
* the value of the `select` and `option` elements will be either the index (for array data sources)
41+
* or property name (for object data sources) of the value within the collection.
42+
* </div>
43+
*
3844
* @param {string} ngModel Assignable angular expression to data-bind to.
3945
* @param {string=} name Property name of the form under which the control is published.
4046
* @param {string=} required The control is considered valid only if value is entered.
@@ -69,7 +75,9 @@ var ngOptionsMinErr = minErr('ngOptions');
6975
* DOM element.
7076
* * `trackexpr`: Used when working with an array of objects. The result of this expression will be
7177
* used to identify the objects in the array. The `trackexpr` will most likely refer to the
72-
* `value` variable (e.g. `value.propertyName`).
78+
* `value` variable (e.g. `value.propertyName`). With this the selection is preserved
79+
* even when the options are recreated (e.g. reloaded from the server).
80+
* Note that this does not work when there is a `select` expression that is different from the `trackexpr`.
7381
*
7482
* @example
7583
<example module="selectExample">
@@ -147,6 +155,28 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
147155
nullModelCtrl = {$setViewValue: noop};
148156
// jshint maxlen: 100
149157

158+
function isSelected(selectAsFn, trackFn, valueName, valueFn, scope, viewValue, locals, selectedSet, mult) {
159+
var compareValueFn;
160+
if (selectAsFn) {
161+
compareValueFn = selectAsFn;
162+
} else if (trackFn) {
163+
var withValueName = {};
164+
withValueName[valueName] = viewValue;
165+
compareValueFn = trackFn;
166+
167+
viewValue = trackFn(withValueName);
168+
} else {
169+
compareValueFn = valueFn;
170+
}
171+
172+
var optionValue = compareValueFn(scope, locals);
173+
174+
if (mult) {
175+
return isDefined(selectedSet.remove(optionValue));
176+
}
177+
return viewValue === optionValue;
178+
};
179+
150180
return {
151181
restrict: 'E',
152182
require: ['select', '?ngModel'],
@@ -326,6 +356,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
326356

327357
var displayFn = $parse(match[2] || match[1]),
328358
valueName = match[4] || match[6],
359+
selectAs = / as /.test(match[0]) && match[1],
360+
selectAsFn = selectAs ? $parse(selectAs) : null,
329361
keyName = match[5],
330362
groupByFn = $parse(match[3] || ''),
331363
valueFn = $parse(match[2] ? match[1] : valueName),
@@ -371,41 +403,12 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
371403

372404
for(index = 1, length = optionGroup.length; index < length; index++) {
373405
if ((optionElement = optionGroup[index].element)[0].selected) {
374-
key = optionElement.val();
375-
if (keyName) locals[keyName] = key;
376-
if (trackFn) {
377-
for (trackIndex = 0; trackIndex < collection.length; trackIndex++) {
378-
locals[valueName] = collection[trackIndex];
379-
if (trackFn(scope, locals) == key) break;
380-
}
381-
} else {
382-
locals[valueName] = collection[key];
383-
}
384-
value.push(valueFn(scope, locals));
406+
value.push(getViewValue(optionElement, scope, locals, collection, true))
385407
}
386408
}
387409
}
388410
} else {
389-
key = selectElement.val();
390-
if (key == '?') {
391-
value = undefined;
392-
} else if (key === ''){
393-
value = null;
394-
} else {
395-
if (trackFn) {
396-
for (trackIndex = 0; trackIndex < collection.length; trackIndex++) {
397-
locals[valueName] = collection[trackIndex];
398-
if (trackFn(scope, locals) == key) {
399-
value = valueFn(scope, locals);
400-
break;
401-
}
402-
}
403-
} else {
404-
locals[valueName] = collection[key];
405-
if (keyName) locals[keyName] = key;
406-
value = valueFn(scope, locals);
407-
}
408-
}
411+
value = getViewValue(selectElement, scope, locals, collection);
409412
}
410413
ctrl.$setViewValue(value);
411414
render();
@@ -437,7 +440,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
437440
var selectedSet = false;
438441
if (multiple) {
439442
var viewValue = ctrl.$viewValue;
440-
if (trackFn && isArray(viewValue)) {
443+
if (trackFn && isArray(viewValue) && !selectAs) {
441444
selectedSet = new HashMap([]);
442445
var locals = {};
443446
for (var trackIndex = 0; trackIndex < viewValue.length; trackIndex++) {
@@ -451,6 +454,58 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
451454
return selectedSet;
452455
}
453456

457+
function getViewValue (el, scope, locals, collection, multiple) {
458+
var key = el.val();
459+
var value;
460+
var calculateViewValue;
461+
462+
if (selectAsFn || trackFn) {
463+
calculateViewValue = function () {
464+
var getterFn = selectAsFn || trackFn;
465+
var wrappedCollectionValue = {};
466+
wrappedCollectionValue[valueName] = collection[key];
467+
wrappedCollectionValue[keyName] = key;
468+
469+
for (var i in collection) {
470+
if (collection.hasOwnProperty(i)) {
471+
locals[valueName] = collection[i];
472+
if (keyName) locals[keyName] = i;
473+
if (getterFn(scope, locals) ==
474+
getterFn(scope, wrappedCollectionValue)) {
475+
/*
476+
* trackBy should not be used for final calculation, because it doesn't
477+
* necessarily return the expected value.
478+
*/
479+
return (selectAsFn||valueFn)(scope, locals);
480+
}
481+
}
482+
}
483+
};
484+
} else {
485+
calculateViewValue = function() {
486+
locals[valueName] = collection[key];
487+
if (keyName) locals[keyName] = key;
488+
return valueFn(scope, locals);
489+
};
490+
}
491+
492+
if (multiple) {
493+
if (keyName) locals[keyName] = key;
494+
calculateViewValue();
495+
return (selectAsFn || valueFn)(scope, locals);
496+
}
497+
498+
if (key == '?') {
499+
value = undefined;
500+
} else if (key === ''){
501+
value = null;
502+
} else {
503+
value = calculateViewValue();
504+
}
505+
506+
return value;
507+
}
508+
454509

455510
function scheduleRendering() {
456511
if (!renderScheduled) {
@@ -483,7 +538,6 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
483538
element,
484539
label;
485540

486-
487541
// We now build up the list of options we need (we merge later)
488542
for (index = 0; length = keys.length, index < length; index++) {
489543

@@ -501,27 +555,26 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
501555
optionGroup = optionGroups[optionGroupName] = [];
502556
optionGroupNames.push(optionGroupName);
503557
}
504-
if (multiple) {
505-
selected = isDefined(
506-
selectedSet.remove(trackFn ? trackFn(scope, locals) : valueFn(scope, locals))
507-
);
508-
} else {
509-
if (trackFn) {
510-
var modelCast = {};
511-
modelCast[valueName] = viewValue;
512-
selected = trackFn(scope, modelCast) === trackFn(scope, locals);
513-
} else {
514-
selected = viewValue === valueFn(scope, locals);
515-
}
516-
selectedSet = selectedSet || selected; // see if at least one item is selected
517-
}
558+
559+
selected = isSelected(
560+
selectAsFn,
561+
trackFn,
562+
valueName,
563+
valueFn,
564+
scope,
565+
viewValue,
566+
locals,
567+
selectedSet,
568+
multiple);
569+
selectedSet = selectedSet || selected;
570+
518571
label = displayFn(scope, locals); // what will be seen by the user
519572

520573
// doing displayFn(scope, locals) || '' overwrites zero values
521574
label = isDefined(label) ? label : '';
522575
optionGroup.push({
523576
// either the index into array or key from object
524-
id: trackFn ? trackFn(scope, locals) : (keyName ? keys[index] : index),
577+
id: (keyName ? keys[index] : index),
525578
label: label,
526579
selected: selected // determine if we should be selected
527580
});

0 commit comments

Comments
 (0)