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

fix(form): make ngForm $pristine after nested control.$setPristine() (counter version) #13773

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
77 changes: 75 additions & 2 deletions src/ng/directive/form.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,9 @@
var nullFormCtrl = {
$addControl: noop,
$$renameControl: nullFormRenameControl,
$$increasePristine: noop,
$$decreasePristine: noop,
$$updatePristine: noop,
$removeControl: noop,
$setValidity: noop,
$setDirty: noop,
Expand Down Expand Up @@ -76,6 +79,7 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
form.$invalid = false;
form.$submitted = false;
form.$$parentForm = nullFormCtrl;
form.$$pristineChildsCounter = 0;

/**
* @ngdoc method
Expand Down Expand Up @@ -143,6 +147,10 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
}

control.$$parentForm = form;
// if form was pristine before - it will be after too
if (control.$pristine) {
form.$$pristineChildsCounter++;
}
};

// Private API: rename a form control
Expand Down Expand Up @@ -188,6 +196,10 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {

arrayRemove(controls, control);
control.$$parentForm = nullFormCtrl;

if (control.$pristine) {
form.$$pristineChildsCounter--;
}
};


Expand Down Expand Up @@ -238,6 +250,12 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
* state (ng-dirty class). This method will also propagate to parent forms.
*/
form.$setDirty = function() {
// see $setPristineBubbling
if (form.$pristine) {
form.$$parentForm.$$decreasePristine();
} else {
form.$$parentForm.$$updatePristine();
}
$animate.removeClass(element, PRISTINE_CLASS);
$animate.addClass(element, DIRTY_CLASS);
form.$dirty = true;
Expand All @@ -254,21 +272,76 @@ function FormController(element, attrs, $scope, $animate, $interpolate) {
*
* This method can be called to remove the 'ng-dirty' class and set the form to its pristine
* state (ng-pristine class). This method will also propagate to all the controls contained
* in this form.
* in this form and to the parent form.
*
* Setting a form back to a pristine state is often useful when we want to 'reuse' a form after
* saving or resetting it.
*/
form.$setPristine = function() {
form.$$setPristineBubbling();
forEach(controls, function(control) {
// since we force pristine state, we don't want nested controls to
// change it
control.$$setPristineCapturing();
});
};

// Private API: Sets the form to its pristine state.
// This method does not affect nested/parent controls.
form.$$setPristineSelf = function() {
$animate.setClass(element, PRISTINE_CLASS, DIRTY_CLASS + ' ' + SUBMITTED_CLASS);
form.$dirty = false;
form.$pristine = true;
form.$submitted = false;
};

// Private API: Sets the form to its pristine state.
// Propagates pristine-ness to parent form
form.$$setPristineBubbling = function() {
// propagate only if pristine state was actually changed
if (form.$dirty) {
form.$$parentForm.$$increasePristine();
} else {
// otherwise tell aprent form to calculate current value,
// since it can be changed after adding/removing nested controls.
// The same applies to $setDirty.
form.$$parentForm.$$updatePristine();
}
Copy link
Member

Choose a reason for hiding this comment

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

Nitpicking: The above lines can be "summarized" as

var isPristine = controls.every(function(control) { return control.$pristine; });

form.$$setPristineSelf();
};

// Private API: Sets the form to its pristine state.
// Propagates pristine-ness to the nested controls
form.$$setPristineCapturing = function() {
form.$$setPristineSelf();
forEach(controls, function(control) {
control.$setPristine();
control.$$setPristineCapturing();
});
};

// Pivate API: nested control become pristine
form.$$increasePristine = function() {
form.$$pristineChildsCounter++;
form.$$updatePristine();
};

// Pivate API: nested control become dirty
form.$$decreasePristine = function() {
form.$$pristineChildsCounter--;
form.$$updatePristine();
};

// Private API: update form pristine-ness
form.$$updatePristine = function() {
if (form.$$pristineChildsCounter === controls.length) {
// since we got update from nested controls, we don't want to
// propagate it to them
form.$$setPristineBubbling();
} else {
form.$setDirty();
}
};

/**
* @ngdoc method
* @name form.FormController#$setUntouched
Expand Down
21 changes: 21 additions & 0 deletions src/ng/directive/ngModel.js
Original file line number Diff line number Diff line change
Expand Up @@ -379,6 +379,21 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
* has not been changed from when first compiled.
*/
this.$setPristine = function() {
// propagate only if pristine state was actually changed
if (ctrl.$dirty) {
ctrl.$$parentForm.$$increasePristine();
} else {
// otherwise tell aprent form to calculate current value,
// since it can be changed after adding/removing nested controls.
// The same applies to $setDirty.
ctrl.$$parentForm.$$updatePristine();
}
ctrl.$$setPristineCapturing();
};

// Private API: Sets the control to its pristine state.
// This method does not affect parent form
this.$$setPristineCapturing = function() {
ctrl.$dirty = false;
ctrl.$pristine = true;
$animate.removeClass($element, DIRTY_CLASS);
Expand All @@ -397,6 +412,12 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
* from when first compiled.
*/
this.$setDirty = function() {
// see $setPristine
if (ctrl.$pristine) {
ctrl.$$parentForm.$$decreasePristine();
} else {
ctrl.$$parentForm.$$updatePristine();
}
ctrl.$dirty = true;
ctrl.$pristine = false;
$animate.removeClass($element, PRISTINE_CLASS);
Expand Down
206 changes: 203 additions & 3 deletions test/ng/directive/formSpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -714,9 +714,7 @@ describe('form', function() {
expect(form.$error.maxlength[0].$name).toBe('childform');

inputController.$setPristine();
expect(form.$dirty).toBe(true);

form.$setPristine();
expect(form.$dirty).toBe(false);

// remove child form
form.$removeControl(childformController);
Expand Down Expand Up @@ -1043,6 +1041,208 @@ describe('form', function() {
expect(nestedInputCtrl.$pristine).toBe(true);
expect(nestedInputCtrl.$dirty).toBe(false);
});


it('should be idempotent on freshly created form', function() {
doc = $compile(
'<form name="form">' +
'<input ng-model="inputModel" name="input">' +
'</form>')(scope);

var form = doc,
inputCtrl = doc.find('input').eq(0).controller('ngModel');

expect(form).toBePristine();

// many times
inputCtrl.$setPristine();
inputCtrl.$setPristine();
scope.$apply();
expect(form).toBePristine();

inputCtrl.$setDirty();
scope.$apply();
expect(form).toBeDirty();

// many times
inputCtrl.$setDirty();
inputCtrl.$setDirty();
scope.$apply();
expect(form).toBeDirty();

inputCtrl.$setPristine();
scope.$apply();
expect(form).toBePristine();
});

it('should propagate pristine-ness to the parent form', function() {
doc = $compile(
'<form name="parentForm">' +
'<div ng-form name="childForm"></div>' +
'</form>')(scope);

var parentForm = doc,
childForm = parentForm.find('div').eq(0),
childFormCtrl = scope.childForm;

childFormCtrl.$setDirty();
scope.$apply();
expect(parentForm).toBeDirty();

childFormCtrl.$setPristine();
scope.$apply();
expect(childForm).toBePristine();
expect(parentForm).toBePristine();
});

it('should be pristine if all the nested controls are pristine', function() {
doc = $compile(
'<form name="form">' +
'<div ng-form name="childForm">' +
'<input ng-model="inputModel1" name="input1">' +
'<input ng-model="inputModel2" name="input2">' +
'</div>' +
'</form>')(scope);

var form = doc,
childForm = form.find('div').eq(0),
input1 = form.find('input').eq(0),
input2 = form.find('input').eq(1),
inputCtrl1 = input1.controller('ngModel'),
inputCtrl2 = input2.controller('ngModel');

inputCtrl1.$setDirty();
inputCtrl1.$setDirty();
scope.$apply();
expect(form).toBeDirty();
expect(childForm).toBeDirty();

inputCtrl2.$setDirty();
inputCtrl2.$setDirty();
scope.$apply();
expect(form).toBeDirty();
expect(childForm).toBeDirty();

inputCtrl1.$setPristine();
scope.$apply();
expect(form).toBeDirty();
expect(childForm).toBeDirty();

inputCtrl2.$setPristine();
scope.$apply();
expect(form).toBePristine();
expect(childForm).toBePristine();
});

it('should be pristine if all the nested forms are pristine', function() {
doc = $compile(
'<form name="outerForm1">' +
'<div ng-form name="outerForm2">' +
'<div ng-form name="childForm1"></div>' +
'<div ng-form name="childForm2"></div>' +
'</div>' +
'</form>')(scope);

var outerForm1 = doc,
outerForm2 = doc.find('div').eq(0),
childFormCtrl1 = scope.childForm1,
childFormCtrl2 = scope.childForm2;

childFormCtrl1.$setDirty();
scope.$apply();
expect(outerForm1).toBeDirty();
expect(outerForm2).toBeDirty();
childFormCtrl2.$setDirty();
scope.$apply();
expect(outerForm1).toBeDirty();
expect(outerForm2).toBeDirty();

childFormCtrl1.$setPristine();
scope.$apply();
expect(outerForm1).toBeDirty();
expect(outerForm2).toBeDirty();

childFormCtrl2.$setPristine();
scope.$apply();
expect(outerForm1).toBePristine();
expect(outerForm2).toBePristine();
});

it('should properly handle added/removed controls and be idempotent', function() {

var test = function(input, inputCtrl) {
doc = $compile(
'<form name="outerForm">' +
'<div ng-form name="innerForm"></div>' +
'</form>')(scope);

var outerForm = doc,
innerForm = doc.find('div').eq(0),
innerFormCtrl = innerForm.controller('form');

inputCtrl.$setDirty();
inputCtrl.$setDirty();

// just add control does not change form pristine-ness
innerFormCtrl.$addControl(inputCtrl);
scope.$apply();
expect(innerForm).toBePristine();
expect(outerForm).toBePristine();

// change after adding
inputCtrl.$setDirty();
inputCtrl.$setDirty();
scope.$apply();
expect(innerForm).toBeDirty();

innerFormCtrl.$removeControl(inputCtrl);

// removed control does not affect
inputCtrl.$setPristine();
scope.$apply();
expect(innerForm).toBeDirty();
expect(outerForm).toBeDirty();

innerFormCtrl.$addControl(inputCtrl);
scope.$apply();
expect(innerForm).toBeDirty();
expect(outerForm).toBeDirty();

// single $setPristine after multiple $setDirty
inputCtrl.$setPristine();
scope.$apply();
expect(innerForm).toBePristine();
expect(outerForm).toBePristine();

// many times
innerFormCtrl.$removeControl(inputCtrl);
inputCtrl.$setPristine();
inputCtrl.$setPristine();
innerFormCtrl.$addControl(inputCtrl);
scope.$apply();
expect(innerForm).toBePristine();
expect(outerForm).toBePristine();

// single setDirty afterm multiple setPristine
inputCtrl.$setDirty();
scope.$apply();
expect(innerForm).toBeDirty();
expect(outerForm).toBeDirty();
};

var input1 = $compile('<input ng-model="inputModel" name="input">')(scope),
inputCtrl1 = input1.controller('ngModel'),

input2 = $compile('<div ng-form name="input"></div>')(scope),
inputCtrl2 = input2.controller('form');

// test for input
test(input1, inputCtrl1);
dealoc(doc);

// test for ng-form
test(input2, inputCtrl2);
});
Copy link
Member

Choose a reason for hiding this comment

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

I'd add a test to make sure a change propagates from a control through it's parent form to the outer-most form.

});

describe('$setUntouched', function() {
Expand Down