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(fireEvent): helper to assign target properties to node #85

Merged
merged 1 commit into from Aug 6, 2018

Conversation

kentcdodds
Copy link
Member

@kentcdodds kentcdodds commented Aug 4, 2018

What: feat(fireEvent): helper to assign target properties to node

Why: Specifically this is so the change event can set the value of the node
in a way that works for React (and all other frameworks and
non-frameworks) nicely.

fixes: testing-library/react-testing-library#152

How: Add some logic to the fireEvent utility, and borrow some code from a thread in react issues.

Checklist:

  • Documentation
  • Tests
  • Ready to be merged
  • Added myself to contributors table N/A

@kentcdodds kentcdodds force-pushed the pr/assign-target-properties branch 3 times, most recently from 1234404 to 1b420b8 Compare August 4, 2018 23:27
@kentcdodds
Copy link
Member Author

I'd love feedback on this. This change will mean we can remove change from this list which I'm trying to slowly wittle down to nothing.

Specifically this is so the change event can set the value of the node
in a way that works for React (and all other frameworks and
non-frameworks) nicely.

fixes: testing-library/react-testing-library#152
@codecov
Copy link

codecov bot commented Aug 4, 2018

Codecov Report

Merging #85 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #85   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           9      9           
  Lines         152    161    +9     
  Branches       39     44    +5     
=====================================
+ Hits          152    161    +9
Impacted Files Coverage Δ
src/events.js 100% <100%> (ø) ⬆️

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 2397f64...1828405. Read the comment docs.

fireEvent.change(node, {target: {value: 'a'}}),
).toThrowErrorMatchingInlineSnapshot(
`"The given element does not have a value setter"`,
)
Copy link
Member Author

Choose a reason for hiding this comment

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

I just gotta say, this inline snapshot business is just the best.

@kentcdodds
Copy link
Member Author

Ok, I'll go ahead and self-approve this PR. I hope I don't regret this! 😅

@kentcdodds kentcdodds merged commit 19c2868 into master Aug 6, 2018
@kentcdodds kentcdodds deleted the pr/assign-target-properties branch August 6, 2018 21:37
@kentcdodds
Copy link
Member Author

🎉 This PR is included in version 3.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Copy link
Collaborator

@jomaxx jomaxx left a comment

Choose a reason for hiding this comment

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

@kentcdodds looks good! sorry i couldn't review it before it got merged!

@kentcdodds
Copy link
Member Author

Thanks @jomaxx!

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.

Form Inputs not updating after change event is fired
2 participants