Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Percent filter should not apply multiplications (or always apply it) - negative numbers break. #86

Closed
williamjulianvicary opened this issue Apr 3, 2020 · 9 comments

Comments

@williamjulianvicary
Copy link

When dealing with numbers less than and greater than 1 (i.e 120% and -20%) the multiplication logic will not function well.

With these two inputs:
1.2 = 1.2% (incorrect)
-0.20 = -200% (correct)

I think removal of the logic that adds/excludes the multiplication (one way or the other) is in order to resolve.

@mcbarros
Copy link

mcbarros commented Apr 3, 2020

agree, maybe a option to disable multiplications?

@williamjulianvicary
Copy link
Author

Sounds sensible to me!

@freearhey
Copy link
Owner

Could you please clarify for me: what you are referring to by saying that 1.2 => 1.2% incorrect result? And why -0.20 = -200% is correct?

I just don't understand what the rules apply in this case. Maybe you can give me a reference to some standard.

@williamjulianvicary
Copy link
Author

With an input of 1.2 and -0.2 I would expect to get a response of 120% and -20% respectively (or non-multiplied numbers). This is because the logic checks if the number is greater than 1 and if not, it multiplies by 100 but if it is it leaves it as-is.

I don’t think there is any standard here, the multiplication should be either applied or not (via a config setting, it shouldn’t try to guess based on the input).

@williamjulianvicary
Copy link
Author

It is this line that I’m referring to here:

if(value <= 1) {

@freearhey
Copy link
Owner

I understand what kind of operation we're talking about, but I don't understand what your claim is based on that it's the right way to convert numbers into percentages.

@mcbarros
Copy link

mcbarros commented Apr 24, 2020

It's about consistence, 0.0001% is valid as is, the library should't multiply this value unless told to do so. Imagine a world where I have a table and after some calculations, I need to display as percentage the following values:
102 -> 102%
99.9 -> 99.9%
15.7 -> 15.7%
1.8 -> 1.8%
0.9 -> 90% (wrong)

Now imagine that to fix this, I will try to divide all numbers by 100:
1.02 -> 1.02% (wrong)
0.999 -> 99.9%
0.157 -> 15.7%
0.018 -> 1.8%
0.009 -> 0.9%

@freearhey
Copy link
Owner

@mcbarros Greate example! Perhaps the filter behavior should really be changed.

I'll try to publish the updated version soon.

@freearhey
Copy link
Owner

Done.

Now all numbers are multiplied by 100 by default, but you can change the multiplier through the filter options if necessary.

More details: https://github.com/freearhey/vue2-filters#percent

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

No branches or pull requests

3 participants