feat: add interactOutsideBehavior to NumberField#9679
feat: add interactOutsideBehavior to NumberField#9679will-stone wants to merge 3 commits intoadobe:mainfrom
Conversation
|
Happy to have a stab at the docs but I'd like to please know if this is something you'd consider adding first 🙏 😅 |
|
@will-stone interesting, this went a bit of a different route than I was expecting but I suppose native number type input fields do behave the same way. @snowystinger Any opinions here? Do you remember any reasons we didn't go this route in the first place? |
|
Hi 👋 🙂 Anything I can do to advance this? If you're happy with the name then I can have a crack at the docs. |
|
@will-stone sorry about the delay, I'll take a look soon. I hadn't realized the team had already discussed this a while back haha, I'll take a look at overall behavior soon. Definitely feel free to take a crack at the docs, the team can help out with those if behavior needs to be refactored/naming gets changed/etc |
| /** | ||
| * Disables value snapping when user finishes editing the value (e.g. on blur). | ||
| */ | ||
| isValueSnappingDisabled?: boolean |
There was a problem hiding this comment.
such a long prop name, how would we feel instead about, question to the team
isSnapDisabled
There was a problem hiding this comment.
I think that shorter name is fine
There was a problem hiding this comment.
perhaps interactOutside behavior related? like RangeCalendar? and DatePicker?
See #8899 (review) as well
| expect(onChangeSpy).toHaveBeenCalledWith(5); | ||
| expect(textField).toHaveAttribute('value', '5'); | ||
|
|
||
| expect(container).not.toHaveAttribute('aria-invalid'); |
There was a problem hiding this comment.
maybe in the v3 spectrum component this should be false, but if we write the same test in the react aria component, does it have the invalid attribute? seems like it should
There was a problem hiding this comment.
perhaps out of scope for this PR's change, but seeing as setting this snapping option would allow a user to enter a value beyond the upper limit, should we also allow users to enter invalid negative numbers? I'm not sure if there is a realistic use case, but that would also bring us closer to native behavior.
There was a problem hiding this comment.
This one is stopped before the user can even see the negative sign reflected in the input, so shouldn't be the same problem. I think we leave it as is for now.
| /** | ||
| * Disables value snapping when user finishes editing the value (e.g. on blur). | ||
| */ | ||
| isValueSnappingDisabled?: boolean |
There was a problem hiding this comment.
I think that shorter name is fine
| * Controls the behavior of the number field when the user interacts outside of the field after editing. | ||
| * 'clamp' will clamp the value to the min/max values. | ||
| * 'none' will not clamp the value and will allow the value to be outside of the min/max values. | ||
| * No native validation around min/max. Provide your own validation function via the `validate` prop. |
There was a problem hiding this comment.
hmm we should mark the field as invalid if it is outside the min/max, or not on a step. You shouldn't need to re-implement this validation yourself
| formatOptions?: Intl.NumberFormatOptions, | ||
| /** | ||
| * Controls the behavior of the number field when the user interacts outside of the field after editing. | ||
| * 'clamp' will clamp the value to the min/max values. |
There was a problem hiding this comment.
| * 'clamp' will clamp the value to the min/max values. | |
| * 'clamp' will clamp the value to the min/max values, and snap to the nearest step value. |
Maybe 'snap' would be more correct since it also applies to step values?
As discussed here: #8776
✅ Pull Request Checklist:
📝 Test Instructions:
Step3WithMin2Max21ValueSnappingDisabledstory.onChangecallback fires with the same value you entered, and the input does not auto-snap to a valid number.🧢 Your Project:
This will be used in an e-commerce situation where the
NumberFieldis used for product quantity. Snapping is dangerous, for us, because a customer may not know that the number has been auto-healed, causing them to order less or more of what they need. I work for RS Group, and RAC powers our component library: ion. The component in question isn't live yet, but the RAC implementation is to replace ourQuantitySteppercomponent.