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

Add option to hide negligible decimal digits in display #203

Closed
JPustkuchen opened this issue Mar 16, 2021 · 11 comments
Closed

Add option to hide negligible decimal digits in display #203

JPustkuchen opened this issue Mar 16, 2021 · 11 comments
Labels
feature New feature or request

Comments

@JPustkuchen
Copy link

JPustkuchen commented Mar 16, 2021

Hi and thank you so much for this wonderful library, it saved my day after a lot of headaches ;)

One wonderful improvement (feature request) for the library would be an option to remove zero fraction digits in display.

For example:

  • 10,00 => 10
  • 10,01 => 10,01
  • 10,20 => 10,20

Bonus:

  • 10,00 => 10,-

Or in code:

return valueFloat.toLocaleString('de-DE', {
  maximumFractionDigits: 2,
  minimumFractionDigits: (valueFloat % 1 != 0 ? 2 : 0),
});

So the condition is: If there are relevant digits ( mod 1 != 0 ), keep precision. If there are only relevant digits, remove them.
This is a little feature request for cases where (due to available space) the numer should be as short as possible.

I guess this should definitely be an optional option.
Additionally, you could even think about instead of removing them to replace them with a certain string (like '-' for typical 10,- €)

If you like the idea, we might discuss a good name and description.

@JPustkuchen JPustkuchen changed the title Add option to remove zero fraction digits in display Add option to hide negligible decimal digits in display Mar 16, 2021
@JPustkuchen
Copy link
Author

Just saw that "Hide negligible decimal digits" is similar functionality but only for input. Of course this should be separate options.

@dm4t2
Copy link
Owner

dm4t2 commented Mar 16, 2021

Hi, I'm glad you like my plugin. Your issue seems to be a duplicate of #186, isn't it?

@dm4t2 dm4t2 added the duplicate This issue or pull request already exists label Mar 16, 2021
@dm4t2 dm4t2 closed this as completed Mar 16, 2021
@JPustkuchen
Copy link
Author

Hi @dm4t2,

thank you for your quick reply!
Well, partially! :) You're right that there is a way to set min and max, but especially for currencies (in contrast to regular number formatting) I think the typical way is "all or nothing" for the decimal digits. So setting min and max is helpful, but not the same, as the decision to display both (if mod 1 != 0) or none isn't possible that way.

So it would be cool if we could keep on discussing that feature as an addition and perhaps you could reopen this issue as feature request?

If it's not clear enough what I'm talking about and what's the benefit and use-case, tell me and I'll give my best to explain it more in detail.

Thanks a lot! :)

@dm4t2
Copy link
Owner

dm4t2 commented Mar 16, 2021

Sounds good to me. I would introduce a new boolean option hideNegligibleDecimalDigits to handle this behavior.

@dm4t2 dm4t2 reopened this Mar 16, 2021
@dm4t2 dm4t2 added feature New feature or request and removed duplicate This issue or pull request already exists labels Mar 16, 2021
@JPustkuchen
Copy link
Author

JPustkuchen commented Mar 17, 2021

Happy to hear that! :)

I'm still thinking about two things, but am not sure yet, what's the perfect fit:

  1. Should hideNegligibleDecimalDigits be added into precision?
    { precision: { min: 0, max: 2 } }
    We should discuss how they behave together for some examples to see what's the influence on each other and if it's better to have the options separated or together.

  2. Should hideNegligibleDecimalDigits allow for a user defined string additional to true and false to use as replacement?
    Currently, I can only see benefits in that option.

But I'm not sure yet...

@dm4t2
Copy link
Owner

dm4t2 commented Mar 17, 2021

Also thought about it and I'm not really satisfied with hideNegligibleDecimalDigits because its name clashes with { distractionFree: { hideNegligibleDecimalDigits } } and the behavior is still different, for example:

10,20 => 10,20 vs. 10,20 => 10,2 (on focus)

My new proposal looks like this:

  1. precision accepts the new string value auto to handle your described "all or nothing" behavior:

    • 10,00 => 10
    • 10,01 => 10,01
    • 10,20 => 10,20
  2. Introducing a new option emptyFractionReplacement that accepts a string to customize the formatting of integer numbers, for example { emptyFractionReplacement: '-' } => "10,-". This would only be considered when using a precision configuration that allows the fraction to be empty:

    • { precision: 0 }
    • { precision: { min: 0 } }
    • { precision: 'auto' }

What do you think?

@JPustkuchen
Copy link
Author

JPustkuchen commented Mar 18, 2021

Yes I understand the naming clash. so if we can't align the behaviour, we should use a different naming. But I think we'll find a matching name in the end.

Re proposal:
How would precision: 'auto' know the maximum precision? After thinking about this for a while I think perhaps
{ precision:: {min: 'auto', [max: 'XX'] } is the right approach? So the decimals would first be trimmed to max and afterwards min decides if they are filled (by value or undefined = from currency), kept or removed. The 'all or nothing' logically never affects max, right? It even needs to know max?
What do you think? Is this correct?

Re emptyFractionReplacement:
I think that could play well with a different name for auto in min, like clear:
I guess we only want to use that replacement string in auto mode, right? In all other cases (like min: 0) we would use min and max with integer or undefined values, but never replace the fraction digits with the string, right?

So perhaps we should simply set emptyFractionReplacement to an empty string by default which means that negligible decimal digits are replaced by that (empty) string if auto detects to clear. So we need no further "special" behaviour.

That leads me to the idea that a combined naming could be better than auto and emptyFractionReplacement. I'm also not sure about the name yet, but perhaps something like this:

{
  precision: {
    min: 'clear',
    clearString: '' // default: '' other values for example empty: '-' or empty: '--'
  }
}

I'm also okay with auto, but I guess clear naming is a bit more expressive in this combination? Perhaps we should ask others if they understand what it does after we have a final result here in discussion.

Nice discussion btw 😄 Makes me quite happy to see the good results!

@dm4t2
Copy link
Owner

dm4t2 commented Mar 18, 2021

I think precision: 'auto' should be an easy-to-use enhancement of the default precision specified by the configured currency. In most cases, it is probably rather uncommon that a higher precision is used. { precision: { min: 'auto', max: 2 } } might also be an interesting option, but brings additional complexity. So I would start with the simple shorthand.

Regarding the emptyFractionReplacement option, there's one big issue when using distractionFree: false and entering a number. The emptyFractionReplacement must not remain in the formatted number in this case. Imagine the value "€ 1.234,-" and then trying to input decimals. How would this work?

  • Disable the emptyFractionReplacement option when using distractionFree: false?
  • distractionFree: { hideNegligibleDecimalDigits: false } is ignored?
  • Replace the emptyFractionReplacement with "00" on focus?

@JPustkuchen
Copy link
Author

JPustkuchen commented Mar 19, 2021

Hi @dm4t2,

thank you! I'm absolutely fine with all the points :)

precision: 'auto' is also ok and a good shortcut. As written above I just see { precision: { min: 'auto', max: 2 } } or { precision: { min: 'clear', max: 2 } } as the longhand better logically fitting solution.
As long as we don't introduce something that makes problems later and can't be changed anymore due to backwards compatibility, it's allright 👍

Re emptyFractionReplacement I'd definitely tend to

Replace the emptyFractionReplacement with "00" on focus?

Warning off-topic starts here ;)
I guess in the end many options might anyway end up to be optionally split into "display" and "input" for maximum flexibility and simple configuration logic, so that idea fits well.
What I want to say with that point is that I could imagine that in a far far future major release it could simplify configuration to be able to split similar settings for display and input like

{
  currency: 'EUR'
}

for both display and input, which can simply be overridden by

{
  display: {
    currency: 'EUR'
  },
  input: {
    currency: null
  }
}

instead of special distractionFree settings. Or perhaps directly define it that split way or whatever... 😄 of course there are settings that are always global. Trying to think about the "real" nature of such settings and how to be most self-explaining while preventing logical problems.
But that's NOT our topic here, the discussion simply led me there. Perhaps it's an overengineered idea, and finally you're the owner and maintainer, so it's not my choice. Just an idea, perhaps good, perhaps bad. 😃 But definitely for the far far future ;)
Off-topic ends here ;)

Your library is great, and I don't want to criticize or pretend I knew better it in any way. I don't! 😛 Please don't get me wrong. Just wrote down what went through my head here. 😄

So finally: Pick from our discussion what you think is best and I'm very very very happy to see that your library will provide a solution for the discussed challenge.

@dm4t2
Copy link
Owner

dm4t2 commented Mar 20, 2021

Thank you very much for contributing and the discussion. Let's see what I make of it in the next v2 release candidate 😉

dm4t2 added a commit that referenced this issue Mar 26, 2021
@dm4t2
Copy link
Owner

dm4t2 commented Mar 26, 2021

The changes are now available in 2.0.0-rc.3 🎉

The "all or nothing" precision behavior is now the new default, so there is no need for precision: 'auto' anymore. This solution feels more appropriate since a fixed precision for integers can still easily configured if desired (for example { precision: 2 })

And finally, there's also a new option decimalDigitsReplacement (former named emptyFractionReplacement).

You can test it on the playground:
https://vue-currency-input-next.netlify.app/playground/

@dm4t2 dm4t2 closed this as completed Mar 26, 2021
dm4t2 added a commit that referenced this issue Apr 2, 2021
BREAKING CHANGE:
the use case of a precision range is longer given because of the new precision default behavior (hide decimal digits for integer numbers)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants