feat: add isValueSnappingDisabled to NumberField#9679
feat: add isValueSnappingDisabled 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
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.