-
Notifications
You must be signed in to change notification settings - Fork 24
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
Add DataTransferItem and related API #55
Conversation
-- | Returns the drag data item kind, which is either "string" or "file". | ||
foreign import kind :: DataTransferItem -> String |
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.
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.
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.
I think we should create a sum type if those are the two options.
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.
Perhaps something like DataTransferItemType = StringItem | FileItem
? It's a bit ugly 🤷
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.
The spec calls them Text and File respectively.
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.
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.
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.
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.
Updated here 879e724
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.
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.
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 |
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.
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 |
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.
Some of these look like they need to be added to the explicit exports
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.
I'd like to see if @garyb has anything he'd like to say, but this looks good from my end!
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.
All looks good to me, thanks!
Prerequisites
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: