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

ngMin/ngMax don't set min/max attributes #11636

Closed
twhitbeck opened this issue Apr 17, 2015 · 21 comments
Closed

ngMin/ngMax don't set min/max attributes #11636

twhitbeck opened this issue Apr 17, 2015 · 21 comments

Comments

@twhitbeck
Copy link
Contributor

It'd be nice if ngMin and ngMax set the min and max attributes accordingly. Similarly to how ngSrc and ngHref do. I realize I can use ng-attr-min and ng-attr-max, but that extra step seems unnecessary.

Ideally, for date types the same date parsing/formatting would be used as in the input directive for the respective types, but this behavior should happen even when there's no ngModel directive on the element. That means this needs to happen outside the directive linking function (since it doesn't get called if there's no ngModel directive present).

@pkozlowski-opensource
Copy link
Member

It'd be nice

Could you elaborate? What is the actual use-case?

@twhitbeck
Copy link
Contributor Author

In Chrome, if there's a min or max attribute on an date input or input[type=number], the native datepicker/number picker widgets impose some restrictions. For example, if an input[type=date] has a min attribute of 2015-01-01, you won't be able to select a date prior to 2015-01-01 in the calendar date picker.

@gkalpak
Copy link
Member

gkalpak commented Apr 18, 2015

Why not just use min/max ?
What do I miss ?

@twhitbeck
Copy link
Contributor Author

I think ngMin and ngMax should work like ngRequired. ngRequired exists so you can bind to the required attribute, and I expected ngMin and ngMax to do the same.
With ngMin say, I could bind to a date object on the scope which would set the min attribute on the element, so that in Chrome when I open the the datepicker I can't choose a date prior to that date.

@gkalpak
Copy link
Member

gkalpak commented Apr 21, 2015

ngRequired works the way it does, only because required is a boolean attribute.
min/max are not boolean attributes, therefore ngMin/ngMax don't have to work that way.

Basically, most of the validation directives (ngMin, ngMax, ngMinlength, ngMaxlength, ngPattern) work is the same way. ngRequired is the exception (for the reason described above).

@twhitbeck
Copy link
Contributor Author

I wasn't saying these would be needed or useful for the same reason as ngRequired.

If ngMin and ngMax also set the min and max attributes, then all I would have to do is add those attributes to my input and I would get the additional benefit of HTML5 datepicker widgets having proper restrictions in addition to the angular form validation. Does that make sense?

@gkalpak
Copy link
Member

gkalpak commented Apr 23, 2015

Yes and...no :)
You can use min/max (which is understood by both Angular and HTML5).

@twhitbeck
Copy link
Contributor Author

yes, but the benefit of ngMin/ngMax is that it is evaluated against the scope.

Take this example:

<input type="date" ng-model="minDate">
<input type="date" ng-model="date" ng-min="minDate">

Can you give me an example of what you mean?

@gkalpak
Copy link
Member

gkalpak commented Apr 24, 2015

@twhitbeck, you can use interpolation (with min/max). E.g.:

// In the controller:
$scope.minDate = '2015-04-01';
$scope.maxDate = '2015-04-30';

<!-- In the view:
<input type="date" ... min="{{minDate}}" max="{{maxDate}}" />

Demo

@twhitbeck
Copy link
Contributor Author

But what if I want to bind to a date, not a string, as in my example? In my situation I have one input[type=date] bound to property on the scope. On another I want to set the min to that date.
I realize I could do min="{{minDate | date:'yyyy-MM-dd'}}", but that feels clunky.

this might be out of scope, but I just wanted to suggest it as a nice-to-have.

There is no documentation on ngMin or ngMax, so I would be happy if there was some documentation and this caveat was listed in there.

  • ngMin/ngMax don't automatically set the min/max attribute, so if you want the benefit of browsers with HTML5 date input widgets correctly restricting user input, don't forget to add min/max="{{theDate | date:'yyyy-MM-dd'}}" for input[type=date] or min/max="{{theDate | date:'yyyy-MM'}}" for input[type=month]

@gkalpak
Copy link
Member

gkalpak commented Apr 30, 2015

I know it's annoying that ngMin/ngMax takes a date, while min/max takes a string. E.g. you don't have the same issue with type="number" where min/max and ngMin/ngMax).

Your comment mentions don't forget to add, which is incorrectly implying that both ngMin/ngMax and min/max are needed. This is not true; you should just use one of both.

The difference between them (funtinality-wise), which is also the reason why Angular supports both forms and why it is not a good idea for ngXyz to automatically set xyz, is that min/max activates both Angular and native HTML5 validation (which might not be always desirable). In contrary, ngMin/ngMax activates Angular validation only.

So, each one is appropriate for different usecases.

The only thing that I think can be improved here, is a mention in the docs that (for date-related inputs), if you want both Angular and HTML5 validation, you can use min/max="{{dateObj | date:'yyyy-MM-dd'}}".

@realityking
Copy link
Contributor

@gkalpak How is that different from ngRequired also activating HTML5 form validation?

Consider that according to the HTML5 spec at least for the number input the attribute min maps to aria-valuemin and is exposed to accessibility solutions.

@gkalpak
Copy link
Member

gkalpak commented Apr 30, 2015

@realityking, as already emntioned, ngRequired and ngDisabled are special cases and work the way they do only because required and disabled are boolean attributes. This is more thoroughly explained on the ngDisabled docs.

Consider that according to the HTML5 spec at least for the number input the attribute min maps to aria-valuemin and is exposed to accessibility solutions.

I don't see how that relates to the discussion above. You are free to use min/max if that is more appropriate for your usecase. Angular understands min/max perfectly.

Also, note that ngAria is already mapping ngMin/ngMax to aria-valuemin/aria-valuemax for "range" inputs. If you think it should also be doing it for other types of issues, please submit an issue about it.

@gkalpak
Copy link
Member

gkalpak commented Apr 30, 2015

BTW, there seems to be a bug in ngAria, so it's not working with ngMin/ngMax (not even for range inputs). Will investigate and submit an issue.

@gkalpak
Copy link
Member

gkalpak commented Apr 30, 2015

For the record: submitted issue (#11770) and PR (#11774)

@twhitbeck
Copy link
Contributor Author

@gkalpak I think I see the big picture, now. I think the solution is to amend the documentation on date related inputs, as you say. For one, there should be a mention of ngMin and ngMax, since they're completely undocumented. It would be logical there to note that they will not set min/max attributes and thus only activate angular validation. And that if both html5 validation and angular validation are desired (although it's not so much about validation, since we usually use novalidate, but rather it's about the function of the html5 date picker widgets to restrict user input) then the user only needs to set min/max attributes like min={{dateObj | date:'yyyy-MM-dd'}}

@gkalpak
Copy link
Member

gkalpak commented May 2, 2015

@twhitbeck or anyone else, would you be interested in submitting a PR to update the docs ?

@twhitbeck
Copy link
Contributor Author

Yes I'll probably get to it sometime next week

@gaffney
Copy link

gaffney commented Jun 22, 2015

Bump for documentation.

@twhitbeck
Copy link
Contributor Author

I'm trying to get to this, but having issues building on windows.

@gkalpak
Copy link
Member

gkalpak commented Jun 26, 2015

@twhitbeck, what kind of issues ?

twhitbeck added a commit to twhitbeck/angular.js that referenced this issue Jun 30, 2015
twhitbeck added a commit to twhitbeck/angular.js that referenced this issue Jul 22, 2015
twhitbeck added a commit to twhitbeck/angular.js that referenced this issue Jul 22, 2015
@Narretz Narretz closed this as completed in 966e01c Sep 1, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants