-
Notifications
You must be signed in to change notification settings - Fork 251
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
Rework userEvent.type #521
Comments
This makes a lot of sense to me, I am keen to try to pick up some part of this as I mentioned yesterday but digging into the code, it is very much written with inputs in mind and it is a little unclear the best way to improve this... This looks like a great summary! |
@kentcdodds Could you weigh in on this before we start any work here?
|
All |
I guess it will be a step towards that premise:
|
It's unusual to have an optional argument at the beginning, and I think it adds more confusion than it's worth when it represents something different. Perhaps we could name it |
I tend to agree with @nickmccurdy. Would it make sense to think about the different types of thing a user is actually trying to do and approach them differently with a different API. Specifically
I feel like there is some core functionality in there perhaps but all the assumptions of clicking first and things like that, don't make sense for someone pressing say shift + f to do a shortcut key in your app. |
As all Testing-Library packages aim for readability of the test, exposing the same core functionality per two APIs makes a lot of sense. @sethreidnz What exactly do you have in mind regarding the assumptions apart from clicking first? |
I've spotted another issue with If it's strictly necessary, I'd suggest an optional callback to allow the default 'type next key' behaviour to be overridden. |
I have found what I believe to be a related issue to this one, which is documented in this CodeSandbox: https://codesandbox.io/s/userevent-unit-test-ncmgu?file=/src/App.test.js Setup of the tested component
Test execution:
|
@psullivan6 |
The rewrite is merged. The remaining issues can be tackled individually. |
We've got multiple open issues regarding the implementation of
userEvent.type
.And while a few people offered help on this, I've got the impression we're a bit lost regarding what the code should do and look like in general as well as how and where each smaller problem should be addressed.
I propose we collect those issues, discuss what we want to achieve and refactor and comment the code so that we get more confidence working on this.
keyEvent.code
userEvent.type: wrong keyCode for dot character? #456userevent.type
andblur()
#460 / Fix #460 #487Should I have overlooked something, please mention it and I'll add it to the list.
What do people expect
userEvent.type
to do? What should it do?While
userEvent.click
is very clear and intuitive to use asuserEvent click this element!
,I guess
userEvent.type
started the same wayuserEvent trigger the key events to add to the value of this element!
,but I think most people expect it to be more
userEvent press those buttons on the keyboard!
.I think it should resolve around
document.getSelection()
anduserEvent.type(null, "a{del}{shift}b{tab}c{/shift}[Semicolon]d{selectall}")
should be a valid expression to test handling of hot keys and similar.One problem that comes to my mind is how to handle the keypress events as they are triggered multiple times as long as the button is pressed. Maybe a configurable random timeout between key events? So that even an expression for
Press [A] and then press [B] before releasing [A]. Then release [B].
would make sense.The text was updated successfully, but these errors were encountered: