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

docs: keyPress warning #1023

Closed

Conversation

dzianis-l-backup
Copy link

To stress out the aspect of not firing the keyPress event not having specified the charCode property.
testing-library/react-testing-library#269 (comment)

Co-authored-by: Philipp Fritsche <ph.fritsche@gmail.com>
@@ -96,6 +96,8 @@ fireEvent.keyDown(domNode, {key: 'Enter', code: 'Enter', charCode: 13})
fireEvent.keyDown(domNode, {key: 'A', code: 'KeyA'})
```

> :warning: `React` doesn't dispatch a `SyntheticEvent` for a native `keypress` event without `event.charCode === 13 || event.charCode >= 32`.
Copy link
Member

Choose a reason for hiding this comment

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

If I'm not mistaken, this would be the first time that we mention something React specific into the core docs. From my point of view, I think we should keep the core docs framework agnostic, or else we can add more and more cases like these into the docs, which would make them harder to read.
What if we move this to the React FAQ or cheatsheet?
Thoughts @MatanBobi ?

Copy link
Member

Choose a reason for hiding this comment

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

From the resolved conversation above:

Note, that this is framework-specific and almost all use cases for this should use user-event instead.

Copy link
Member

Choose a reason for hiding this comment

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

Yea, you're correct @ph-fritsche , but then maybe this comment shouldn't be added to the docs?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, this is some framework-specific knowledge that should be irrelevant to almost all developers.
The longer I think about it, the more I'm convinced that not adding such information would be better and that the fact that someone ran into this rather means we should do more to encourage people to use the right tool.
See #963 (comment)

@timdeschryver
Copy link
Member

Thanks for the PR @Owrlean , but based on the discussion in this PR I'm going to close it.

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

Successfully merging this pull request may close these issues.

None yet

4 participants