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

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

Closed
fhschan opened this issue Aug 10, 2021 · 7 comments · Fixed by #3709
Closed

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

fhschan opened this issue Aug 10, 2021 · 7 comments · Fixed by #3709
Labels
bug Something isn't working rsp:NumberField

Comments

@fhschan
Copy link

fhschan commented Aug 10, 2021

🐛 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
Copy link
Author

fhschan commented Aug 10, 2021

@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!

@LFDanLu
Copy link
Member

LFDanLu commented Aug 10, 2021

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 LFDanLu added the bug Something isn't working label Aug 10, 2021
@LFDanLu
Copy link
Member

LFDanLu commented Aug 11, 2021

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

@hennessyevan
Copy link

Is there a workaround for this?

@LFDanLu
Copy link
Member

LFDanLu commented Jan 18, 2022

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.

@msdrigg
Copy link

msdrigg commented Jun 27, 2022

This is also an issue with the datePicker component see #3187. I don't think there is an equivalent NaN workaround for this component either.

@lishichengyan
Copy link
Contributor

lishichengyan commented Nov 3, 2022

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]);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working rsp:NumberField
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants