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

Add drag event API binding #641

Merged
merged 2 commits into from
Dec 16, 2022
Merged

Add drag event API binding #641

merged 2 commits into from
Dec 16, 2022

Conversation

ersinakinci
Copy link
Contributor

Add and use a new ReactEvent.Drag module for onDrag, onDrop, etc. The new module is like ReactEvent.Mouse but also includes a binding for dataTransfer.

@ersinakinci ersinakinci mentioned this pull request Dec 31, 2020
Copy link
Member

@davesnx davesnx left a comment

Choose a reason for hiding this comment

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

There hasn't been any reason why Drag wasn't part of reason-react. I left a small comment about dataTransfer, which has been the case for other events that expose Js.t instead of the opaque type.

I know that I'm reviewing this PR super late, but that's what we got. If there's no interest from your part, I'm happy to push it.

[@bs.get] external screenY: t => int = "screenY";
[@bs.get] external shiftKey: t => bool = "shiftKey";

[@bs.get] external dataTransfer: t => Dom.dataTransfer = "dataTransfer";
Copy link
Member

Choose a reason for hiding this comment

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

Dom.dataTransfer is an opaque type, probably better to return Js.t({..}). Even though it's unsafe it's better to allow users to access by object accessor.

@davesnx davesnx merged commit 4129739 into reasonml:master Dec 16, 2022
davesnx pushed a commit that referenced this pull request Dec 20, 2022
* feat(drag): add drag event API binding

* feat(drag): update onDrag*/onDrop bindings
davesnx added a commit that referenced this pull request Dec 20, 2022
…de-bs-platform-to-902

* 'master' of github.com:/reasonml/reason-react:
  Add ocaml lsp editor (#699)
  Add all aria attributes (#697)
  Update Drag.dataTransfer to Js.t (#701)
  Add drag event API binding (#641)
  Added `as` property to DOM props (#603)
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.

2 participants