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

Commit ecd55d0

Browse files
chenlijun99Splaktar
authored andcommitted
fix(select): when using trackBy, trigger ng-change only when tracked property is different
* Add mdSelect demo to ease debugging * Add support for ng-model-options to be updated after initialization * Add and fix tests Closes #11108
1 parent 90c8b8d commit ecd55d0

File tree

5 files changed

+179
-21
lines changed

5 files changed

+179
-21
lines changed
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
<div layout="column" layout-align="center center" class="md-padding" ng-cloak>
2+
<div layout="row" layout-align="space-between">
3+
<div ng-controller="AppCtrl as ctrl" layout="column" flex="40">
4+
<div>
5+
<h1 class="md-title">Without trackBy</h1>
6+
<div layout="row">
7+
<md-input-container>
8+
<label>Items</label>
9+
<md-select ng-model="ctrl.selectedItem"
10+
ng-change="ctrl.modelHasChanged = true">
11+
<md-option ng-repeat="item in ctrl.items" ng-value="item">
12+
{{ item.name }}
13+
</md-option>
14+
</md-select>
15+
</md-input-container>
16+
</div>
17+
</div>
18+
<div layout="column">
19+
<h5>Initial model</h5>
20+
<code><pre>{{ ::ctrl.selectedItem | json }}</pre></code>
21+
<h5>Current model</h5>
22+
<code><pre>{{ ctrl.selectedItem | json }}</pre></code>
23+
<span ng-show="ctrl.modelHasChanged">Model has changed</span>
24+
</div>
25+
</div>
26+
27+
<div ng-controller="AppCtrl as ctrl" layout="column" flex="40">
28+
<div>
29+
<h1 class="md-title">With trackBy</h1>
30+
<div layout="row">
31+
<md-input-container>
32+
<label>Items</label>
33+
<md-select ng-model="ctrl.selectedItem"
34+
ng-change="ctrl.modelHasChanged = true"
35+
ng-model-options="{ trackBy: '$value.id' }">
36+
<md-option ng-repeat="item in ctrl.items" ng-value="item">
37+
{{ item.name }}
38+
</md-option>
39+
</md-select>
40+
</md-input-container>
41+
</div>
42+
</div>
43+
<div layout="column">
44+
<h5>Initial model</h5>
45+
<code><pre>{{ ::ctrl.selectedItem | json }}</pre></code>
46+
<h5>Current model</h5>
47+
<code><pre>{{ ctrl.selectedItem | json }}</pre></code>
48+
<span ng-show="ctrl.modelHasChanged">Model has changed</span>
49+
</div>
50+
</div>
51+
</div>
52+
</div>
Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
(function() {
2+
'use strict';
3+
angular
4+
.module('selectDemoTrackBy', ['ngMaterial', 'ngMessages'])
5+
.controller('AppCtrl', function() {
6+
this.selectedItem = {
7+
id: '5a61e00',
8+
name: 'Bob',
9+
randomAddedProperty: 123
10+
};
11+
12+
this.items = [
13+
{
14+
id: '5a61e00',
15+
name: 'Bob',
16+
},
17+
{
18+
id: '5a61e01',
19+
name: 'Max',
20+
},
21+
{
22+
id: '5a61e02',
23+
name: 'Alice',
24+
},
25+
];
26+
});
27+
})();
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
code {
2+
display: block;
3+
padding: 8px;
4+
}

src/components/select/select.js

Lines changed: 59 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -235,12 +235,13 @@ function SelectDirective($mdSelect, $mdUtil, $mdConstant, $mdTheming, $mdAria, $
235235

236236
// Use everything that's left inside element.contents() as the contents of the menu
237237
var multipleContent = isMultiple ? 'multiple' : '';
238+
var ngModelOptions = attr.ngModelOptions ? $mdUtil.supplant('ng-model-options="{0}"', [attr.ngModelOptions]) : '';
238239
var selectTemplate = '' +
239240
'<div class="md-select-menu-container" aria-hidden="true" role="presentation">' +
240-
'<md-select-menu role="presentation" {0}>{1}</md-select-menu>' +
241+
'<md-select-menu role="presentation" {0} {1}>{2}</md-select-menu>' +
241242
'</div>';
242243

243-
selectTemplate = $mdUtil.supplant(selectTemplate, [multipleContent, element.html()]);
244+
selectTemplate = $mdUtil.supplant(selectTemplate, [multipleContent, ngModelOptions, element.html()]);
244245
element.empty().append(valueEl);
245246
element.append(selectTemplate);
246247

@@ -742,28 +743,40 @@ function SelectMenuDirective($parse, $mdUtil, $mdConstant, $mdTheming) {
742743
return !self.options[self.hashGetter($viewValue)];
743744
};
744745

745-
// Allow users to provide `ng-model="foo" ng-model-options="{trackBy: 'foo.id'}"` so
746+
// Allow users to provide `ng-model="foo" ng-model-options="{trackBy: '$value.id'}"` so
746747
// that we can properly compare objects set on the model to the available options
747-
var trackByOption = $mdUtil.getModelOption(ngModel, 'trackBy');
748-
749-
if (trackByOption) {
750-
var trackByLocals = {};
751-
var trackByParsed = $parse(trackByOption);
752-
self.hashGetter = function(value, valueScope) {
753-
trackByLocals.$value = value;
754-
return trackByParsed(valueScope || $scope, trackByLocals);
755-
};
756-
// If the user doesn't provide a trackBy, we automatically generate an id for every
757-
// value passed in
758-
} else {
759-
self.hashGetter = function getHashValue(value) {
760-
if (angular.isObject(value)) {
761-
return 'object_' + (value.$$mdSelectId || (value.$$mdSelectId = ++selectNextId));
748+
//
749+
// If the user doesn't provide a trackBy, we automatically generate an id for every
750+
// value passed in with the getId function
751+
if ($attrs.ngModelOptions) {
752+
self.hashGetter = function(value) {
753+
var ngModelOptions = $parse($attrs.ngModelOptions)($scope);
754+
var trackByOption = ngModelOptions && ngModelOptions.trackBy;
755+
756+
if (trackByOption) {
757+
return $parse(trackByOption)($scope, { $value: value });
758+
} else if (angular.isObject(value)) {
759+
return getId(value);
762760
}
763761
return value;
764762
};
763+
} else {
764+
self.hashGetter = getId;
765765
}
766766
self.setMultiple(self.isMultiple);
767+
768+
/**
769+
* If the value is an object, get the unique, incremental id of the value.
770+
* If it's not an object, the value will be converted to a string and then returned.
771+
* @param value
772+
* @returns {string}
773+
*/
774+
function getId(value) {
775+
if (angular.isObject(value) && !angular.isArray(value)) {
776+
return 'object_' + (value.$$mdSelectId || (value.$$mdSelectId = ++selectNextId));
777+
}
778+
return value + '';
779+
}
767780
};
768781

769782
self.selectedLabels = function(opts) {
@@ -867,15 +880,41 @@ function SelectMenuDirective($parse, $mdUtil, $mdConstant, $mdTheming) {
867880
values.push(self.selected[hashKey]);
868881
}
869882
}
870-
var usingTrackBy = $mdUtil.getModelOption(self.ngModel, 'trackBy');
871883

872884
var newVal = self.isMultiple ? values : values[0];
873885
var prevVal = self.ngModel.$modelValue;
874886

875-
if (usingTrackBy ? !angular.equals(prevVal, newVal) : (prevVal + '') !== newVal) {
887+
if (!equals(prevVal, newVal)) {
876888
self.ngModel.$setViewValue(newVal);
877889
self.ngModel.$render();
878890
}
891+
892+
function equals(prevVal, newVal) {
893+
if (self.isMultiple) {
894+
if (!angular.isArray(prevVal)) {
895+
// newVal is always an array when self.isMultiple is true
896+
// thus, if prevVal is not an array they are different
897+
return false;
898+
} else if (prevVal.length !== newVal.length) {
899+
// they are different if they have different length
900+
return false;
901+
} else {
902+
// if they have the same length, then they are different
903+
// if an item in the newVal array can't be found in the prevVal
904+
var prevValHashes = prevVal.map(function(prevValItem) {
905+
return self.hashGetter(prevValItem);
906+
});
907+
return newVal.every(function(newValItem) {
908+
var newValItemHash = self.hashGetter(newValItem);
909+
return prevValHashes.some(function(prevValHash) {
910+
return prevValHash === newValItemHash;
911+
});
912+
});
913+
}
914+
} else {
915+
return self.hashGetter(prevVal) === self.hashGetter(newVal);
916+
}
917+
}
879918
};
880919

881920
function renderMultiple() {

src/components/select/select.spec.js

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -172,7 +172,11 @@ describe('<md-select>', function() {
172172
it('should not trigger ng-change without a change when using trackBy', function() {
173173
var changed = false;
174174
$rootScope.onChange = function() { changed = true; };
175-
$rootScope.val = { id: 1, name: 'Bob' };
175+
176+
// Since we're tracking by id, ng-change shouldn't be triggered
177+
// when we have two objects that are not strictly equivalent (one has a 'randomAddedProperty')
178+
// but that have the same tracked field
179+
$rootScope.val = { id: 1, name: 'Bob', randomAddedProperty: 'random' };
176180

177181
var opts = [{ id: 1, name: 'Bob' }, { id: 2, name: 'Alice' }];
178182
var select = setupSelect('ng-model="$root.val" ng-change="onChange()" ng-model-options="{trackBy: \'$value.id\'}"', opts);
@@ -185,6 +189,38 @@ describe('<md-select>', function() {
185189
expect(changed).toBe(true);
186190
});
187191

192+
it('should support trackBy to be updated', function() {
193+
var changed = false;
194+
$rootScope.onChange = function() { changed = true; };
195+
$rootScope.useTrackBy = false;
196+
$rootScope.trackByOption = '$value.id';
197+
198+
var opts = [ { id: 1, name: 'Bob' }, { id: 2, name: 'Alice' } ];
199+
$rootScope.val = opts[0];
200+
var select = setupSelect('ng-model="$root.val"' +
201+
'ng-change="onChange()"' +
202+
'ng-model-options="{ trackBy: $root.useTrackBy ? $root.trackByOption : undefined }"', opts);
203+
expect(changed).toBe(false);
204+
205+
$rootScope.$apply(function() {
206+
$rootScope.useTrackBy = true;
207+
// Since we're tracking by id, ng-change shouldn't be triggered
208+
// when we have two objects that are not strictly equivalent (one has a 'randomAddedProperty')
209+
// but that have the same tracked field
210+
$rootScope.val = { id: 1, name: 'Bob', randomAddedProperty: 'random' };
211+
});
212+
openSelect(select);
213+
clickOption(select, 0);
214+
$material.flushInterimElement();
215+
expect(changed).toBe(false);
216+
217+
openSelect(select);
218+
clickOption(select, 1);
219+
$material.flushInterimElement();
220+
expect($rootScope.val.id).toBe(2);
221+
expect(changed).toBe(true);
222+
});
223+
188224
it('should set touched only after closing', function() {
189225
var form = $compile('<form name="myForm">' +
190226
'<md-select name="select" ng-model="val">' +

0 commit comments

Comments
 (0)