Fix whole number replacement when using fixedDecimalScale and prefix/suffix #228
Conversation
…cimalScale and prefix/suffix
src/number_format.js
Outdated
| @@ -595,8 +595,8 @@ class NumberFormat extends React.Component { | |||
| value.length > lastValue.length | |||
| || !value.length || | |||
| start === end || | |||
| (start === 0 && end === lastValue.length) || | |||
| (selectionStart === 0 && selectionEnd === lastValue.length) | |||
| (start <= prefix.length && end >= lastValue.length - suffix.length) || | |||
There was a problem hiding this comment.
- ^update the comment
- create two variables, and check for weather format pattern is not defined.
const leftBound = !!format ? 0 : prefix.length;
const rightBound = lastValue.length - (!!format ? 0 : suffix.length);There was a problem hiding this comment.
Also the condition start <= prefix.length && end >= lastValue.length - suffix.length will return half formatted value when multiple character is used for prefix/ suffix. For example if you used $$ as prefix.
and the value is $$3.00. And if you replace $3.00 with character 4. This method will return $4 which this method is trying to avoid. This might cause some other bug.
There was a problem hiding this comment.
The point about partially selected prefixes/suffixes is very valid. In my manual tests the overall behaviour in such a case still seemed to be fine, but since I do not know the whole context here I am now suggesting a more defensive approach.
Only adding one additional condition for returning early from correctInputValue(), which is if exactly the number without prefix/suffix is selected/changed. If parts of prefix/suffix are part of the selection, this results in no change to the value.
| @@ -591,12 +591,16 @@ class NumberFormat extends React.Component { | |||
| or if value is empty string (when whole input is cleared) | |||
| or whole input is replace with a number | |||
| */ | |||
| const leftBound = !!format ? 0 : prefix.length; | |||
| const rightBound = lastValue.length - (!!format ? 0 : suffix.length); | |||
| if ( | |||
| value.length > lastValue.length | |||
| || !value.length || | |||
| start === end || | |||
| (start === 0 && end === lastValue.length) || | |||
There was a problem hiding this comment.
logically when ever start === 0 && end === lastValue.length is true selectionStart === 0 && selectionEnd === lastValue.length will always be true (in case of deletion, any addition case is already handled on value.length > lastValue.length). Same for leftBound and rightBound condition.
I will remove the extra condition.
There was a problem hiding this comment.
Please refer this for understanding more about these conditions and why we need both
- For selection condition - PR: Fix selection value issue #516, Issue: [4.5.2] With prefix, typed numbers appear with incorrect order #515
- For start and end condition - PR: Fix full value replacement issue when allowEmptyString is set to true #511, Issue: Safari iOS autofill with react-number-format allowEmptyFormatting #450
|
Rebased and merged with master. |
|
Awesome! Thanks for merging! |
wolfib
deleted the
bugfix-whole-number-replacement-with-fixedDecimalScale
branch
wolfib
restored the
bugfix-whole-number-replacement-with-fixedDecimalScale
branch
Double clicking on a number selects the whole number, but not any prefix/suffix. Previously, when making an edit after such a double click, the first digit entered would not lead to any change, if the NumberFormat component uses fixedDecimalScale and prefix/suffix.
That's because replacing the comma separator is not allowed, except when replacing the whole number. But since the prefix/suffix is not selected, this "replacing the whole number" behaviour was not triggered.
This commit fixes the behaviour by taking prefixes and suffixes into account.
See issue #159