Skip to content
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

feat: add special keys to userEvent.type() #235

Conversation

malwilley
Copy link
Collaborator

@malwilley malwilley commented Mar 15, 2020

Hello! I hope it's okay if I submit a PR for this feature. It seems to be highly requested (#181, #172, #31) and it would make this library much more useful for some of my own use cases. Was hoping this would be a smaller change, but the scope got away from me a little bit!

This adds support for some new key presses in userEvent.type(). I am following the same syntax that cypress uses in their .type() command.

Commands added:

  • {enter}
  • {esc}
  • {backspace}
  • {shift}
  • {ctrl}
  • {alt}
  • {meta}

This does not yet track cursor position or selection range, so keys like {leftarrow} have not been implemented. This work could be done in a followup PR, but I wanted to keep the scope of this relatively small so I stopped here.

Code notes:

  • The code for the type() command was getting pretty unwieldy so I've broken it out into its own file.
  • Testing that events were being called with the right properties was resulting in some hairy test code. To combat this I've added toHaveBeenCalledWithEventAtIndex which helps assert that the event handlers are being called with the correct events.

Checklist:

  • Documentation
  • Tests
  • Typings (N/A)
  • Ready to be merged

@malwilley malwilley force-pushed the feature/type-special-characters branch from cc9866b to 9302145 Compare March 15, 2020 22:47
@codecov
Copy link

codecov bot commented Mar 15, 2020

Codecov Report

Merging #235 into master will increase coverage by 0.17%.
The diff coverage is 97.36%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #235      +/-   ##
==========================================
+ Coverage   98.90%   99.07%   +0.17%     
==========================================
  Files           1        2       +1     
  Lines         182      216      +34     
  Branches       55       60       +5     
==========================================
+ Hits          180      214      +34     
  Misses          2        2              
Impacted Files Coverage Δ
src/index.js 98.75% <90.00%> (-0.16%) ⬇️
src/type.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6239c3a...f6d606a. Read the comment docs.

@malwilley
Copy link
Collaborator Author

@kentcdodds @Gpx happy to see a flurry of activity on this repo!

I'd be happy to fix the conflicts on this PR if the changes are still wanted. Since this is such a big feature, however, I would not at all be offended if you preferred to tackle this in a different way. Let me know, and thank you for taking time to maintain this library!

@kentcdodds
Copy link
Member

Hi! Thanks so much for this. Sorry for messing up the git history for you a bit 😬

I hadn't noticed this PR, but I'm definitely a fan. If you'd like to get it updated, I'll help get it merged. Thanks!

@malwilley malwilley force-pushed the feature/type-special-characters branch from 9302145 to 1d59b8e Compare May 25, 2020 20:16
@malwilley malwilley force-pushed the feature/type-special-characters branch from 1d59b8e to f6d606a Compare May 25, 2020 20:56
@malwilley malwilley changed the title Add support for special keys in userEvent.type() feat: add special keys to userEvent.type() May 25, 2020
@malwilley
Copy link
Collaborator Author

@kentcdodds all cleaned up and ready for review!

Note that there are some prettier changes in index.js due to a previous PR getting merged without the new formatting requirements.

Also, code coverage is technically down in index.js, but I believe that's only because I reduced the number of lines in that file when I extracted the type function.

@kentcdodds
Copy link
Member

Thanks for your work on this. Just want to let you know that I've seen this. I need more time to review it though. I hope to get back to you soon though.

'{backspace}': {
key: 'Backspace',
code: 8,
inputType: 'deleteWordBackward',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be deleteContentBackward?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Yes it should

@kentcdodds
Copy link
Member

Hi @malwilley,

I promise I'm still thinking about this PR. It's in my inbox and I think about it multiple times a day. I really feel like there's something we could do to improve the design and reduce code complexity, but I think I need to experiment with it myself a little bit. I may work on it a little bit today on my live stream at https://kcd.im/discord if you want to join in later.

@malwilley
Copy link
Collaborator Author

@kentcdodds No problem at all! I too think there is a lot of room for improvement -- I'm all ears if there is a more elegant solution to be found.

kentcdodds added a commit that referenced this pull request Jun 4, 2020
@kentcdodds
Copy link
Member

Thanks for all your work on this! I've opened a new PR that I think you'll really like. Would love your feedback: #308

kentcdodds added a commit that referenced this pull request Jun 4, 2020
@kentcdodds
Copy link
Member

🎉 This issue has been resolved in version 11.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants