Skip to content
This repository was archived by the owner on Oct 4, 2020. It is now read-only.

Add dropEffect getter and setter on DataTransfer#140

Merged
garyb merged 2 commits into
purescript-deprecated:masterfrom
fehrenbach:dropeffect
Jan 21, 2018
Merged

Add dropEffect getter and setter on DataTransfer#140
garyb merged 2 commits into
purescript-deprecated:masterfrom
fehrenbach:dropeffect

Conversation

@fehrenbach

Copy link
Copy Markdown
Contributor

No description provided.

@garyb garyb left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Your PR to Halogen reminded me of this one that has been outstanding for ages. Sorry about the delay!

"copy" -> Copy
"move" -> Move
"link" -> Link
"none" -> None

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

unsafePartialBecause is problematic (purescript/purescript-partial#7), could you use crashWith in an _ case, and then unsafePartial the case instead please?

Comment thread src/DOM/HTML/Event/DataTransfer.purs Outdated

foreign import dropEffectImpl :: forall eff. DataTransfer -> Eff (dom :: DOM | eff) String

data DropEffect = None | Copy | Move | Link

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Could you add derived instances for Eq and Ord for this too please?

@fehrenbach

Copy link
Copy Markdown
Contributor Author

No worries. What kind of heathen makes pull requests on Christmas Day anyways? ;)

Thanks for the review. I addressed your remarks and sorted the cases while I was at it.

@garyb

garyb commented Jan 21, 2018

Copy link
Copy Markdown
Member

Thanks!

@garyb garyb merged commit a0729ea into purescript-deprecated:master Jan 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants