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

fix(ngModel): test & update correct model when running $validate - #7837

Closed
shahata wants to merge 1 commit into
angular:masterfrom
shahata:validators-fix
Closed

shahata wants to merge 1 commit into
angular:masterfrom
shahata:validators-fix

Conversation

@shahata

@shahata shahata commented Jun 14, 2014

Copy link
Copy Markdown
Contributor

If $validate is invoked when the model is already invalid, $validate should pass $$invalidModelValue to the validators, not $modelValue. More over, if the model has become valid, the previously invalid model should be assigned to $modelValue. Lastly, in case the model has become invalid, the previously valid model should be assigned to $$invalidModelValue.

Closes #7836

@shahata shahata added cla: yes and removed cla: no labels Jun 14, 2014
@shahata

shahata commented Jun 18, 2014

Copy link
Copy Markdown
Contributor Author

I've been thinking about this some more. What is the use case that makes the $validate important? I mean, when do we want to run validation rules again on existing input? It seems to me this creates more problems than it solves... Maybe we can change this so validators can be changed, but the change is effecting only future updates to the input and the existing input remains with the validity it already has?

@shahata

shahata commented Jun 23, 2014

Copy link
Copy Markdown
Contributor Author

Bumping this. It seems to me like a real issue that needs to be dealt with. Would be great to get some feedback on this...

@Narretz Narretz added this to the 1.3.0-beta.14 milestone Jun 23, 2014
Comment thread src/ng/directive/input.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I talked with @matsko about this in a8c7cb8#commitcomment-6681109 and the consensus is that there should be initial validation. So maybe this can dropped later. And I don't think it's not even that bad, as the modelController sets these odd values itself.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

okay, I'll remove the WIP comments and tidy this up then.

@Narretz

Narretz commented Jun 23, 2014

Copy link
Copy Markdown
Contributor

One example for re-running validation is some sort of constraint. If A is true, then B can be 8 characters long, but if A is false, then only 4. Then you need to change the validator on B and re-run it. Or do you mean something else?

@shahata

shahata commented Jun 23, 2014

Copy link
Copy Markdown
Contributor Author

Yes, but I was thinking that I'm not sure that this behavior is really required many times in real life. My general feeling was that developers will usually prefer to reset the value in B themselves once A is changed instead of letting angular re-validate. Never mind, I'm probably wrong about that.

@wawyed

wawyed commented Jun 30, 2014

Copy link
Copy Markdown

This fix is really important when you want to do cross field validation.

I'm having lots of issues trying to implement min-date/max-date validation in a datepicker. At the minute, when you $setViewValue of a value that validates to false against another property on the scope the value is set to undefined. However, if the property that validates against changes and you rerun the validators (via $validate()) the error is cleared and the field is set to $valid but the value stays as undefined.

@petebacondarwin

Copy link
Copy Markdown
Contributor

@shahata - I think it is important if you have asynchronous validation of any kind. Imagine a validator that looks up a value in a server. Only when the response arrives would you know if the field is valid or not.

Comment thread src/ng/directive/input.js Outdated

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we make this more clear $$setModel -> $$writeModelToScope ?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

done

If `$validate` is invoked when the model is already invalid, `$validate` should pass `$$invalidModelValue` to the validators, not `$modelValue`. More over, if the model has become valid, the previously invalid model should be assigned to `$modelValue`. Lastly, in case the model has become invalid, the previously valid model should be assigned to `$$invalidModelValue`.

Closes #7836

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

just as a quick note, this is less code if you say scope.$apply("max = 1"), and easier to read. I wouldn't worry about assignment expressions being removed from the grammar, that's not going to happen.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

+1

ckknight pushed a commit to ckknight/angular.js that referenced this pull request Jul 16, 2014
If `$validate` is invoked when the model is already invalid, `$validate`
should pass `$$invalidModelValue` to the validators, not `$modelValue`.

Moreover, if `$validate` is invoked and it is found that the invalid model
has become valid, this previously invalid model should be assigned to
`$modelValue`.

Lastly, if `$validate` is invoked and it is found that the model has
become invalid, the previously valid model should be assigned to
`$$invalidModelValue`.

Closes angular#7836
Closes angular#7837
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multiple problems with $validate after a validator changes

6 participants