react-spectrum icon indicating copy to clipboard operation
react-spectrum copied to clipboard

[NumberField] Setting null to value does not reset to blank state

Open fhschan opened this issue 4 years ago โ€ข 6 comments

๐Ÿ› Bug Report

When using the NumberField component, if the implementor wants to use a controlled state to control the value of NumberField, passing in null does not reset this component to the initial state.

๐Ÿค” Expected Behavior

  • Passing null should reset to the empty state for this component

๐Ÿ˜ฏ Current Behavior

  • Passing null sets the value to 0.

๐Ÿ’ Possible Solution

Update https://github.com/adobe/react-spectrum/blob/main/packages/@react-stately/numberfield/src/useNumberFieldState.ts#L91 so it retains the empty state if the component is used in a controlled state.

๐Ÿ”ฆ Context

  • If we have external logic using react-hook-form to control this component, we want to be able to reset the NumberField component to its initial case. A use case for example, for form validation. If user resets the form and we have a parent Form component that holds all the values of the form components, we want to reset it to the original empty value.
  • Currently, by passing in null to value prop would result in 0, which can be a valid value and does not tell the implementor that user has not filled this form from looking at the value. If we pass in undefined, it looks like the NumberField retains the previous value. (eg. user enters 5, then hits reset button which results in passing undefined to the value prop, the value shown is 5 still)

๐Ÿ’ป Code Sample

https://codesandbox.io/s/nice-moore-ochpe?file=/src/App.js

๐ŸŒ Your Environment

Software Version(s)
react-spectrum 3.13.0
Browser Chrome Version 92.0.4515.107
Operating System macOS BigSur v11.4

๐Ÿงข Your Company/Team

Adobe/quarry

๐Ÿ•ท Tracking Issue (optional)

fhschan avatar Aug 10 '21 23:08 fhschan

@LFDanLu - sorry I couldn't directly assign to you. Pinging you in comment and leaving a comment in our Slack thread as well. Thanks again!

fhschan avatar Aug 10 '21 23:08 fhschan

Another line of interest: https://github.com/adobe/react-spectrum/blob/main/packages/@react-stately/numberfield/src/useNumberFieldState.ts#L108 IMO we should check if numberValue == null and call setInputValue with to "" in that case perhaps

LFDanLu avatar Aug 10 '21 23:08 LFDanLu

We'll also need to check for other areas where we do a similar NaN check and account for those as well.

LFDanLu avatar Aug 11 '21 18:08 LFDanLu

Is there a workaround for this?

hennessyevan avatar Jan 17 '22 18:01 hennessyevan

At the moment none that I know of, short of using something like patch package to patch the bug locally.

EDIT: @snowystinger has reminded me that you can use NaN instead of null here to properly clear the value within the NumberField for now.

LFDanLu avatar Jan 18 '22 17:01 LFDanLu

This is also an issue with the datePicker component see https://github.com/adobe/react-spectrum/issues/3187. I don't think there is an equivalent NaN workaround for this component either.

msdrigg avatar Jun 27 '22 03:06 msdrigg

I noticed the previous way to update a controlled value is via a useEffect hook but now it's changed to

if (!Object.is(numberValue, prevValue.current) || locale !== prevLocale.current || formatOptions !== prevFormatOptions.current) {
  setInputValue(format(numberValue));
  // ...
}

Looks like we can solve the problem by simply adding a sanity check in L97

let format = useCallback((value: number) => isNaN(value) ? '' : formatter.format(value), [formatter]); ===> let format = useCallback((value: number) => (isNaN(value) || value === null) ? '' : formatter.format(value), [formatter]);

lishichengyan avatar Nov 03 '22 17:11 lishichengyan