-
Notifications
You must be signed in to change notification settings - Fork 27.4k
bug($ng): change numberInputType to check for leading decimals #5682
Conversation
add check for leading decimal. Some browsers swallow leading decimals on input type="number". fixes angular#5680
I'm sorry, but I wasn't able to verify your CLA signature. CLA signature is required for any code contributions to AngularJS. Please sign our CLA and ensure that the CLA signature email address and the email address in this PR's commits match. If you signed the CLA as a corporation, please let me know the company's name. Thanks a bunch! PS: If you signed the CLA in the past then most likely the email addresses don't match. Please sign the CLA again or update the email address in the commit of this PR. |
CLA Updated to match my GitHub email address |
CLA signature verified! Thank you! Someone from the team will now triage your PR and it will be processed based on the determined priority (doc updates and fixes with tests are prioritized over other changes). |
Hi, |
status of this? |
@@ -409,1064 +409,1070 @@ describe('ngModel', function() { | |||
}); | |||
|
|||
|
|||
describe('input', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks like your IDE screwed up indentation in this file, could you fix that? Should be two spaces.
@joeenzminger, can you explain what is meant by "swallow leading decimals" for me? This change only modifies values which are exactly one period, and I'm not sure I see why a single '.' shouldn't be NaN. Now, if this affects more legitimate values, the right fix might be a change to the regexp instead -- actually, looks like the regexp deals with leading periods already. I'm not sure this really fixes anything, so if you can provide info on the exact problem, I'll be happy to take another look |
it('should allow "." in input element', function () { | ||
compileInput('<input type="number" ng-model="price" />'); | ||
changeInputValueTo('.'); | ||
expect(inputElm.val()).toBe('.'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A '.' is not a real number, as far as I'm aware. It's also a badInput in modern browsers, which means the value would be the empty string if a user typed it.
@caitp I think he means it doesn't allow the first keyboard entry to be a |
@caitp - the fix attempt in the PR is not valid (for all browsers anyway). I think I'll pull the PR and submit another one later, but the bug is still valid. I'll also fix the indentation issues. While '.' is a badInput for type="number", without angular, browsers don't "swallow" it. They continue to display it (even while reporting value as 0 or "") in anticipation of the user continuing to add new characters (and thus making the input valid). With angular, on some browsers, you can't even type the '.', so it is impossible to enter a decimal number unless you first enter a leading 0, etc. This is not very intuitive for most users. I am also a little stuck on writing a test that validates the fix, since there really isn't a way to validate that affected browsers are displaying the "." with the fix applied (they still report 0 or "" as their value). |
Ah I see, you're right, FF26 is broken in this way. |
There's a SO question about this http://stackoverflow.com/questions/20985443/entering-number-with-leading-decimal-in-angular-input-type-number. And here's a jsFiddle demo of the issue http://jsfiddle.net/NEhG9/3/ |
Hi, |
02dc2aa
to
fd2d6c0
Compare
cad9560
to
f294244
Compare
abdaab7
to
30996f8
Compare
e8dc429
to
e83fab9
Compare
4dd5a20
to
998c61c
Compare
I'm not able to reproduce this anymore in any version of angular I've tried with, in FF33. I think we can close this? Please re-open if this is still an issue. |
Still facing this issue with angular 1.5.7 and FF 47.0.1. |
@credifiable, this issue has been closed. If you still see the problem, please open a new issue providing a live reproduction (e.g. using CodePen, Plnkr etc). |
why is this issue closed? it's clearly not dealt with... |
@jamesvanleuven, the issue is still open: #5680 |
add check for leading decimal. Some browsers swallow leading decimals on
input type="number".
fixes #5680