Add `allowedDecimalSeparators` prop #345

macobo · 2019-09-24T10:20:26Z

When defined, this prop defines which keys to accept as decimal
separators. This is useful in cases where users might want to enter both
comma and dot as separators

Related issues: #324, #133, #223, #261

When defined, this prop defines which keys to accept as decimal separators. This is useful in cases where users might want to enter both comma and dot as separators Related issues: s-yadav#324, s-yadav#133, s-yadav#223, s-yadav#261

macobo · 2019-09-24T10:22:01Z

src/number_format.js

-    /** Check if only . is added in the numeric format and replace it with decimal separator */
-    if (!format && decimalSeparator !== '.' && start === end && value[selectionStart] === '.') {
+    /** Check for any allowed decimal separator is added in the numeric format and replace it with decimal separator */
+    if (!format && start === end && allowedDecimalSeparators.includes(value[selectionStart])) {


Note: This might not work in all browsers. Should I extract a util instead?

You can use .indexOf instead.

macobo · 2019-09-27T14:06:26Z

Hi @s-yadav, mind taking a look? Happy to iterate on this as/if needed.

s-yadav

Thanks @macobo for putting this. The PR looks good with this two small changes.

s-yadav · 2019-09-29T05:02:03Z

src/number_format.js

-    /** Check if only . is added in the numeric format and replace it with decimal separator */
-    if (!format && decimalSeparator !== '.' && start === end && value[selectionStart] === '.') {
+    /** Check for any allowed decimal separator is added in the numeric format and replace it with decimal separator */
+    if (!format && start === end && allowedDecimalSeparators.includes(value[selectionStart])) {


You can use .indexOf instead.

s-yadav · 2019-09-29T05:07:18Z

src/number_format.js


    if (thousandSeparator === true) {
      thousandSeparator = ','
    }
+    if (!allowedDecimalSeparators) {
+      allowedDecimalSeparators = [decimalSeparator]
+      if (decimalSeparator !== '.') {


No need for this if condition. You can just define

allowedDecimalSeparators = ['.', decimalSeparator]

It's okay if dot gets duplicate as we are using this array just for checking includes.

macobo · 2019-09-29T08:31:53Z

Thanks for the feedback @s-yadav. I've updated the PR.

s-yadav · 2019-09-29T08:39:45Z

Can you also update the readme for the new prop

macobo · 2019-09-29T09:01:28Z

Also done @s-yadav.

s-yadav · 2019-09-29T09:35:25Z

I see that you have updated the example file, we will need it to update on README file.

macobo · 2019-09-29T09:38:43Z

Oops - sorry, tried to push to the wrong remote thus the PR didn't get updated. Done.

s-yadav · 2019-09-29T11:14:15Z

Merged. Right now I am traveling. Will test it out and release it on npm tomorrow.

macobo · 2019-09-29T13:25:52Z

Thank you!

…

On Sun, Sep 29, 2019 at 2:14 PM Sudhanshu Yadav ***@***.***> wrote: Merged. Right now I am traveling. Will test it out and release it on npm tomorrow. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#345?email_source=notifications&email_token=AABEKVC24R2GFICAALHLBHTQMCEYRA5CNFSM4IZ5WP7KYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD73REAA#issuecomment-536285696>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AABEKVCKCFG4BU5GUCPMTNDQMCEYRANCNFSM4IZ5WP7A> .

tenkij · 2019-10-01T20:16:39Z

Thank you!
Little problems:
https://codesandbox.io/s/react-number-format-demo-9y7j7

If you paste a number '11,11', then the formatted number will be '1 111'. If you enter a number '11,11', then the formatted number will be '11.11'.
Press the “11, 11” button. The input value will be “11, 11”. Then click on the input. After losing focus, the input value will be “1 111”.

macobo · 2019-10-01T20:25:09Z

Hi @tenkij it's worth creating a new issue for these rather than on a merged PR. The pasting issue seems to exist before this PR at least.

s-yadav · 2019-10-01T20:41:06Z

@tenkij the allowedDecimalSeparators is to only allow user to type a character which will be converted to passed decimalSeparator prop. When you paste or change a state value, the incorrect characters are removed.
But when you pass isNumericString true, it is an indication to NumberFormat that the passed value prop is a valid numeric string (without formatting) do not try to process it.

s-yadav · 2019-10-01T20:43:22Z

But there seem to be some inconsistencies on paste, which I will look into.

tenkij · 2019-10-01T20:58:25Z

@macobo, ok.
Problem only if the following conditions are true:

allowedDecimalSeparators={[",", "."]}
decimalSeparator = "."
paste '11,11'

paste '11.11' - ok.

Add allowedDecimalSeparators prop

6685771

When defined, this prop defines which keys to accept as decimal separators. This is useful in cases where users might want to enter both comma and dot as separators Related issues: s-yadav#324, s-yadav#133, s-yadav#223, s-yadav#261

macobo commented Sep 24, 2019

View reviewed changes

s-yadav reviewed Sep 29, 2019

View reviewed changes

Solve PR feedback

56374a0

Add prop to readme

916bda8

s-yadav approved these changes Sep 29, 2019

View reviewed changes

s-yadav merged commit 3fe3806 into s-yadav:master Sep 29, 2019

s-yadav added a commit that referenced this pull request Oct 1, 2019

Release for #345 - Add allowedDecimalSeparators prop

1f2df39

macobo mentioned this pull request Oct 1, 2019

Add missing typescript typings #348

Merged

macobo deleted the add-allowed-decimal-separators-prop branch October 1, 2019 20:25

s-yadav mentioned this pull request Oct 3, 2019

Comma character could be taken as period(default decimalSeparator) #324

Closed

nikhil-varma mentioned this pull request Jun 29, 2021

add support for paste event given allowedDecimalSeparators #556

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `allowedDecimalSeparators` prop #345

Add `allowedDecimalSeparators` prop #345

macobo commented Sep 24, 2019

macobo Sep 24, 2019

s-yadav Sep 29, 2019

macobo commented Sep 27, 2019

s-yadav left a comment

s-yadav Sep 29, 2019

s-yadav Sep 29, 2019

macobo commented Sep 29, 2019

s-yadav commented Sep 29, 2019 •

edited

Loading

macobo commented Sep 29, 2019

s-yadav commented Sep 29, 2019

macobo commented Sep 29, 2019

s-yadav commented Sep 29, 2019

macobo commented Sep 29, 2019 via email

tenkij commented Oct 1, 2019 •

edited

Loading

macobo commented Oct 1, 2019

s-yadav commented Oct 1, 2019

s-yadav commented Oct 1, 2019

tenkij commented Oct 1, 2019

Add allowedDecimalSeparators prop #345

Add allowedDecimalSeparators prop #345

Conversation

macobo commented Sep 24, 2019

macobo Sep 24, 2019

Choose a reason for hiding this comment

s-yadav Sep 29, 2019

Choose a reason for hiding this comment

macobo commented Sep 27, 2019

s-yadav left a comment

Choose a reason for hiding this comment

s-yadav Sep 29, 2019

Choose a reason for hiding this comment

s-yadav Sep 29, 2019

Choose a reason for hiding this comment

macobo commented Sep 29, 2019

s-yadav commented Sep 29, 2019 • edited Loading

macobo commented Sep 29, 2019

s-yadav commented Sep 29, 2019

macobo commented Sep 29, 2019

s-yadav commented Sep 29, 2019

macobo commented Sep 29, 2019 via email

tenkij commented Oct 1, 2019 • edited Loading

macobo commented Oct 1, 2019

s-yadav commented Oct 1, 2019

s-yadav commented Oct 1, 2019

tenkij commented Oct 1, 2019

Add `allowedDecimalSeparators` prop #345

Add `allowedDecimalSeparators` prop #345

s-yadav commented Sep 29, 2019 •

edited

Loading

tenkij commented Oct 1, 2019 •

edited

Loading