8000 fix(ngOptions): ensure that tracked properties are always watched · petebacondarwin/angular.js@150f372 · GitHub
[go: up one dir, main page]
Skip to content

Commit 150f372

Browse files
petebacondarwinpetebacondarwin
committed
fix(ngOptions): ensure that tracked properti 8000 es are always watched
Commit 47f9fc3 failed to account for changes to the tracked value of model items in a collection where the select was `multiple`. See angular#11743 (comment)
1 parent bf74c72 commit 150f372

File tree

2 files changed

+72
-46
lines changed

2 files changed

+72
-46
lines changed

‎src/ng/directive/ngOptions.js

Lines changed: 66 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -261,9 +261,13 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
261261
// Get the value by which we are going to track the option
262262
// if we have a trackFn then use that (passing scope and locals)
263263
// otherwise just hash the given viewValue
264-
var getTrackByValue = trackBy ?
265-
function(viewValue, locals) { return trackByFn(scope, locals); } :
266-
function getHashOfValue(viewValue) { return hashKey(viewValue); };
264+
var getTrackByValueFn = trackBy ?
265+
function(value, locals) { return trackByFn(scope, locals); } :
266+
function getHashOfValue(value) { return hashKey(value); };
267+
var getTrackByValue = function(value, key) {
268+
return getTrackByValueFn(value, getLocals(value, key));
269+
};
270+
267271
var displayFn = $parse(match[2] || match[1]);
268272
var groupByFn = $parse(match[3] || '');
269273
var disableWhenFn = $parse(match[4] || '');
@@ -290,6 +294,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
290294

291295
return {
292296
trackBy: trackBy,
297+
getTrackByValue: getTrackByValue,
293298
getWatchables: $parse(valuesFn, function(values) {
294299
// Create a collection of things that we would like to watch (watchedArray)
295300
// so that they can all be watched using a single $watchCollection
@@ -299,7 +304,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
299304

300305
Object.keys(values).forEach(function getWatchable(key) {
301306
var locals = getLocals(values[key], key);
302-
var selectValue = getTrackByValue(values[key], locals);
307+
var selectValue = getTrackByValueFn(values[key], locals);
303308
watchedArray.push(selectValue);
304309

305310
// Only need to watch the displayFn if there is a specific label expression
@@ -347,7 +352,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
347352
var value = optionValues[key];
348353
var locals = getLocals(value, key);
349354
var viewValue = viewValueFn(scope, locals);
350-
var selectValue = getTrackByValue(viewValue, locals);
355+
var selectValue = getTrackByValueFn(viewValue, locals);
351356
var label = displayFn(scope, locals);
352357
var group = groupByFn(scope, locals);
353358
var disabled = disableWhenFn(scope, locals);
@@ -361,7 +366,7 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
361366
items: optionItems,
362367
selectValueMap: selectValueMap,
363368
getOptionFromViewValue: function(value) {
364-
return selectValueMap[getTrackByValue(value, getLocals(value))];
369+
return selectValueMap[getTrackByValue(value)];
365370
},
366371
getViewValueFromOption: function(option) {
367372
// If the viewValue could be an object that may be mutated by the application,
@@ -439,44 +444,54 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
439444
};
440445

441446

442-
selectCtrl.writeValue = function writeNgOptionsValue(value) {
443-
var option = options.getOptionFromViewValue(value);
447+
// Update the controller methods for multiple selectable options
448+
if (!multiple) {
444449

445-
if (option && !option.disabled) {
446-
if (selectElement[0].value !== option.selectValue) {
447-
removeUnknownOption();
448-
removeEmptyOption();
450+
selectCtrl.writeValue = function writeNgOptionsValue(value) {
451+
var option = options.getOptionFromViewValue(value);
449452

450-
selectElement[0].value = option.selectValue;
451-
option.element.selected = true;
452-
option.element.setAttribute('selected', 'selected');
453-
}
454-
} else {
455-
if (value === null || providedEmptyOption) {
456-
removeUnknownOption();
457-
renderEmptyOption();
453+
if (option && !option.disabled) {
454+
if (selectElement[0].value !== option.selectValue) {
455+
removeUnknownOption();
456+
removeEmptyOption();
457+
458+
selectElement[0].value = option.selectValue;
459+
option.element.selected = true;
460+
option.element.setAttribute('selected', 'selected');
461+
}
458462
} else {
459-
removeEmptyOption();
460-
renderUnknownOption();
463+
if (value === null || providedEmptyOption) {
464+
removeUnknownOption();
465+
renderEmptyOption();
466+
} else {
467+
removeEmptyOption();
468+
renderUnknownOption();
469+
}
461470
}
462-
}
463-
};
471+
};
464472

465-
selectCtrl.readValue = function readNgOptionsValue() {
473+
selectCtrl.readValue = function readNgOptionsValue() {
466474

467-
var selectedOption = options.selectValueMap[selectElement.val()];
475+
var selectedOption = options.selectValueMap[selectElement.val()];
468476

469-
if (selectedOption && !selectedOption.disabled) {
470-
removeEmptyOption();
471-
removeUnknownOption();
472-
return options.getViewValueFromOption(selectedOption);
473-
}
474-
return null;
475-
};
477+
if (selectedOption && !selectedOption.disabled) {
478+
removeEmptyOption();
479+
removeUnknownOption();
480+
return options.getViewValueFromOption(selectedOption);
481+
}
482+
return null;
483+
};
476484

485+
// If we are using `track by` then we must watch the tracked value on the model
486+
// since ngModel only watches for object identity change
487+
if (ngOptions.trackBy) {
488+
scope.$watch(
489+
function() { return ngOptions.getTrackByValue(ngModelCtrl.$viewValue); },
490+
function() { ngModelCtrl.$render(); }
491+
);
492+
}
477493

478-
// Update the controller methods for multiple selectable options
479-
if (multiple) {
494+
} else {
480495

481496
ngModelCtrl.$isEmpty = function(value) {
482497
return !value || value.length === 0;
@@ -508,6 +523,22 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
508523

509524
return selections;
510525
};
526+
527+
// If we are using `track by` then we must watch these tracked values on the model
528+
// since ngModel only watches for object identity change
529+
if (ngOptions.trackBy) {
530+
531+
scope.$watchCollection(function() {
532+
if (isArray(ngModelCtrl.$viewValue)) {
533+
return ngModelCtrl.$viewValue.map(function(value) {
534+
return ngOptions.getTrackByValue(value);
535+
});
536+
}
537+
}, function() {
538+
ngModelCtrl.$render();
539+
});
540+
541+
}
511542
}
512543

513544

@@ -534,11 +565,6 @@ var ngOptionsDirective = ['$compile', '$parse', function($compile, $parse) {
534565
// We will re-render the option elements if the option values or labels change
535566
scope.$watchCollection(ngOptions.getWatchables, updateOptions);
536567

537-
// We also need to watch to see if the internals of the model changes, since
538-
// ngModel only watches for object identity change
539-
if (ngOptions.trackBy) {
540-
scope.$watchCollection(attr.ngModel, function() { ngModelCtrl.$render(); });
541-
}
542568
// ------------------------------------------------------------------ //
543569

544570

‎test/ng/directive/ngOptionsSpec.js

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
'use strict';
22

3-
describe('ngOptions', function() {
3+
ddescribe('ngOptions', function() {
44

55
var scope, formElement, element, $compile;
66

@@ -988,9 +988,9 @@ describe('ngOptions', function() {
988988

989989
expect(element.val()).toEqual(['10']);
990990

991-
// Update the properties on the object in the selected array, rather than replacing the whole object
991+
// Update the tracked propertiy on the object in the selected array, rather than replacing the whole object
992992
scope.$apply(function() {
993-
scope.selected[0] = {id: 20, label: 'new twenty'};
993+
scope.selected[0].id = 20;
994994
});
995995

996996
// The value of the select should change since the id property changed
@@ -1130,21 +1130,21 @@ describe('ngOptions', function() {
11301130
}).not.toThrow();
11311131
});
11321132

1133-
it('should re-render if a propery of the model is changed when using trackBy', function() {
1133+
it('should re-render if the tracked property of the model is changed when using trackBy', function() {
11341134

11351135
createSelect({
11361136
'ng-model': 'selected',
11371137
'ng-options': 'item for item in arr track by item.id'
11381138
});
11391139

11401140
scope.$apply(function() {
1141-
scope.selected = scope.arr[0];
1141+
scope.selected = {id: 10, label: 'ten'};
11421142
});
11431143

11441144
spyOn(element.controller('ngModel'), '$render');
11451145

11461146
scope.$apply(function() {
1147-
scope.selected.label = 'changed';
1147+
scope.arr[0].id = 20;
11481148
});
11491149

11501150
// update render due to equality watch

0 commit comments

Comments
 (0)
0