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

<input type="number" max="{{max}}"> does not set $error.max property #2404

Closed
Scaalp opened this issue Apr 14, 2013 · 46 comments
Closed

<input type="number" max="{{max}}"> does not set $error.max property #2404

Scaalp opened this issue Apr 14, 2013 · 46 comments

Comments

@Scaalp
Copy link

Scaalp commented Apr 14, 2013

When using an input of type number, with the max (or min) attribute set from the model, then the max field is not set in the error object of the input. This is due to the fact that in method "numberInputType", the value of attr.max is undefined. It is (probably) set from the model only later.

Checking the HTML, shows that the max attribute gets the correct value, set from the model.

In summary, the following does not work as expected:
input type="number" max="{{max}}"
The $error for this input field does not contain a boolean value named "max".

Here is a demo : http://plnkr.co/edit/YiExG5GpFt5MKKXoO4GE

@vorburger
Copy link

this is possibly related to issue #2144 - there clearly are some issue with cross-browser input type=number.. :(

@jamie-pate
Copy link

I think the parseFloat line should be moved inside the maxValidator function. The interp seems like it's already being done beforehand, but the validator is trying to be 'smart' and pre-compute it. I've tried it and it seems to work well (in chrome)

@jamie-pate
Copy link

Will require further testing and unit tests, but this fixed it for me:
https://github.com/jamie-pate/angular.js/compare/scwp

@jamie-pate
Copy link

This appears to be solved in the latest angular 1.2.3

@rmourato
Copy link

rmourato commented Jan 8, 2014

Problem still occurring in v1.2.7.

The documentation may indicate that it's expecting a string, but it does go against expectation as this complicates things if you want all properties to be databound and not something mid-term.
Silly thing is that the spinner still limits min/max properly according to the {{expression}}, and the browser also validates the form correctly on submission if "novalidate" isn't present.
The missing bit, really, is $error.min/max not updating.

@IgorMinar
Copy link
Contributor

I can't repro this with 1.2.7: http://plnkr.co/edit/4HU4jdG1ljrnoXYuegQo?p=preview

it appears to work correctly. can you please post expected vs actual result and any other instructions needed to repro the bug.

@rmourato
Copy link

rmourato commented Jan 9, 2014

Hi Igor,

Thanks for having a look at this!
I can see everything works fine in your Plunkr and can't get it to break, so I don't know why on my case it doesn't work. The only thing I'm realising is that, on the generated HTML for my input element, angular isn't adding the ng-valid-min and ng-valid-max classes as in your Plunkr.

An adapted version of yours generated (works):

<input type="number" name="P4" min="0" max="100" ng-model="container.P4.quantity" 
class="ng-pristine ng-valid ng-valid-number ng-valid-max ng-valid-min">

My generated HTML (doesn't work):

<input type="number" name="P4" min="0" max="100" ng-model="container.P4.Value" 
class="ng-pristine ng-valid ng-valid-number">

Resulting in:

myform.P4.$error = {"number":false}
myform.P4.$error.min =
myform.P4.$error.max =

I can't reproduce my problem in the Plunkr, so I'm at a loss.
Any ideas on what I might be doing wrong?

@Deklin
Copy link
Contributor

Deklin commented Feb 19, 2014

We are having this issue with 1.2.13
Here is a fiddle showing the issue. Please use chrome to test:
http://jsfiddle.net/sberube/BnE73/

Steps:

  1. For enter a number, type in 115
    1a. Observe that value.$error.max goes to true meaning the # has failed MAX validation
  2. Change the dropdown to Unlimited, this should set {{max}} to 500 where before it was 100
    2a. Observe that the $error still shows a max error when the number is now in the valid range.
  3. Type 116 in the input field
    3a. observe that $error.max goes to false

So the {{max}} interpolation is not re-validating the input, but changing the input causes re-validation including the new value. This is the bug we are experiencing with 1.2.13

EDIT:
We are going to see if this solves our issue until angular natively supports this:
http://jsfiddle.net/g/s5gKC/

@Deklin
Copy link
Contributor

Deklin commented Feb 19, 2014

Caitlin;
According to the documentation it does:
http://docs.angularjs.org/api/ng/input/input%5Bnumber%5D

On Wed, Feb 19, 2014 at 11:03 AM, Caitlin Potter
[email protected]:

I'm not sure what you were expecting, angular doesn't support the maxattribute

Reply to this email directly or view it on GitHubhttps://github.com//issues/2404#issuecomment-35514255
.

@rwinzhang
Copy link

The issue appears on this case http://plnkr.co/edit/4CPpJl when min and max value are dynamically set.

@robjames
Copy link

robjames commented Jun 2, 2014

I think the solution to this is the novalidate attribute - Well, solved it for me anyway.

@jamie-pate
Copy link

I am currently having a related issue. In my case, when the numberInputType controller is instantiated, attr.min and/or attr.max evaluate as falsy. The solution is to replace if (attr.max) with if ('max' in attr) and if (attr.min) with if ('min' in attr)

Unfortunately my employer thus far has not approved (and looks like it won't any time soon) the CLA so someone else will have to make this PR.

PS: also needs a check for NaN since attr.max could be anything during validation now.

@gkocjan
Copy link

gkocjan commented Jul 1, 2014

Still present in 1.2.18

@caitp
Copy link
Contributor

caitp commented Jul 1, 2014

@gkocjan can you provide an example? this seems to have been fixed back in 1.1.3, based on a quick bisect of releases --- http://plnkr.co/edit/7KNcOK7CVSjWDUt9JHbN?p=preview (works in 1.1.3, fails in 1.1.2), probably was originally fixed by bb8448c (It's also working in 1.2.0, 1.2.18, 1.2.17, snapshot, 1.3-beta.1, ...)

@petebacondarwin we could probably close this issue if this was fixed by that CL

@gkocjan
Copy link

gkocjan commented Jul 1, 2014

@caitp here is example: http://plnkr.co/edit/vLq79q4tZJn4PHDbN73i?p=preview
I notice that simple example, when every variable is created in controller, works (first input in example). Problem is, when variable is created by directive, in my case it was pagination from bootstrap-ui (second input in example). If type number greater than max input is still valid.

In second example after adding variable in controller it also starts to work. Just uncomment
//$scope.numPagesFromPagination = 0; in controller.

@caitp
Copy link
Contributor

caitp commented Jul 1, 2014

@gkocjan http://plnkr.co/edit/Vy6OYQH18FcjqEnOoVT9?p=preview

(setting the page to page 10 will invalidate the model)

(Also, the pagination directive doesn't use ng-model and instead depends on page, an isolate scope property)

@lhahne
Copy link

lhahne commented Jul 4, 2014

I can confirm this. If I set $scope.amount=5in controller, then max="{{amount}}" works as expected but when I set the amount some other way (e.g. from scope), angular seems to ignore the max attribute completely whereas the browser sees it.

@caitp
Copy link
Contributor

caitp commented Jul 4, 2014

if you can reproduce that, you need to show your issue, because as far as anyone can tell this is working. Can you make a demo, @lhahne?

@lhahne
Copy link

lhahne commented Jul 4, 2014

This was a concurrency issue at least for me. If max is null in the beginning and later gets updated, the browser will pick this up but angular will not.

@caitp
Copy link
Contributor

caitp commented Jul 4, 2014

Can you make a demo, @lhahne?

This means an actual reproduction using a recent release or snapshot build, so that we can determine if this is a bug to be fixed.

@lhahne
Copy link

lhahne commented Jul 4, 2014

Here you go http://plnkr.co/edit/EuuRHb9vUVw4ODhmsfjq?p=preview The first checkbox checks for max, the second does not. Not sure if this is actually even supposed to work in this case.

@caitp
Copy link
Contributor

caitp commented Jul 4, 2014

@lhahne I see, that's a fairly trivial fix.

The issue is the way we're testing if we need to register a min/max validator or not: https://github.com/angular/angular.js/blob/master/src/ng/directive/input.js#L1083

      if(attr.max) {

This evaluates to the empty string because the property is not set in scope yet, though the attribute is provided.

This is trivial to fix if you'd like to submit a pull request. If not I'll patch that later today

@Narretz
Copy link
Contributor

Narretz commented Jul 4, 2014

@caitp I think you even had a pull request with that fix back when max/min was still on $parsers: #6002

Here's an issue that would be closed by this fix: #6369 (comment)

@caitp
Copy link
Contributor

caitp commented Jul 4, 2014

nah, that PR was for a different issue, but this one is similarly trivial

@lhahne
Copy link

lhahne commented Jul 6, 2014

I was trying to fix this but I can't seem to get this to fail in unit tests. This is what I tried and I do not understand why it does not fail even though it is completely analogous to my previous example of the problem.

  it('should validate even if max value changes on-the-fly and is null in the beginning', function(done) {
    scope.max = null;
    compileInput('<input type="number" ng-model="value" name="alias" max="{{max}}" />');
    scope.max = 10;
    scope.$digest();

    changeInputValueTo('5');
    expect(inputElm).toBeValid();

    scope.max = 0;
    scope.$digest(function () {
      expect(inputElm).toBeInvalid();
      done();
    });
  });

@Narretz
Copy link
Contributor

Narretz commented Jul 6, 2014

Looking at the number validator, there are two problems here:
a) First, it doesn't set up the max/min validators if they are initially falsey
b) It doesn't fire validation when max/min change later

Theoretically, the number validations should be added to the $validators collection, but since number also uses formatters, and there is currently some discussion about the whole inout processing, it's probably best to defer this.

@lhahne Your test doesn't work, because when max is undefined or null before compilation, no max validator is set. When you change max afterwards, it doesn't fail because the max validator is not set, and even if it was, it would still fail because the directive doesn't listen to max={{value}} changes.

@idoo
Copy link
Contributor

idoo commented Aug 12, 2014

👍

@aderowbotham
Copy link

This is still unfixed right? Or does Angular by design only handle the max attribute as a fixed value?

Here's a simple example of the bug in Angular 1.2.22: http://plnkr.co/edit/lNemYyFrHF9gyRgY8KIT?p=preview

@btford btford removed the gh: issue label Aug 20, 2014
@pandamouse
Copy link

Same problem with input type='date' min max for 1.3.0-beta.19.
Plunkr modified from @lhahne 's

http://plnkr.co/edit/HOIsjV?p=preview

@IgorMinar
Copy link
Contributor

@matsko is this something we can address in RCs?

@matsko
Copy link
Contributor

matsko commented Aug 29, 2014

I have a PR that fixes this for date. The same logic applies here. And no it won't cause any breaking changes so we're safe to patch during RC.

@IgorMinar IgorMinar modified the milestones: Purgatory, 1.3.0-rc.1 Aug 29, 2014
matsko added a commit to matsko/angular.js that referenced this issue Sep 3, 2014
As of this fix if the max or min value is changed via scope or by another ngModel
then it will trigger the model containing the min/max attributes to revalidate itself.

Closes angular#2404
@matsko
Copy link
Contributor

matsko commented Sep 3, 2014

This is working on the current master, but if the min or max value is changed then the input field doesn't reconsider itself. This PR fixes the issue for input[type="number"]: #8913

matsko added a commit to matsko/angular.js that referenced this issue Sep 3, 2014
…nge for number inputs

As of this fix if the max or min value is changed via scope or by another ngModel
then it will trigger the model containing the min/max attributes to revalidate itself.

Closes angular#2404
matsko added a commit to matsko/angular.js that referenced this issue Sep 4, 2014
…nge for number inputs

As of this fix if the max or min value is changed via scope or by another ngModel
then it will trigger the model containing the min/max attributes to revalidate itself.

Closes angular#2404
@matsko matsko closed this as completed in 7b273a2 Sep 4, 2014
@adrianduke
Copy link

Still experiencing this issue on v1.3.10... http://plnkr.co/edit/uS4zsh05jeP7pF7A2aVk

Using an initialisation value that isn't null or "" resolves the issue for me e.g. $scope.max = "1"

@idoo
Copy link
Contributor

idoo commented Mar 19, 2015

@adrianduke try

ng-max='{{max}}'

http://plnkr.co/edit/DF4R6vfQpf909jj19sFv?p=preview

@gkalpak
Copy link
Member

gkalpak commented Mar 20, 2015

BTW, this seems to have been fixed in 1.3.14 (plnkr) and 1.4.0-beta.5 (plnkr).

@petebacondarwin
Copy link
Contributor

It would have expected this commit to fix it - b350283
But this was already there at 1.3.0

@petebacondarwin
Copy link
Contributor

Ah! Fixed here: abfce53

@piotrkulpinski
Copy link

@petebacondarwin WIll it be released in upcoming 1.3.16 release?

@petebacondarwin
Copy link
Contributor

@piotrkulpinski - this was already there in 1.3.14 and so will continue to appear in the next release too.

@stewones
Copy link

+1 here 1.4.x

@favio41
Copy link

favio41 commented Jun 6, 2016

Hi guys, in the documentation is missing the attr ng-max and ng-min for input type number.

@gkalpak
Copy link
Member

gkalpak commented Jun 6, 2016

Interesting! They are not documented as standalone directives either...
@favio41, would you like to submit a PR ?

@User195
Copy link

User195 commented Jan 25, 2019

just use ng-min and ng-max
It solved all my problems!

@andre719mv
Copy link

For for old Angular versions use fallback values before model is initialized:
max="{{max || 99999}}"
It worked for me.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.