feat(input type=number): min/max attributes are evaluated

[witoldsz]

#1068

Before the change, one could specify min="10" or max="50". Now it is possible to use expressions, like min="{{expression}}".

[mkmik]

Tried out, it works! Also drier code imho.

+1

[mkmik]

Hmm, actually I noticed that it has a bug:

The previous value of the model is ignored. I use this quick workaround but I have no idea if it makes sense as a fix.

diff --git a/app/lib/angular/angular.js b/app/lib/angular/angular.js
index ef15023..44e7dd4 100644
--- a/app/lib/angular/angular.js
+++ b/app/lib/angular/angular.js
@@ -11225,7 +11225,8 @@ function numberInputType(scope, element, attr, ctrl, $sniffer, $browser) {

       attr.$observe(minOrMax, function() {
         // triggering validation
-        ctrl.$setViewValue(ctrl.$viewValue);
+        if (!isNaN(ctrl.$viewValue))
+          ctrl.$setViewValue(ctrl.$viewValue);
       });
     }
   }

[chicoxyzzy]

usefull pull request

+1

[pkozlowski-opensource]

@witoldsz Are you planning to work on this PR to accommodate @mmikulicic input? I'm asking since @florianorben opened a new PR based on yours and I wonder if you guys synced on this topic?

[witoldsz]

@pkozlowski-opensource I did not check the issue reported by @mmikulicic. Too bad @mmikulicic did not provide failing test case, so it would be easier to validate and fix.
@florianorben did write his pull request is based on this one, but did not clarify what was changed.

[pkozlowski-opensource]

@witoldsz @florianorben It would be great if you guys could join forces and come up with one PR that add expression support for both min, max and ng-min, ng-max directives. My understanding is that @florianorben wanted to support the ng- versions of those directives.

@witoldsz @florianorben it looks like it would be nice addition but now things a bit confusing with 2 PRs addressing the same functionality.

[florianorben]

Hey there,

basically I didn't really change anything regarding the functionality.
The only difference is that I pass the ngMinlength/ngMaxlength attributes and the input.value's length (in contrast to the actual input value) to your applyMinMax() function (+ also invalidating minlength/maxlength (instead of min/max) formController.error values + some minor var renames to reflect the use case).

I guess the two PRs could/should be merged together, since your function works for both scenarios (and we ideally don't want duplicated code).

Sadly, I could not reproduce @mmikulicic bug sofar.

Edit: @pkozlowski-opensource My pull request actually handled ng-minlength (/ng-maxlength) directives - which provide a different functionality than min/max ( =min/max value.length vs. min/max actual value) - otherwise agreed.

@witoldsz Any suggestions about proceeding - dou you want to apply the "ng-min/maxllength scenario" to your PR?

[jbdeboer]

@witoldsz I took a look at this PR. There are a few test failures that need to be resolved before it can be merged: http://ci.angularjs.org/job/angular.js-james/6/

I don't think they would be fixed by @mmikulicic's patch. Marko, do you have a reproducible test case for your bug?

[jbdeboer]

Closing this PR for now to keep the queue clean. Please re-open when the tests are passing.