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] onValueChange not being called when input value is changed #579

Open
foobarvaz opened this issue Sep 3, 2024 · 7 comments
Assignees
Labels
component: number field This is the name of the generic UI component, not the React module! status: waiting for maintainer These issues haven't been looked at yet by a maintainer

Comments

@foobarvaz
Copy link

foobarvaz commented Sep 3, 2024

Steps to reproduce

Link to live example:
https://codesandbox.io/p/sandbox/wpqjmg?file=%2Fsrc%2FApp.tsx%3A10%2C43-10%2C56

Steps:

  1. Focus on input and change it's value

Current behavior

onValueChange not being called when input value is changed

You can see that the text below the input will remain value: 0 until you focus out of the input. The blur leads to onValueChange being called.

Expected behavior

onValueChange being called when input value is changed

Context

Thanks for adding support for decimals in the new version of the number input ❤️

I'm working on an interactive calculator in which changes to input values should be reflected in calculations that are shown in the UI.

I tried to debug the issue and I found that on each change I reach this part of the code:

Because event.isTrusted === true in this case, we call only setInputValue without calling setValue (which internally calls onValueChange)

I'm not sure if it's by design or not. So forgive me if I classified this issues as bug 🐛 by mistake.

Your environment

npx @mui/envinfo
  Don't forget to mention which browser you used.
  Output from `npx @mui/envinfo` goes here.

Search keywords: onValueChange

@foobarvaz foobarvaz added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 3, 2024
@zannager zannager added the component: number field This is the name of the generic UI component, not the React module! label Sep 3, 2024
@atomiks
Copy link
Contributor

atomiks commented Sep 3, 2024

I'm not sure if it's by design or not. So forgive me if I classified this issues as bug 🐛 by mistake.

Yes it's by design. The reason is partial input may not be parseable as a valid number, so it can't properly be called in realtime/on change.

@bengry
Copy link

bengry commented Sep 3, 2024

I'm not sure if it's by design or not. So forgive me if I classified this issues as bug 🐛 by mistake.

Yes it's by design. The reason is partial input may not be parseable as a valid number, so it can't properly be called in realtime/on change.

Can this potentially be made available on the hook variant of NumberField? Or an optional prop that defaults to false (= immediate change behavior disabled).

I understand the concern, but it gives a worse UX (less immediate feedback) because of an edge case, for a lack of a better term - when a used enters invalid values. In the happy flow path, the UX is just worse without any benefit.
E.g. react-number-format deals fine with this - demo.

@atomiks
Copy link
Contributor

atomiks commented Sep 3, 2024

I think the main concern is with compositional inputs such as pinyin, which depends on the client's locale/number system. The input already blocks most invalid input before it can be typed in, but composition events aren't blocked yet represent incomplete data that can't be parsed?

This behavior matches the RA library, though it doesn't state what "partial input":

The onChange event is triggered whenever the number value updates. This happens when the user types a value and blurs the input, or when incrementing or decrementing the value. It does not happen as the user types because partial input may not be parseable to a valid number.

Open to suggestions or PRs to improve this behavior however

@bengry
Copy link

bengry commented Sep 3, 2024

The onChange event is triggered whenever the number value updates. This happens when the user types a value and blurs the input, or when incrementing or decrementing the value. It does not happen as the user types because partial input may not be parseable to a valid number.

Open to suggestions or PRs to improve this behavior however

I suggest the onValueChange fires only if the parsed input is valid.
e.g.
1onValueChange(1)
2onValueChange(12)
. → onValueChange not fired
3onValueChange(12.3)

@oliviertassinari oliviertassinari changed the title [NumberField]: onValueChange not being called when input value is changed [NumberField] onValueChange not being called when input value is changed Sep 11, 2024
@oliviertassinari
Copy link
Member

oliviertassinari commented Sep 11, 2024

Maybe an onInputChange event like in https://mui.com/material-ui/api/autocomplete/#autocomplete-prop-onInputChange would cover both needs (!== onChange)?


A side note, made me think of #600.

@bengry
Copy link

bengry commented Sep 11, 2024

Maybe an onInputChange event like in mui.com/material-ui/api/autocomplete#autocomplete-prop-onInputChange would cover both needs (!== onChange)?

This means that the component must be controlled though, no? Or at the very least - moves the burden of parsing the data to see if the number is valid (yet) to the consumer. When onInputChange can update on every keystroke, and not just on blur, since it already handles the parsing.
LMK if I missed something though.

@atomiks
Copy link
Contributor

atomiks commented Sep 12, 2024

@bengry it does seem ok to me that onValueChange is called as soon as it can, as long as the current number string is parseable/valid.

@oliviertassinari In our API, instead of onInputChange you can just use onChange on the Input subcomponent to read the string value in realtime.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: number field This is the name of the generic UI component, not the React module! status: waiting for maintainer These issues haven't been looked at yet by a maintainer
Projects
None yet
Development

No branches or pull requests

5 participants