-
Notifications
You must be signed in to change notification settings - Fork 470
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(events): Add dataTransfer event property option #554
feat(events): Add dataTransfer event property option #554
Conversation
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 94d4eaa:
|
Codecov Report
@@ Coverage Diff @@
## master #554 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 23 23
Lines 454 459 +5
Branches 111 113 +2
=========================================
+ Hits 454 459 +5
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.
Looks good to me! Just one thing.
src/events.js
Outdated
} | ||
|
||
// Approximate dataTransfer on the event object | ||
// jsdom does not support DataTransfer constructor |
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.
We don't only support JSDOM, so let's try the DataTransfer constructor first and if that's unsupported then we can do this workaround. Sound ok?
That being the case, we'll probably need to add /* istanbul ignore if */
or /* istanbul ignore else */
to maintain 100% code coverage.
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.
Super 👍
@all-contributors please add @jamsinclair for code and tests |
I've put up a pull request to add @jamsinclair! 🎉 |
🎉 This PR is included in version 7.5.0 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What:
Resolves #551
Adds a convenience option to approximate the
dataTransfer
event property when firing eventsWhy:
Provides a consistent and convenient way to populate
dataTransfer
properties, much like populatingtarget
properties. This would help developers better simulate testing for drag and drop interactions.There was previous discussion and feature validation over in testing-library/react-testing-library#339
How:
Update the
fireEvent.*
functions to populate thedataTransfer
property for events only when defined in theeventProperties
(second argument).The property is assigned to the event object in a simple way. It does not construct the
DataTransfer
object and for good reasons.DataTransfer
is privately handled and constructed by the browser and is not supported by jsdom.MDN also warns:
Checklist:
docs site: docs: add dataTransfer event property info testing-library-docs#459
DefinitelyTyped N/A