feat(ngModel): add $touched and $untouched states #7673

guzart · 2014-06-03T05:27:21Z

Sets the ngModel controller property $touched to True and $untouched to False whenever a 'blur' event is triggered over a control with the ngModel directive.

References #583

mary-poppins · 2014-06-03T05:27:23Z

Thanks for the PR! Please check the items below to help us merge this faster. See the contributing docs for more information.

Uses the issue template (#7673)

If you need to make changes to your pull request, you can update the commit with git commit --amend.
Then, update the pull request with git push -f.

Thanks again for your help!

guzart · 2014-06-03T05:54:24Z

@matsko Here's an initial PR.

Couple questions:

I feel it should have some relationship to the FormController, I saw that in AngularDart the Form#reset calls the ngModelController markAsUntouched. Not sure how it would apply to angular.js.
I think it's missing a way to reset back to untouched, similar to $setPristine, not sure if ngModel#$setUntouched would be enough.
On your (previous comment)[https://github.com/More states for form validation #583#issuecomment-44602946] you mentioned that it should be associated to FormController#$submitted, could you explain why? Seems to me that it makes the feature less versatile.

matsko · 2014-06-03T06:04:27Z

@guzart

Yes I thought about this too. Is there a good usecase to see if the form has been touched? Displaying a bunch of errors once one field has been blurred out of. Let's leave this out for now and see if it is requested.
Yes we do need a $setUntouched function. If you add then then make $$setTouched to only have one dollar sign as well and include some docs for both functions please.
Yes. The original idea was that when you submit a form it should mark everything as touched. But I think it's better if we just have $submitted and $touched for now. Having to propagate $submitted to each model is too messy and it feels weird. I'm fine with having a ng-if statement like:

<div ng-if="myForm.$submitted || myForm.myField.$touched">...</div>

matsko · 2014-06-03T06:05:14Z

Once those two functions are set and the $setClass code is there then it looks good to merge in.

matsko · 2014-06-03T06:05:39Z

Excellent work BTW :)

matsko · 2014-06-03T06:07:48Z

Also, for now, let's put the $submitted flag on pause until this PR is resolved: #5574

smashercosmo · 2014-06-03T07:08:22Z

Cool, I think this will also resolve my yesterday issue #7666

guzart · 2014-06-04T06:13:28Z

@matsko I updated the PR to use $animate.setClass and added the methods $setTouched, $setUntouched to the ngModelController, tho, I have to say am not super happy with their tests, any suggestions?

smashercosmo · 2014-06-04T07:12:06Z

Don't you think that parent form should also get these classes?

matsko · 2014-06-04T19:23:12Z

@guzart can you make another test and trigger the event using browserTrigger + to execute a blur event? Here's an example of how to call it: https://github.com/angular/angular.js/blob/master/test/ngAnimate/animateSpec.js#L294

Sets the ngModel controller property $touched to True and $untouched to False whenever a 'blur' event is triggered over a control with the ngModel directive. Also adds the $setTouched and $setUntouched methods to the NgModelController. References #583

guzart · 2014-06-06T03:40:55Z

hey @matsko, I've added the test that ensures ngModel sets the touched state on blur.
Let me know of any other change/add/remove, anything I can do to help.

guzart · 2014-06-06T03:41:36Z

@smashercosmo do you mean like myForm.$touched? do you have a particular use case?

smashercosmo · 2014-06-06T07:19:24Z

@guzart I thought it should work the same way .ng-dirty and .ng-pristine classes work. When one input becomes dirty the whole form also becomes dirty.

smashercosmo · 2014-06-06T09:57:53Z

src/ng/directive/input.js

@@ -2017,6 +2059,12 @@ var ngModelDirective = function() {
            });
          });
        }
+
+        element.on('blur', function(ev) {
+          scope.$apply(function() {


I don't like that we get one extra-digest here. May be it's better to move it inside upper $apply block and check event object for blur event?

I don't think that there is any other way.

We could have some code that checks if the updatedOn has a blur event, and then piggyback on the first digest, otherwise bind to the element the way it currently is.
I think it could be be a good optimization, not sure how expensive a digest is. @matsko what do you think?

matsko · 2014-06-10T05:26:18Z

@guzart this is an excellent PR. Great work. I'm just waiting on @smashercosmo's reply and then I will merge this in.

smashercosmo · 2014-06-10T07:12:11Z

@matsko my reply about what?))

matsko · 2014-06-10T07:30:32Z

@guzart #7673 (comment)

matsko · 2014-06-11T02:37:32Z

@guzart @smashercosmo let's apply the fixes in another PR. Perhaps they could be applied to other parts of NgModel.

I will continue now and merge this in.

matsko · 2014-06-11T02:42:22Z

This PR Looks Good to Me.

matsko · 2014-06-11T03:29:03Z

MERGED

matsko · 2014-06-11T03:29:07Z

Landed as adcc5a0

matsko self-assigned this Jun 3, 2014

matsko added this to the 1.3.0-beta.11 milestone Jun 3, 2014

guzart added cla: yes and removed cla: no labels Jun 5, 2014

smashercosmo reviewed Jun 6, 2014
View reviewed changes

jeffbcross modified the milestones: 1.3.0-beta.12, 1.3.0-beta.11 Jun 6, 2014

matsko closed this Jun 11, 2014

smashercosmo mentioned this pull request Jun 16, 2014

Possibility to make model $dirty even if nothing was changed #7666

Closed

guzart mentioned this pull request Aug 1, 2014

perf(input): prevent multiple $digest when input is blurred #8450

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(ngModel): add $touched and $untouched states #7673

feat(ngModel): add $touched and $untouched states #7673

guzart commented Jun 3, 2014

mary-poppins commented Jun 3, 2014

guzart commented Jun 3, 2014

matsko commented Jun 3, 2014

matsko commented Jun 3, 2014

matsko commented Jun 3, 2014

matsko commented Jun 3, 2014

smashercosmo commented Jun 3, 2014

guzart commented Jun 4, 2014

smashercosmo commented Jun 4, 2014

matsko commented Jun 4, 2014

guzart commented Jun 6, 2014

guzart commented Jun 6, 2014

smashercosmo commented Jun 6, 2014

smashercosmo Jun 6, 2014

matsko Jun 10, 2014

guzart Jun 11, 2014

matsko commented Jun 10, 2014

smashercosmo commented Jun 10, 2014

matsko commented Jun 10, 2014

matsko commented Jun 11, 2014

matsko commented Jun 11, 2014

matsko commented Jun 11, 2014

matsko commented Jun 11, 2014

feat(ngModel): add $touched and $untouched states #7673

feat(ngModel): add $touched and $untouched states #7673

Conversation

guzart commented Jun 3, 2014

mary-poppins commented Jun 3, 2014

guzart commented Jun 3, 2014

matsko commented Jun 3, 2014

matsko commented Jun 3, 2014

matsko commented Jun 3, 2014

matsko commented Jun 3, 2014

smashercosmo commented Jun 3, 2014

guzart commented Jun 4, 2014

smashercosmo commented Jun 4, 2014

matsko commented Jun 4, 2014

guzart commented Jun 6, 2014

guzart commented Jun 6, 2014

smashercosmo commented Jun 6, 2014

smashercosmo Jun 6, 2014

Choose a reason for hiding this comment

matsko Jun 10, 2014

Choose a reason for hiding this comment

guzart Jun 11, 2014

Choose a reason for hiding this comment

matsko commented Jun 10, 2014

smashercosmo commented Jun 10, 2014

matsko commented Jun 10, 2014

matsko commented Jun 11, 2014

matsko commented Jun 11, 2014

matsko commented Jun 11, 2014

matsko commented Jun 11, 2014