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

Commit

Permalink
fix(select): fix ng-change over firing when using trackBy
Browse files Browse the repository at this point in the history
closes #4118
  • Loading branch information
rschmukler committed Nov 5, 2015
1 parent 27c65fd commit 41671e2
Show file tree
Hide file tree
Showing 2 changed files with 70 additions and 11 deletions.
14 changes: 9 additions & 5 deletions src/components/select/select.js
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,6 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
syncLabelText();
};
selectMenuCtrl.refreshViewValue();
ngModelCtrl.$render();
}
});
});
Expand Down Expand Up @@ -430,7 +429,6 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
}
selectMenuCtrl.select(optionCtrl.hashKey, optionCtrl.value);
selectMenuCtrl.refreshViewValue();
ngModelCtrl.$render();
}
}
}
Expand Down Expand Up @@ -688,8 +686,15 @@ function SelectMenuDirective($parse, $mdUtil, $mdTheming) {
values.push(self.selected[hashKey]);
}
}
self.ngModel.$setViewValue(self.isMultiple ? values : values[0]);
self.ngModel.$render();
var usingTrackBy = self.ngModel.$options && self.ngModel.$options.trackBy;

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

if (usingTrackBy ? !angular.equals(prevVal, newVal) : prevVal != newVal) {

This comment has been minimized.

Copy link
@jordanms

jordanms Mar 2, 2016

If usingTrackBy, equality should be done using it, not full object equality. This implementation brings back the Angular circular reference problem that TrackBy fixes: angular/angular.js#7839

self.ngModel.$setViewValue(newVal);
self.ngModel.$render();
}
};

function renderMultiple() {
Expand Down Expand Up @@ -770,7 +775,6 @@ function OptionDirective($mdButtonInkRipple, $mdUtil) {
selectCtrl.deselect(optionCtrl.hashKey);
}
selectCtrl.refreshViewValue();
selectCtrl.ngModel.$render();
});
});

Expand Down
67 changes: 61 additions & 6 deletions src/components/select/select.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,9 @@ describe('<md-select>', function() {

var selectMenus = $document.find('md-select-menu');
selectMenus.remove();

var backdrops = $document.find('md-backdrop');
backdrops.remove();
}));

it('should preserve tabindex', inject(function($document) {
Expand Down Expand Up @@ -40,7 +43,7 @@ describe('<md-select>', function() {
expect(container.classList.contains('test')).toBe(true);
}));

it('closes the menu if the element on backdrop click', inject(function($document, $rootScope) {
it('calls md-on-close when the select menu closes', inject(function($document, $rootScope) {
var called = false;
$rootScope.onClose = function() {
called = true;
Expand All @@ -49,16 +52,44 @@ describe('<md-select>', function() {
openSelect(select);

// Simulate click bubble from option to select menu handler
select.triggerHandler({
type: 'click',
target: angular.element($document.find('md-option')[0])
});
clickOption(0);

waitForSelectClose();

expect(called).toBe(true);
}));

it('closes on backdrop click', inject(function($document) {
var select = setupSelect('ng-model="val"', [1, 2, 3]).find('md-select');
openSelect(select);

// Simulate click bubble from option to select menu handler
var backdrop = $document.find('md-backdrop');
expect(backdrop.length).toBe(1);
backdrop.triggerHandler('click');

waitForSelectClose();

backdrop = $document.find('md-backdrop');
expect(backdrop.length).toBe(0);
}));

it('should not trigger ng-change without a change when using trackBy', inject(function($rootScope) {
var changed = false;
$rootScope.onChange = function() { changed = true; };
$rootScope.val = { id: 1, name: 'Bob' };

var opts = [ { id: 1, name: 'Bob' }, { id: 2, name: 'Alice' } ];
var select = setupSelect('ng-model="$root.val" ng-change="onChange()" ng-model-options="{trackBy: \'$value.id\'}"', opts);
expect(changed).toBe(false);

openSelect(select);
clickOption(1);
waitForSelectClose();
expect($rootScope.val.id).toBe(2);
expect(changed).toBe(true);
}));

it('should set touched only after closing', inject(function($compile, $rootScope) {
var form = $compile('<form name="myForm">' +
'<md-select name="select" ng-model="val">' +
Expand Down Expand Up @@ -825,15 +856,20 @@ describe('<md-select>', function() {
}

function openSelect(el) {
if (el[0].nodeName != 'MD-SELECT') {
el = el.find('md-select');
}
try {
el.triggerHandler('click');
waitForSelectOpen();
el.triggerHandler('blur');
} catch(e) { }
} catch (e) { }
}

function closeSelect() {
inject(function($document) {
var backdrop = $document.find('md-backdrop');
if (!backdrop.length) throw Error('Attempted to close select with no backdrop present');
$document.find('md-backdrop').triggerHandler('click');
});
waitForSelectClose();
Expand All @@ -859,4 +895,23 @@ describe('<md-select>', function() {
});
}

function clickOption(index) {
inject(function($rootScope, $document) {
var openMenu = $document.find('md-select-menu');
var opt = $document.find('md-option')[index];

if (!openMenu.length) throw Error('No select menu currently open');
if (!opt) throw Error('Could not find option at index: ' + index);
var target = angular.element(opt);
angular.element(openMenu).triggerHandler({
type: 'click',
target: target
});
angular.element(openMenu).triggerHandler({
type: 'mouseup',
currentTarget: openMenu[0]
});
});
}

});

0 comments on commit 41671e2

Please sign in to comment.