mantine icon indicating copy to clipboard operation
mantine copied to clipboard

NumberInput handles non-numeric values differently on initial render

Open mattrunyon opened this issue 3 years ago • 2 comments

What package has an issue

@mantine/core

Describe the bug

When NumberInput is set to a non-numeric value without a default initially, it defaults to undefined as seen here

When NumberInput in a controlled state is set to a non-numeric value after the initial render, it ignores the value unless it is explicitly undefined as seen here

This seems like it would be related to #1495

While this can trigger some errors w/ TS, I think the logic should be consistent between initial value and controlled changes to the value

In which browser did the problem occur

Chrome

If possible, please include a link to a codesandbox with the reproduced problem

https://codesandbox.io/s/gallant-euclid-exck63?file=/src/App.tsx

Do you know how to fix the issue

Yes

Are you willing to participate in fixing this issue and create a pull request with the fix

Yes

Possible fix

The useEffect updating the NumberInput value when the value prop changes should use the same logic as the initial state and set the internal state to undefined if a non-numeric value is given to the component

mattrunyon avatar Jun 07 '22 05:06 mattrunyon

Empty string maybe fine and can help with form resetting, but I do not think that null should be supported

rtivital avatar Jun 07 '22 07:06 rtivital

Null is currently an allowed initial state (results in undefined for internal value), but not state after initial render

These issues are also TS errors, so it is partially a user error if they only use JS. I didn't check how it interacts with the form input props spread onto the component

mattrunyon avatar Jun 07 '22 07:06 mattrunyon

I've decided that we should only support number and undefined in NumberInput, these cases work as expected – https://codesandbox.io/s/cool-cherry-0415m1?file=/src/App.tsx

rtivital avatar Aug 12 '22 06:08 rtivital