Skip to content
This repository has been archived by the owner on Aug 29, 2023. It is now read-only.

Commit

Permalink
fix(select): fix set form to pristine if ng-model for multiple select…
Browse files Browse the repository at this point in the history
… is predefined.

Bug introduced in 09bd5a3

Fixes #6556

Closes #6782

#breaking
  • Loading branch information
devversion authored and ThomasBurleson committed Jan 31, 2016
1 parent 4d535e2 commit 19966a3
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 3 deletions.
7 changes: 5 additions & 2 deletions src/components/select/select.js
Expand Up @@ -230,9 +230,12 @@ function SelectDirective($mdSelect, $mdUtil, $mdTheming, $mdAria, $compile, $par
});
}

if (formCtrl) {
if (formCtrl && angular.isDefined(attr.multiple)) {
$mdUtil.nextTick(function() {
formCtrl.$setPristine();
var hasModelValue = ngModelCtrl.$modelValue || ngModelCtrl.$viewValue;
if (hasModelValue) {
formCtrl.$setPristine();
}
});
}

Expand Down
27 changes: 26 additions & 1 deletion src/components/select/select.spec.js
Expand Up @@ -387,6 +387,19 @@ describe('<md-select>', function() {
expect(selectedOptions(el).length).toBe(0);
}));

it('should keep the form pristine when model is predefined', inject(function($rootScope, $timeout, $compile) {
$rootScope.model = 2;
$rootScope.opts = [1, 2, 3, 4];
$compile('<form name="testForm">' +
'<md-select ng-model="model" name="multiSelect">' +
'<md-option ng-repeat="opt in opts" ng-value="opt"></md-option>' +
'</md-select></form>')($rootScope);
$rootScope.$digest();
$timeout.flush();

expect($rootScope.testForm.$pristine).toBe(true);
}));

});

describe('view->model', function() {
Expand Down Expand Up @@ -637,13 +650,25 @@ describe('<md-select>', function() {
$rootScope.model = [];
$rootScope.opts = [1, 2, 3, 4];
$compile('<form name="testForm">' +
'<md-select ng-model="model", name="multiSelect" required="required" multiple="multiple">' +
'<md-select ng-model="model" name="multiSelect" required="required" multiple="multiple">' +
'<md-option ng-repeat="opt in opts" ng-value="opt"></md-option>' +
'</md-select></form>')($rootScope);
$rootScope.$digest();
expect($rootScope.testForm.$valid).toBe(false);
}));

it('should keep the form pristine when model is predefined', inject(function($rootScope, $timeout, $compile) {
$rootScope.model = [1, 2];
$rootScope.opts = [1, 2, 3, 4];
$compile('<form name="testForm">' +
'<md-select ng-model="model" name="multiSelect" multiple="multiple">' +
'<md-option ng-repeat="opt in opts" ng-value="opt"></md-option>' +
'</md-select></form>')($rootScope);
$rootScope.$digest();
$timeout.flush();

expect($rootScope.testForm.$pristine).toBe(true);
}));
});

describe('view->model', function() {
Expand Down

3 comments on commit 19966a3

@cstephe
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Anyone else have an issue with this? Basically it means if you dynamically add/remove these in an ngrepeat via a filter your form keeps getting marked as pristine. This fix seems like such a hack to fix an issue, like why isn't this fix tring to fix the reason its modifying the form to begin with? I'm going to be forking and looking into this, but if someone sees this and has any information that might be helpful let me know.

@Splaktar
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@cstephe please open an issue with a reproduction and reference this commit.

@cstephe
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Splaktar cool, done: #11490 reading up on PR process, will (attempt) submit soon.

Please sign in to comment.