Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Issue #856: add ability to reset a form to its pristine state #1127

Closed
wants to merge 2 commits into from

10 participants

@pkozlowski-opensource
Collaborator

This is a fix for the (quite up-voted now) issue #856. The whole commit boils down to adding the $setPristine() method to both ng.directive:form.FormController and ng.directive:ngModel.NgModelController; tests are added as well.

Here is a jsFiddle that shows an issue: http://jsfiddle.net/pkozlowski_opensource/qT9pu/2/ (just type sth in a field and then delete it).

Here is another jsFillde with the issue fixed: http://jsfiddle.net/pkozlowski_opensource/2x8HD/4/ (just type sth in a field and then delete it and click on 'Revert edits').

@ajoslin

This would be nice, +1 :)

@IgorMinar

missing semicolon.

Grr, sloppy me... Preparing a commit with corrections.

@IgorMinar

affect -> propagate to

@IgorMinar

have you considered combining $setPristine with $setDirty into one method that takes a boolean flag? I'm not saying that that's what should be done, but I wonder if you did consider it.

Yes, for a moment I was feeling a bit uneasy about introducing $setPristine that is almost an inverse of $setDirty but finally decided to go with 2 separate methods for those reasons:

  • personally I find $setPristine and $setDirty more readable in the API as opposed to one method with a flag. For me the $setPristine is more explicit as compared to sth like: $setDirty(false)

  • I wanted to minimize API changes (especially the breaking ones). It is true that we could easily add a flag to the $setDirty() method but in this case it wouldn't be symmetrical with the existing $pristine / $dirty properties. So I was feeling like I would have to change those properties as well to have one, sth like $isDirty (or keep the $dirty flag only and get rid of $pristine altogether).

Another idea: we could keep $setPristine, $setDirty but refactor the implementation to extract the common code to a private method... but I'm really not sure if this is worth it in this case.

Don't think I've got more arguments, I was just thinking along those lines. Please let me know what you think, I can prepare an alternative PR.

@IgorMinar
Owner

otherwise this looks pretty good!

What version of Angular will this show up in? I don't see it in v1.0.2

@pkozlowski-opensource
Collaborator

OK, pushed another commit (df35bb7) with the discussed corrections. The only open question would be to keep both $setPristine / $setDirty methods or refactor it to sth else. Would be grateful for your input.

@btford
Owner

Looks good! Can we merge this, @IgorMinar ?

@mhevery
Owner

Thanks for your contribution! In order for us to be able to accept it, we ask you to sign our CLA (contributor's license agreement).

CLA is important for us to be able to avoid legal troubles down the road.

For individuals (a simple click-through form):
http://code.google.com/legal/individual-cla-v1.0.html

For corporations (print, sign and scan+email, fax or mail):
http://code.google.com/legal/corporate-cla-v1.0.html

@pkozlowski-opensource
Collaborator

@mhevery I've already signed the CLA during the previous contribution (Pawel Kozlowski, pkozlowski.opensource@gmail.com). From what I understand there is no need to sign it again, right?

@ProLoser

Random suggestion, maybe you guys should make an org team for people who have already signed? Don't necessarily have to give them any permissions whatsoever, but it would be a convenient and accessible way to see if they are already okay'd

@petebacondarwin

Don't forget this nice enhancement!

@ghost

Really, really, really want this feature. Any news on when it's expected to be approved and which version it'll ship in? Seeing as it was ready for approval over 2 months ago, this is kind of stretching out a bit ;o)

@rtcarlson

+1 for releasing this issue. it is the last outstanding feature i need for an app i'm developing.

@rbygrave

Perhaps I am misunderstanding this but the proposed fix as shown in http://jsfiddle.net/pkozlowski_opensource/2x8HD/4/ ... doesn't 'reset/rebind' the input field. The value in the input control doesn't get reset/rebound to the value in ng-model and keeps whatever value you have entered.

I was expecting the value in all the controls in the form to 'rebound' to the appropriate values as per their ng-model attribute.

@pkozlowski-opensource
Collaborator

@rbygrave Rob the goal of this PR is not to change model values but just change form's state. You still need to change model values like so: http://jsfiddle.net/UAMzA/4/

BTW: to see why this PR is really needed try this jsFiddle and reset a value: http://jsfiddle.net/UAMzA/5/
You will see that the input still has "invalid" classes even if we want to "reset" it for the next model entry.

@IgorMinar IgorMinar commented on the diff
src/ng/directive/form.js
((9 lines not shown))
+ * @description
+ * Sets the form to its pristine state.
+ *
+ * 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.
+ *
+ * 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 () {
+ element.removeClass(DIRTY_CLASS).addClass(PRISTINE_CLASS);
+ form.$dirty = false;
+ form.$pristine = true;
+ angular.forEach(form, function(value, key) {
+ if (value.$name && value.$setPristine) {
@IgorMinar Owner

why do you check for $name? anonymous form controls should be reset as well.

@pkozlowski-opensource Collaborator

I thought that anonymous form controls are not tracked as part of the form controller. Looked at the $addControl method:

https://github.com/angular/angular.js/blob/master/src/ng/directive/form.js#L64 and the same check for $removeControl:
https://github.com/angular/angular.js/blob/master/src/ng/directive/form.js#L70

If I'm correct then we don't have anonymous controls (inputs and sub-forms) on the FormController level and thus can't reset them to their pristine state. Still individual controls can be reset.

We could relay on the presence of the $setPristine function only but somehow it didn't feel "right". Let me know if you want me to remove this test for the $name property. In any case it won't change the fact that we can't (?) reset anonymous controls from a form level.

@IgorMinar might have missed something here but at least this was reasoning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@IgorMinar IgorMinar commented on the diff
src/ng/directive/form.js
((8 lines not shown))
+ *
+ * @description
+ * Sets the form to its pristine state.
+ *
+ * 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.
+ *
+ * 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 () {
+ element.removeClass(DIRTY_CLASS).addClass(PRISTINE_CLASS);
+ form.$dirty = false;
+ form.$pristine = true;
+ angular.forEach(form, function(value, key) {
@IgorMinar Owner

no angular, just forEach

@pkozlowski-opensource Collaborator

Ah, yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@pkozlowski-opensource
Collaborator

@IgorMinar Thnx for looking into this one! I'm going to swap the angular.forEach for forEach, squash commits, rebase etc. Please let me know if you want me to remove the check for $name (see my in-line comments).

@IgorMinar
Owner

so I worked on this on the plane and found several issues - mainly that anonymous form controls and nested forms were not supported at all.

I fixed these problems and added more tests. please see: https://github.com/IgorMinar/angular.js/commits/merge-pr-1127

if you are ok with these changes, I'll squash them and merge them.

@pkozlowski-opensource
Collaborator

@IgorMinar I had a look and it looks good - at least as far as I can tell :-)
I totally missed that nested forms are anonymous most of the time :-( Should have added tests for this....

Anyway, created a jsFiddle, it works OK: http://jsfiddle.net/Z9s3T/

Thnx once again for looking into this one!

@IgorMinar
Owner

landed as 733a97a

@IgorMinar IgorMinar closed this
@petebacondarwin
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
28 src/ng/directive/form.js
@@ -5,7 +5,8 @@ var nullFormCtrl = {
$addControl: noop,
$removeControl: noop,
$setValidity: noop,
- $setDirty: noop
+ $setDirty: noop,
+ $setPristine: noop
};
/**
@@ -119,6 +120,31 @@ function FormController(element, attrs) {
form.$pristine = false;
};
+ /**
+ * @ngdoc function
+ * @name ng.directive:form.FormController#$setPristine
+ * @methodOf ng.directive:form.FormController
+ *
+ * @description
+ * Sets the form to its pristine state.
+ *
+ * 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.
+ *
+ * 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 () {
+ element.removeClass(DIRTY_CLASS).addClass(PRISTINE_CLASS);
+ form.$dirty = false;
+ form.$pristine = true;
+ angular.forEach(form, function(value, key) {
@IgorMinar Owner

no angular, just forEach

@pkozlowski-opensource Collaborator

Ah, yes!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ if (value.$name && value.$setPristine) {
@IgorMinar Owner

why do you check for $name? anonymous form controls should be reset as well.

@pkozlowski-opensource Collaborator

I thought that anonymous form controls are not tracked as part of the form controller. Looked at the $addControl method:

https://github.com/angular/angular.js/blob/master/src/ng/directive/form.js#L64 and the same check for $removeControl:
https://github.com/angular/angular.js/blob/master/src/ng/directive/form.js#L70

If I'm correct then we don't have anonymous controls (inputs and sub-forms) on the FormController level and thus can't reset them to their pristine state. Still individual controls can be reset.

We could relay on the presence of the $setPristine function only but somehow it didn't feel "right". Let me know if you want me to remove this test for the $name property. In any case it won't change the fact that we can't (?) reset anonymous controls from a form level.

@IgorMinar might have missed something here but at least this was reasoning.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ value.$setPristine();
+ }
+ });
+ };
}
View
16 src/ng/directive/input.js
@@ -947,6 +947,22 @@ var NgModelController = ['$scope', '$exceptionHandler', '$attrs', '$element', '$
parentForm.$setValidity(validationErrorKey, isValid, this);
};
+ /**
+ * @ngdoc function
+ * @name ng.directive:ngModel.NgModelController#$setPristine
+ * @methodOf ng.directive:ngModel.NgModelController
+ *
+ * @description
+ * Sets the control to its pristine state.
+ *
+ * This method can be called to remove the 'ng-dirty' class and set the control to its pristine
+ * state (ng-pristine class).
+ */
+ this.$setPristine = function () {
+ this.$dirty = false;
+ this.$pristine = true;
+ $element.removeClass(DIRTY_CLASS).addClass(PRISTINE_CLASS);
+ }
/**
* @ngdoc function
View
26 test/ng/directive/formSpec.js
@@ -327,4 +327,30 @@ describe('form', function() {
expect(doc).toBeDirty();
});
});
+
+ describe('set pristine state', function() {
+
+ beforeEach(function() {
+ doc = $compile(
+ '<form name="form">' +
+ '<input ng-model="name" name="name" store-model-ctrl/>' +
+ '</form>')(scope);
+
+ scope.$digest();
+ });
+
+ it('should set form and controls to the pristine state', function() {
+
+ var form = scope.form, input = doc.find('input').eq(0);
+
+ control.$setViewValue('');
+ scope.$apply();
+ expect(doc).toBeDirty();
+
+ form.$setPristine();
+ expect(doc).toBePristine();
+ expect(input).toBePristine();
+ expect(control.$pristine).toBeTruthy();
+ });
+ });
});
View
12 test/ng/directive/inputSpec.js
@@ -117,6 +117,18 @@ describe('NgModelController', function() {
});
});
+ describe('setPristine', function() {
+
+ it('should set control to its pristine state', function() {
+ ctrl.$setViewValue('edit');
+ expect(ctrl.$dirty).toBe(true);
+ expect(ctrl.$pristine).toBe(false);
+
+ ctrl.$setPristine();
+ expect(ctrl.$dirty).toBe(false);
+ expect(ctrl.$pristine).toBe(true);
+ });
+ });
describe('view -> model', function() {
Something went wrong with that request. Please try again.