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

ngPluralize example has problem when a value >3 is deleted #10836

Closed
mrsoto opened this issue Jan 22, 2015 · 9 comments
Closed

ngPluralize example has problem when a value >3 is deleted #10836

mrsoto opened this issue Jan 22, 2015 · 9 comments

Comments

@mrsoto
Copy link

mrsoto commented Jan 22, 2015

In this page
https://code.angularjs.org/1.3.10/docs/api/ng/directive/ngPluralize

Change the number of peoples to 3 or more and then clear the field.

typing 3 and cleaning get -2 other peoples...

@caitp
Copy link
Contributor

caitp commented Jan 22, 2015

yeah, that's kind of weird --- @petebacondarwin do you think ngPluralize should support signed values by default? or maybe there's a better way

@gkalpak
Copy link
Member

gkalpak commented Jan 22, 2015

FWIW, I can't reproduce on Chrome/Firefox on Windows, but it sounds reasonable to (properly) handle negative values as 0.
On what browsers/platforms do you see this ?

@caitp
Copy link
Contributor

caitp commented Jan 22, 2015

you can see it in chrome and safari on a mac, i expect you can see it on windows too

@mrsoto
Copy link
Author

mrsoto commented Jan 22, 2015

I've the problem in Windows Chrome
El 22/01/2015 16:01, "⭐caitp⭐" notifications@github.com escribió:

you can see it in chrome and safari on a mac, i expect you can see it on
windows too


Reply to this email directly or view it on GitHub
#10836 (comment)
.

@mrsoto
Copy link
Author

mrsoto commented Jan 22, 2015

This example start assuming that blank is zero and this assumption remain
for 1 and 2 after the field is cleared but if you type 3 or more and then
clear the field got negative values.
El 22/01/2015 15:37, "⭐caitp⭐" notifications@github.com escribió:

yeah, that's kind of weird --- @petebacondarwin do you think ngPluralize
should support signed values by default? or maybe there's a better way


Reply to this email directly or view it on GitHub.

@gkalpak
Copy link
Member

gkalpak commented Jan 22, 2015

I can reproduce it with 4 or more but not 3. Weird...

@gkalpak
Copy link
Member

gkalpak commented Jan 22, 2015

I think the problem is that lastCount was assumed to always be numeric (thus assuming that isNaN(lastCount)===true means lastCount -> NaN (which isn't true)).

gkalpak added a commit to gkalpak/angular.js that referenced this issue Jan 22, 2015
When `lastCount` was evaluated to an non-numeric value (e.g. "other") and
`count` was evaluated to `NaN` (e.g. `null`/`undefined`), the text content
would be (wrongly) based on the previous template.
This commits makes sure the text content is updated correctly.

In order to customize the message shown upon `null`/`undefined` one can
specify a `'NaN'` property on the `when` exression object.

Closes angular#10836
gkalpak added a commit to gkalpak/angular.js that referenced this issue Jan 23, 2015
When `lastCount` was evaluated to an non-numeric value (e.g. "other") and
`count` was evaluated to `NaN` (e.g. `null`/`undefined`), the text content
would be (wrongly) based on the previous template.
This commits makes sure the text content is updated correctly.

In order to customize the message shown upon `null`/`undefined` one can
specify a `'NaN'` property on the `when` exression object.

Closes angular#10836
gkalpak added a commit to gkalpak/angular.js that referenced this issue Jan 23, 2015
When `lastCount` was evaluated to an non-numeric value (e.g. "other") and
`count` was evaluated to `NaN` (e.g. `null`/`undefined`), the text content
would be (wrongly) based on the previous template.
This commits makes sure the text content is updated correctly.

In order to customize the message shown upon `null`/`undefined` one can
specify a `'NaN'` property on the `when` exression object.

Closes angular#10836
@Narretz Narretz added this to the Backlog milestone Jan 23, 2015
@Narretz
Copy link
Contributor

Narretz commented Jan 23, 2015

✨ Don't forget to label / milestone everyone! ✨

@petebacondarwin
Copy link
Contributor

+1 Narretz!

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