Skip to content

Add DataTransferItem and related API #55

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

Merged

Conversation

pete-murphy
Copy link
Contributor

@pete-murphy pete-murphy commented Mar 16, 2021

Prerequisites

  • Before opening a pull request, please check the HTML standard (https://dom.spec.whatwg.org/). If it doesn't appear in this spec, it may be present in the spec for one of the other purescript-web projects. Although MDN is a great resource, it is not a suitable reference for this project.

Description of the change

Fixes: #54

This would add support DataTransfer.items (HTML spec reference here, MDN docs here)


Checklist:

  • Added the change to the changelog's "Unreleased" section with a reference to this PR (e.g. "- Made a change (#0000)")
  • Linked any existing issues or proposals that this pull request should close
  • Updated or added relevant documentation
  • Added a test for the contribution (if applicable)

Comment on lines 11 to 12
-- | Returns the drag data item kind, which is either "string" or "file".
foreign import kind :: DataTransferItem -> String
Copy link
Contributor Author

Choose a reason for hiding this comment

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

An alternative here would be to create a sum type for this return value, since the spec says that it must be one of either "string" or "file". I don't know if that would be preferable.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should create a sum type if those are the two options.

Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps something like DataTransferItemType = StringItem | FileItem? It's a bit ugly 🤷

Copy link
Contributor Author

@pete-murphy pete-murphy Oct 8, 2021

Choose a reason for hiding this comment

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

The spec calls them Text and File respectively.
image
image

https://html.spec.whatwg.org/#dom-datatransferitem-kind
https://html.spec.whatwg.org/#the-drag-data-item-kind

So my initial thought was

data DataTransferItemKind = Text | File

but I can understand if those are too generic and we'd prefer to suffix with *Kind or *Type (or if Text is maybe an unexpected name for what has a JS value of "string"). Plenty of decisions here 😄 I'll push what I have but am open to suggestions for the naming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm, I've got this wrong actually... it can return an empty string "if the DataTransferItem object is in the disabled mode." I'll open this back up for review after I make the changes there.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated here 879e724

Copy link
Contributor

Choose a reason for hiding this comment

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

I could see suffixing them with *Kind to avoid clashes with other similar types, but I think it's also fine to assume this module will be imported qualified.

@pete-murphy pete-murphy marked this pull request as ready for review March 16, 2021 16:34
@thomashoneyman
Copy link
Contributor

Sorry for the late review on this, @ptrfrncsmrph -- I have one comment, but otherwise this looks good to me. @garyb, do you have any additional thoughts?

@@ -29,6 +30,10 @@ files = toMaybe <$> _files

foreign import _files :: DataTransfer -> Nullable FileList

-- | Returns a `DataTransferItemList` object which is a list of all of the drag
-- | data.
foreign import items :: DataTransfer -> DataTransferItemList
Copy link
Contributor

Choose a reason for hiding this comment

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

Remember to update the explicit exports at the top of the file so this is usable!


-- | Access an item in the `DataTransferItemList` by index.
dataTransferItem :: Int -> DataTransferItemList -> Maybe DataTransferItem
dataTransferItem = map Nullable.toMaybe <$> _dataTransferItem
Copy link
Contributor

Choose a reason for hiding this comment

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

Some of these look like they need to be added to the explicit exports

@pete-murphy pete-murphy marked this pull request as draft October 8, 2021 01:49
@pete-murphy pete-murphy marked this pull request as ready for review October 8, 2021 02:17
Copy link
Contributor

@thomashoneyman thomashoneyman left a comment

Choose a reason for hiding this comment

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

I'd like to see if @garyb has anything he'd like to say, but this looks good from my end!

Copy link
Member

@garyb garyb left a comment

Choose a reason for hiding this comment

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

All looks good to me, thanks!

@thomashoneyman thomashoneyman merged commit 0c49778 into purescript-web:master Oct 8, 2021
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.

DataTransfer.items
3 participants