-
Notifications
You must be signed in to change notification settings - Fork 381
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
Remove lines about using fireEvent to gain focus or blur #187
Conversation
Codecov Report
@@ Coverage Diff @@
## master #187 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 19 19
Lines 234 234
Branches 57 57
=====================================
Hits 234 234 Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing the documentation! I would rather apply the suggestions from #186 (comment) though i.e. input.blur()
instead of fireEvent.blur(input)
Yes, or people would just use |
Thanks for this contribution, and for noticing this in the first place. Indeed, using |
@eps1lon if you look at the diff, you will see that the example with This allows you to assert whether an element has focus or not.
#### Examples
```html
<div><input type="text" data-testid="element-to-focus" /></div>
```
##### Using document.querySelector
```javascript
const input = document.querySelector(['data-testid="element-to-focus"'])
input.focus()
expect(input).toHaveFocus()
input.blur()
expect(input).not.toHaveFocus()
```
- ##### Using DOM Testing Library
-
- ```javascript
- const input = queryByTestId(container, 'element-to-focus')
-
- fireEvent.focus(input)
- expect(input).toHaveFocus()
-
- fireEvent.blur(input)
- expect(input).not.toHaveFocus()
- ``` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@eps1lon if you look at the diff,
I did. The diff does not tell me that we already had a working example above.
This change is good as-is. The extra documentation wasn't needed in the first place.
@jzarzeckis indeed. But the existence of two examples was not because one used So your fix should be about making the example that was incorrectly using If you wonder why we show duplicate examples like this, indeed, I share the pain, and I've been meaning to remove that duplication. I'll accept your PR as is, because I'll remove that duplication in our documentation soon. |
Follow up: created #188 to solve that documentation duplication for good. In case anyone wants to take a look or chime in. |
🎉 This PR is included in version 5.0.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
@all-contributors please add @jzarzeckis for doc |
I've put up a pull request to add @jzarzeckis! 🎉 |
Close #186
What:
Removing lines in documentation that state you can use
fireEvent.focus(element)
Why:
To avoid confusion. More details & example provided in #186
Checklist: