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

fix(ngModel): let aliased validator directives work on any element #12658

Closed
wants to merge 1 commit into from

Conversation

Narretz
Copy link
Contributor

@Narretz Narretz commented Aug 23, 2015

ng-(pattern|minlength|maxlength) directives will now validate the
ngModel when on an element that is not an input or
a textarea.
The commit also adds a test for ng-required, although that validator
worked on all elements before this fix.

Fixes #12158

@gkalpak
Copy link
Member

gkalpak commented Aug 24, 2015

Doesn't it also affect ngMin/Max ? If so, it would be nice to have tests for them as well.
LGTM other than that.

@Narretz
Copy link
Contributor Author

Narretz commented Aug 27, 2015

Hmm ... min / ngMin and max / ngMax validators are only added for input|textarea[number|date]. So they cannot exist on other elements.
The whole problem stems from the fact that we made min/maxlength, required, and pattern into separate directives, which are expected to work with ngModel on all elements.

But before that, we made it so that ngPattern, ngMaxlength, ngMinlength, ngMax and ngMin will set their corresponding non-ng attributes, so that the validator directives only have to observe the non-ng attributes (instead of having another set of directives that watch ngXYZ), see d9b90d7 and 25541c1 That's why the aliased attribute path in $set would only set the attributes for input and textarea.

It's a bit confusing, but there I can't think of any drawbacks atm. There's just the case that e.g. ngMax on a non-input element will set max, too and make it observable, without actually adding a validator.

@gkalpak
Copy link
Member

gkalpak commented Aug 27, 2015

Hm...you are right...weird (that all alliased attributes are directives except for min/max).
I guess ideally min/max should be their own directives as well (unless there is some caveat I am missing), so we could make them also work on any element.

But this is not an ideal world, so this LGTM as is 👍

@Narretz
Copy link
Contributor Author

Narretz commented Aug 27, 2015

It seems we have basically split our validators into two groups: generic one's that validate the input / viewValue (which is always a string) and specific one's, such as date / number. min / max really only make sense for date / number, so it'd be hard to make them generic.
And thanks for the review!

Narretz added a commit to Narretz/angular.js that referenced this pull request Aug 28, 2015
`ng(Pattern|Minlength|Maxlength)` directives will now validate the
`ngModel` when on an element that is not an `input` or
a `textarea`. Previously, only their HTML5 counterparts worked on
every element.

This is because the three validators were extracted
into  separate directives (see 26d91b6
and 1be9bb9), whereas the aliased
attribute handling assumes the validators will only exist on
`input|textarea` (see d9b90d7 and
25541c1).

Since `ngMin` and `ngMax` are also aliased attributes, this means
observers of `min` and `max` will be fired if `ngMin` and `ngMax`
change. This will happen on any element, even if it does not have
an `ngModel`. However, since min/max validators are only ever added
as part of the `input[number|textarea]` types, even if the element
has an `ngModel`, no validators will be added.

Finally the commit also tests that `ng-required` works on any element,
although that validator worked on all elements before this fix.

Fixes angular#12158
Closes angular#12658
`ng(Pattern|Minlength|Maxlength)` directives will now validate the
`ngModel` when on an element that is not an `input` or
a `textarea`. Previously, only their HTML5 counterparts worked on
every element.

This is because the three validators were extracted
into  separate directives (see 26d91b6
and 1be9bb9), whereas the aliased
attribute handling assumes the validators will only exist on
`input|textarea` (see d9b90d7 and
25541c1).

Since `ngMin` and `ngMax` are also aliased attributes, this means
observers of `min` and `max` will be fired if `ngMin` and `ngMax`
change. This will happen on any element, even if it does not have
an `ngModel`. However, since min/max validators are only ever added
as part of the `input[number|textarea]` types, even if the element
has an `ngModel`, no validators will be added.

Finally the commit also tests that `ng-required` works on any element,
although that validator worked on all elements before this fix.

Fixes angular#12158
Closes angular#12658
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.

minlength validator binded to ng-model not input
3 participants