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

Polyfill event.dataTransfer.type for drag & drop events #5529

Closed
wants to merge 1 commit into from

Conversation

jansedivy
Copy link

This pull request demonstrates our idea (me and @hejld) how to fix #3656 and #2676. We started with just polyfilling the types property for now.

There are few aspects that are not finished yet - object pooling, checking if it is possible to use defineProperty, and tests - but it should be enough to gather initial feedback.

Thanks a lot!

cc @syranide

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at [email protected]. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@hejld
Copy link
Contributor

hejld commented Jan 5, 2016

@spicyj @zpao any thoughts on this? Should we proceed with implementation? Thanks!

@syranide
Copy link
Contributor

syranide commented Jan 5, 2016

Object.defineProperty isn't available on IE8.

Also, this.dataTransfer should be set using the same principle as in (or is there a reason not to?):
https://github.com/jansedivy/react/blob/drag-fix/src/renderers/dom/client/syntheticEvents/SyntheticMouseEvent.js#L34

It also seems problematic as this doesn't polyfill functionality universally and, as far as I can tell, will simply error if you try to use this API on a lesser browser. So it seems to me that this deviates from existing event polyfills provided by React and it's not immediately clear to me whether going this route is a good idea or not (long-term/maintainability/etc).

@facebook-github-bot
Copy link

@jansedivy updated the pull request.

@hejld
Copy link
Contributor

hejld commented Jan 6, 2016

Thanks! This helps a lot. Will get back to you once the feedback is incorporated.

@gaearon
Copy link
Collaborator

gaearon commented Mar 27, 2016

Thank you for taking time to contribute. I’m closing as this PR has not received updates in response to the feedback in #5529 (comment). Please feel free to open another one if you plan to get back to this!

@gaearon gaearon closed this Mar 27, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

In Chrome, synthetic drag and drop events should use DOMStringList for event.dataTransfer.types
5 participants