You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
In text-field, onChange is only called when change originates from user interaction, but not when the controlled value changes. I think this is the right semantics. Some use cases need to be able to make the distinction if change comes from user action.
In case 1) note that when inputting a number bigger than 1000, and then pressing Set Number to 10 it first calls onChange with the formatted value having the thousand separator, which shouldn't be called when value is discarded when controlled value changes.
Side Note: In practice the text mode isn't really suitable as I don't have access to formatter/parser from the outside and it's necessary to convert numbers. Note that as soon as value is formatted with separators, controlled value is wrong.
Case 2) is the one that matters in practice. At construction, onRawValueChange is called with NaN, which is debatable if it should be called. While typing the number bigger than 1000 onRawValueChange is called as expected. However when the Set Number to 10 button is pressed, onRawValueChange is called twice, once again with the typed number (after formatting) and then with 10. If wanting to reproduce text-field semantics, it should not be called at all since it just takes the externally controlled value.
I add some slightly subjective opinion about number-field here:
There is following API to specify controlled value value: Accessor<string | number | undefined> and rawValue: Accessor<number>. It's not clear why two props are needed and if using always value is equivalent or not. What happens if two separate signals are passed? Maybe it's better to have a single source of truth for the controlled value (eg. remove rawValue prop).
I think it's hard to have the correct behavior in edge cases when having controlled values for both text and number. In react spectrum NumberField, they have chose to just expose the number, which I think would be sufficient. I rather have a simpler API with the event semantics like text-field rather than more flexibility but unpredictable calls to events.
When text of number field is empty, I prefer having undefined in the onRawValueChange callback, rather than NaN. The main reason I prefer to avoid it is the NaN != NaN behavior which can be nasty in change detection (in signals or other places). Note that this is quite subjective.
Naming: I was initially a bit confused by the naming of rawValue. Given this component is about number, my brain associates value with the number and raw with the text form as typed. It's obviously not a big issue, especially since the documentation is great. I would find the inverse naming more logical, or alternatively some name that avoids raw like numberValue.
The text was updated successfully, but these errors were encountered:
jcmonnin
changed the title
number-field: inconsistentcies when onChange and onRawValueChange is called
number-field: inconsistencies when onChange and onRawValueChange is called
May 17, 2024
The onChange and onRawValueChange get called whenever any async events cause them to change internally. There might be a way to predict exactly when they should and shouldn't be called but it would be highly speculative.
The component works if both are controlled, although might be a bit hard to predict, but it's the only way currently to access those values (until contexts are exposed). For example if you need to display the formatted value elsewhere on the screen and also need the number value for processing.
This is definitely something I'd like to improve however so I'll keep this open while I consider what to do.
Thanks for the issue!
In
text-field
,onChange
is only called when change originates from user interaction, but not when the controlled value changes. I think this is the right semantics. Some use cases need to be able to make the distinction if change comes from user action.See this playground to illustrate the
text-field
behavior. Note that on the console, noonChange
message appears whenSet Text to "Test"
button is pressed:https://playground.solidjs.com/anonymous/174c66df-0efe-4815-9469-abc6f0ecc552
number-field
doesn't behave that way. Here are the examples:In case 1) note that when inputting a number bigger than 1000, and then pressing
Set Number to 10
it first callsonChange
with the formatted value having the thousand separator, which shouldn't be called when value is discarded when controlled value changes.Side Note: In practice the text mode isn't really suitable as I don't have access to formatter/parser from the outside and it's necessary to convert numbers. Note that as soon as value is formatted with separators, controlled value is wrong.
Case 2) is the one that matters in practice. At construction,
onRawValueChange
is called with NaN, which is debatable if it should be called. While typing the number bigger than 1000onRawValueChange
is called as expected. However when theSet Number to 10
button is pressed,onRawValueChange
is called twice, once again with the typed number (after formatting) and then with 10. If wanting to reproducetext-field
semantics, it should not be called at all since it just takes the externally controlled value.I add some slightly subjective opinion about
number-field
here:There is following API to specify controlled value
value: Accessor<string | number | undefined>
andrawValue: Accessor<number>
. It's not clear why two props are needed and if using alwaysvalue
is equivalent or not. What happens if two separate signals are passed? Maybe it's better to have a single source of truth for the controlled value (eg. removerawValue
prop).I think it's hard to have the correct behavior in edge cases when having controlled values for both text and number. In react spectrum NumberField, they have chose to just expose the number, which I think would be sufficient. I rather have a simpler API with the event semantics like
text-field
rather than more flexibility but unpredictable calls to events.When text of number field is empty, I prefer having
undefined
in theonRawValueChange
callback, rather than NaN. The main reason I prefer to avoid it is the NaN != NaN behavior which can be nasty in change detection (in signals or other places). Note that this is quite subjective.Naming: I was initially a bit confused by the naming of
rawValue
. Given this component is about number, my brain associates value with the number and raw with the text form as typed. It's obviously not a big issue, especially since the documentation is great. I would find the inverse naming more logical, or alternatively some name that avoids raw likenumberValue
.The text was updated successfully, but these errors were encountered: