-
Notifications
You must be signed in to change notification settings - Fork 273
RFC: UserEvent to throw errors in case element is disabled or not focusable #1443
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
Comments
Sounds great to me! |
If I recall correctly the reasoning behind removing the error in fireEvent was that it was a test specific behaviour and we try to match production as much as possible. I'm not sure why we would want to change this for userEvent. If the OG userEvent does throw then it's definitely an argument in favour of throwing though. Personally I like the fact that fireEvent doesn't throw because it allows me to test that my button is disabled by trying to click on it and verifying nothing happened. I'd do the same things for inputs. I'd argue it's the best way to test that it's disabled because what you really want is that user cannot interact with it so how best to test it than to try to interact with it? For press I don't really know how we could throw errors because we go up in the tree if the element is disabled so we could throw eventually if all elements are disabled but I'm not sure it would be very valuable @mdjastrzebski I would be curious to have your thoughts on how throwing would improve DX. Thinking of it it could be useful for debugging tests but because I want to be able to fire events on disabled elements I would not want this feature always. Maybe it could be an optional behaviour that could be configured through configure API? It would allow more flexibility and like you could enable throw while debugging test. The downside is that it makes the API more complicated and I don't know if many users would use it |
Thinking more about it jest native marchers are a very good alternative to test disabled elements. The only downside is you need direct access to the TextInput but you also need it for type and clear anyway. So now I think it's a good idea to throw. I'm just wondering now if the error message should recommend jest native marchers as a way to test that an element is disabled |
I've done some experiments in RTL's User Event on how do they handle disabled elements:
BTW: you can tinker yourself in #1447 RTL experiments app. If we wanted to refer to OG User Event for consistent, well-thought approach, then it's not there 🤷♂️ While there might be some logic with |
Few thoughts that I have about throwing/no throwing errors.
|
I think I would throw when user uses |
I am also leaning to the direction that we should throw on invalid actions: e.g. typing or clearing disabled |
Closing as stale. |
Describe the Feature
User Event should throw errors in case of given element is disabled in any way that prevents the interaction to run in order to improve DX.
OG User Event does throw errors in similar cases for
clear()
, andclick()
/pointer()
interactions.Note: This would be a breaking change, but according to User Event beta status would only trigger a minor change in the version.
Note 2: In the past we fireEvent was already throwing errors in the past when no enable event handler was found but that has been removed. Here are refs to the issue: #469, and PR #470
Possible Implementations
type()
&clear()
should throw helpful error when element is noteditable
or not focusable due topointerEvent
prop on itself or any ancestorpress()
should throw helpful error when element is not pressable due topointerEvents
prop on itself on ancestor.These should not throw in case no event handler is found, as user interaction can invoke multiple events handler, hence any particular event handler is optional.
Related Issues
Initial discussion
CC: @pierrezimmermannbam @MattAgn @AugustinLF @thymikee
The text was updated successfully, but these errors were encountered: