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

Thousand and decimal formatting broken while typing when valueIsNumericString is set to false with react-final-form #741

Closed
Nico924 opened this issue Mar 6, 2023 · 4 comments · Fixed by #756
Labels

Comments

@Nico924
Copy link

Nico924 commented Mar 6, 2023

Describe the issue and the actual behavior

When using react-number-format with react-final-form where I need to store the floatValue, if I set the parameter valueIsNumericString to false even if I provide a numeric value to NumericFormat, the formatting of the input text is not done while typing.
If I remove valueIsNumericString or if I set it to true (even with numeric value) or undefined, it works.

Describe the expected behavior

I would like the decimal and thousand formatting to work while typing.

Provide a CodeSandbox link illustrating the issue

https://codesandbox.io/embed/react-final-form-w-react-number-format-forked-7byc8q?fontsize=14&hidenavigation=1&theme=dark

Provide steps to reproduce this issue

Start writing anything in the input (adding 0 for example), no formatting will occur

@TheSharpieOne
Copy link

TheSharpieOne commented Apr 17, 2023

Running into this as well, but without react-final-form. From what I can tell, internally the value is always a string, and setting valueIsNumericString to false makes it not perform the string formatting (in fact, it makes it remove the formatting.)

Here is the code that is doing that:

if (isNil(value) || isNanValue(value)) {
numAsString = '';
formattedValue = '';
} else if (typeof value === 'number' || valueIsNumericString) {
numAsString = typeof value === 'number' ? toNumericString(value) : value;
formattedValue = format(numAsString);
} else {
numAsString = removeFormatting(value, undefined);
formattedValue = value;
}

Now you may be thinking typeof value === 'number' would be true, we are supplying a number. But it is always a string at this point (when getting the input's value to be displayed). It uses roundIncomingValueToPrecision as the value when passing it to useInternalValues and returns formattedValue which isn't formatted in this case.
const [{ numAsString, formattedValue }, _onValueChange] = useInternalValues(
roundIncomingValueToPrecision(value),
roundIncomingValueToPrecision(defaultValue),
Boolean(_valueIsNumericString),
_format,
_removeFormatting,
onValueChange,
);

Here is roundIncomingValueToPrecision and you can see if the value is a number, it will make it a string
const roundIncomingValueToPrecision = (value: string | number | null | undefined) => {
if (isNil(value) || isNanValue(value)) return value;
if (typeof value === 'number') {
value = toNumericString(value);
}
/**
* only round numeric or float string values coming through props,
* we don't need to do it for onChange events, as we want to prevent typing there
*/
if (_valueIsNumericString && typeof decimalScale === 'number') {
return roundToPrecision(value, decimalScale, Boolean(fixedDecimalScale));
}
return value;
};

Here is that formattedValue being returned as value. This returned value is later passed to NumberFormatBase
return {
...(restProps as NumberFormatBaseProps<BaseType>),
value: formattedValue,
valueIsNumericString: false,
isValidInputCharacter,
onValueChange: _onValueChange,
format: _format,
removeFormatting: _removeFormatting,
getCaretBoundary: (formattedValue: string) => getCaretBoundary(formattedValue, props),
onKeyDown: _onKeyDown,
onBlur: _onBlur,
};

export default function NumericFormat<BaseType = InputAttributes>(
props: NumericFormatProps<BaseType>,
) {
const numericFormatProps = useNumericFormat(props);
return <NumberFormatBase {...numericFormatProps} />;
}

NumberFormatBase actually runs useInternalValues again. This time is it with the value from that previous run that is already a string at this point.

Here is a sandbox without final form:
https://codesandbox.io/s/float-value-demo-qrw2l7

Assuming what the above is correct, setting this to always be true would fix the issue since it is always passing a string to the function:

Boolean(_valueIsNumericString),

OR after rounding, convert it back to a number.

The workaround (as mentioned in the original post) is to just set valueIsNumericString to true. This works as long as you are not passing a pre-formatted string.

@s-yadav s-yadav added the bug label May 7, 2023
@s-yadav
Copy link
Owner

s-yadav commented May 7, 2023

@TheSharpieOne thanks for digging on this.
Yes, looks like a bug. I was doing some refactoring around guessing if a value is a numeric string based on the other formatting prop and value. Will fix it along with that. Will be available on the next release.

s-yadav added a commit that referenced this issue May 10, 2023
@jcrawl3y
Copy link

@s-yadav Confirming that this bug is occurring to me too. Do you have a rough estimate on when this fix will be available?

@s-yadav
Copy link
Owner

s-yadav commented May 14, 2023

This is fixed on v5.2.0

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

Successfully merging a pull request may close this issue.

4 participants