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

Add classList + functions of DOMTokenList #106

Merged
merged 7 commits into from
Jun 11, 2017
Merged

Add classList + functions of DOMTokenList #106

merged 7 commits into from
Jun 11, 2017

Conversation

sectore
Copy link
Contributor

@sectore sectore commented Jun 9, 2017

domtokenlist-tests

- Following functions of `DOMTokenList` are supported by modern
browsers:
* `add`
* `remove`
* `toggle`
* `contains`
* `item`

- incl. tests
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.

Thanks for working on this! Just a few minor things, and this PR needs a rebase since the test PR was merged.

test/Main.purs Outdated

-- liftEff :: forall eff a. Eff (phantomjs :: PHANTOMJS | eff) a -> Aff (phantomjs :: PHANTOMJS | eff) a
-- liftEff = EffClass.liftEff
Copy link
Member

Choose a reason for hiding this comment

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

Dead code

foreign import _item :: forall eff. Int -> DOMTokenList -> Eff (dom :: DOM | eff) (Nullable String)

item :: forall eff. Int -> DOMTokenList -> Eff (dom :: DOM | eff) (Maybe String)
item index = map toMaybe <<< _item index
Copy link
Member

Choose a reason for hiding this comment

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

Can you rearrange the arguments in here (and accordingly in the FFI) so the DOMTokenList comes first in each case please?

@sectore
Copy link
Contributor Author

sectore commented Jun 11, 2017

@garyb No problem, latest commits ⬆️ come with these fixes (rebase + remove dead code + rearrange arguments).

into feature/classlist-domtokenlist
@garyb
Copy link
Member

garyb commented Jun 11, 2017

Thank you!

@garyb garyb merged commit 2bea9c7 into purescript-deprecated:master Jun 11, 2017
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.

4 participants