Skip to content

Commit

Permalink
fix(ngModel): deregister from the form on scope not DOM destruction
Browse files Browse the repository at this point in the history
Due to animations, DOM might get destroyed much later than scope and so the element $destroy event
might get fired outside of $digest, which causes changes to the validation model go unobserved
until the next digest. By deregistering on scope  event, the deregistration always happens
in $digest and the form validation model changes will be observed.

Closes angular#4226
Closes angular#4779
  • Loading branch information
IgorMinar authored and jamesdaily committed Jan 27, 2014
1 parent d31a36f commit 861955c
Show file tree
Hide file tree
Showing 3 changed files with 94 additions and 18 deletions.
2 changes: 1 addition & 1 deletion src/ng/directive/input.js
Original file line number Diff line number Diff line change
Expand Up @@ -1222,7 +1222,7 @@ var ngModelDirective = function() {

formCtrl.$addControl(modelCtrl);

element.on('$destroy', function() {
scope.$on('$destroy', function() {
formCtrl.$removeControl(modelCtrl);
});
}
Expand Down
21 changes: 15 additions & 6 deletions test/ng/directive/formSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,18 +36,23 @@ describe('form', function() {
});


it('should remove the widget when element removed', function() {
it('should remove form control references from the form when nested control is removed from the DOM', function() {
doc = $compile(
'<form name="myForm">' +
'<input type="text" name="alias" ng-model="value" store-model-ctrl/>' +
'<input ng-if="inputPresent" name="alias" ng-model="value" store-model-ctrl/>' +
'</form>')(scope);
scope.inputPresent = true;
scope.$digest();

var form = scope.myForm;
control.$setValidity('required', false);
expect(form.alias).toBe(control);
expect(form.$error.required).toEqual([control]);

doc.find('input').remove();
// remove nested control
scope.inputPresent = false;
scope.$apply();

expect(form.$error.required).toBe(false);
expect(form.alias).toBeUndefined();
});
Expand Down Expand Up @@ -362,14 +367,15 @@ describe('form', function() {
});


it('should deregister a input when its removed from DOM', function() {
it('should deregister a input when it is removed from DOM', function() {
doc = jqLite(
'<form name="parent">' +
'<div class="ng-form" name="child">' +
'<input ng:model="modelA" name="inputA" required>' +
'<input ng-if="inputPresent" ng-model="modelA" name="inputA" required>' +
'</div>' +
'</form>');
$compile(doc)(scope);
scope.inputPresent = true;
scope.$apply();

var parent = scope.parent,
Expand All @@ -384,7 +390,10 @@ describe('form', function() {
expect(doc.hasClass('ng-invalid-required')).toBe(true);
expect(doc.find('div').hasClass('ng-invalid')).toBe(true);
expect(doc.find('div').hasClass('ng-invalid-required')).toBe(true);
doc.find('input').remove(); //remove child

//remove child input
scope.inputPresent = false;
scope.$apply();

expect(parent.$error.required).toBe(false);
expect(child.$error.required).toBe(false);
Expand Down
89 changes: 78 additions & 11 deletions test/ng/directive/inputSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -305,6 +305,84 @@ describe('ngModel', function() {
expect(element).toBeInvalid();
expect(element).toHaveClass('ng-invalid-required');
}));


it('should register/deregister a nested ngModel with parent form when entering or leaving DOM',
inject(function($compile, $rootScope) {

var element = $compile('<form name="myForm">' +
'<input ng-if="inputPresent" name="myControl" ng-model="value" required >' +
'</form>')($rootScope);
var isFormValid;

$rootScope.inputPresent = false;
$rootScope.$watch('myForm.$valid', function(value) { isFormValid = value; });

$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

$rootScope.inputPresent = true;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(false);
expect(isFormValid).toBe(false);
expect($rootScope.myForm.myControl).toBeDefined();

$rootScope.inputPresent = false;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

dealoc(element);
}));


it('should register/deregister a nested ngModel with parent form when entering or leaving DOM with animations',
function() {

// ngAnimate performs the dom manipulation after digest, and since the form validity can be affected by a form
// control going away we must ensure that the deregistration happens during the digest while we are still doing
// dirty checking.
module('ngAnimate');

inject(function($compile, $rootScope) {
var element = $compile('<form name="myForm">' +
'<input ng-if="inputPresent" name="myControl" ng-model="value" required >' +
'</form>')($rootScope);
var isFormValid;

$rootScope.inputPresent = false;
// this watch ensure that the form validity gets updated during digest (so that we can observe it)
$rootScope.$watch('myForm.$valid', function(value) { isFormValid = value; });

$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

$rootScope.inputPresent = true;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(false);
expect(isFormValid).toBe(false);
expect($rootScope.myForm.myControl).toBeDefined();

$rootScope.inputPresent = false;
$rootScope.$apply();

expect($rootScope.myForm.$valid).toBe(true);
expect(isFormValid).toBe(true);
expect($rootScope.myForm.myControl).toBeUndefined();

dealoc(element);
});
});
});


Expand Down Expand Up @@ -369,17 +447,6 @@ describe('input', function() {
});


it('should cleanup it self from the parent form', function() {
compileInput('<input ng-model="name" name="alias" required>');

scope.$apply();
expect(scope.form.$error.required.length).toBe(1);

inputElm.remove();
expect(scope.form.$error.required).toBe(false);
});


it('should update the model on "blur" event', function() {
compileInput('<input type="text" ng-model="name" name="alias" ng-change="change()" />');

Expand Down

0 comments on commit 861955c

Please sign in to comment.