Skip to content

Commit c258609

Browse files
jeffbcrossbullgare
authored andcommitted
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 angular#6564
1 parent f948131 commit c258609

File tree

2 files changed

+447
-55
lines changed

2 files changed

+447
-55
lines changed

src/ng/directive/select.js

+94-50
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,8 @@ 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).
7380
*
7481
* @example
7582
<example module="selectExample">
@@ -326,6 +333,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
326333

327334
var displayFn = $parse(match[2] || match[1]),
328335
valueName = match[4] || match[6],
336+
selectAs = / as /.test(match[0]) && match[1],
337+
selectAsFn = selectAs ? $parse(selectAs) : null,
329338
keyName = match[5],
330339
groupByFn = $parse(match[3] || ''),
331340
valueFn = $parse(match[2] ? match[1] : valueName),
@@ -371,41 +380,12 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
371380

372381
for(index = 1, length = optionGroup.length; index < length; index++) {
373382
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));
383+
value.push(getViewValue(optionElement, locals, collection));
385384
}
386385
}
387386
}
388387
} 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-
}
388+
value = getViewValue(selectElement, locals, collection);
409389
}
410390
ctrl.$setViewValue(value);
411391
render();
@@ -437,7 +417,7 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
437417
var selectedSet = false;
438418
if (multiple) {
439419
var viewValue = ctrl.$viewValue;
440-
if (trackFn && isArray(viewValue)) {
420+
if (trackFn && isArray(viewValue) && !selectAs) {
441421
selectedSet = new HashMap([]);
442422
var locals = {};
443423
for (var trackIndex = 0; trackIndex < viewValue.length; trackIndex++) {
@@ -451,6 +431,79 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
451431
return selectedSet;
452432
}
453433

434+
function getViewValue (el, locals, collection) {
435+
var key = el.val();
436+
var value;
437+
var calculateViewValue;
438+
439+
if (selectAsFn || trackFn) {
440+
calculateViewValue = function () {
441+
var getterFn = selectAsFn || trackFn;
442+
var wrappedCollectionValue = {};
443+
wrappedCollectionValue[valueName] = collection[key];
444+
wrappedCollectionValue[keyName] = key;
445+
446+
for (var i in collection) {
447+
if (collection.hasOwnProperty(i)) {
448+
locals[valueName] = collection[i];
449+
if (keyName) locals[keyName] = i;
450+
if (getterFn(scope, locals) ==
451+
getterFn(scope, wrappedCollectionValue)) {
452+
/*
453+
* trackBy should not be used for final calculation, because it doesn't
454+
* necessarily return the expected value.
455+
*/
456+
return (selectAsFn||valueFn)(scope, locals);
457+
}
458+
}
459+
}
460+
};
461+
} else {
462+
calculateViewValue = function() {
463+
locals[valueName] = collection[key];
464+
if (keyName) locals[keyName] = key;
465+
return valueFn(scope, locals);
466+
};
467+
}
468+
469+
if (multiple) {
470+
if (keyName) locals[keyName] = key;
471+
calculateViewValue();
472+
return (selectAsFn || valueFn)(scope, locals);
473+
}
474+
475+
if (key == '?') {
476+
value = undefined;
477+
} else if (key === ''){
478+
value = null;
479+
} else {
480+
value = calculateViewValue();
481+
}
482+
483+
return value;
484+
}
485+
486+
function isSelected(viewValue, locals, selectedSet) {
487+
var compareValueFn;
488+
if (selectAsFn) {
489+
compareValueFn = selectAsFn;
490+
} else if (trackFn) {
491+
var withValueName = {};
492+
withValueName[valueName] = viewValue;
493+
compareValueFn = trackFn;
494+
495+
viewValue = trackFn(withValueName);
496+
} else {
497+
compareValueFn = valueFn;
498+
}
499+
500+
var optionValue = compareValueFn(scope, locals);
501+
502+
if (multiple) {
503+
return isDefined(selectedSet.remove(optionValue));
504+
}
505+
return viewValue === optionValue;
506+
}
454507

455508
function scheduleRendering() {
456509
if (!renderScheduled) {
@@ -483,10 +536,8 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
483536
element,
484537
label;
485538

486-
487539
// We now build up the list of options we need (we merge later)
488540
for (index = 0; length = keys.length, index < length; index++) {
489-
490541
key = index;
491542
if (keyName) {
492543
key = keys[index];
@@ -501,27 +552,20 @@ var selectDirective = ['$compile', '$parse', function($compile, $parse) {
501552
optionGroup = optionGroups[optionGroupName] = [];
502553
optionGroupNames.push(optionGroupName);
503554
}
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-
}
555+
556+
selected = isSelected(
557+
viewValue,
558+
locals,
559+
selectedSet);
560+
selectedSet = selectedSet || selected;
561+
518562
label = displayFn(scope, locals); // what will be seen by the user
519563

520564
// doing displayFn(scope, locals) || '' overwrites zero values
521565
label = isDefined(label) ? label : '';
522566
optionGroup.push({
523567
// either the index into array or key from object
524-
id: trackFn ? trackFn(scope, locals) : (keyName ? keys[index] : index),
568+
id: (keyName ? keys[index] : index),
525569
label: label,
526570
selected: selected // determine if we should be selected
527571
});

0 commit comments

Comments
 (0)