Skip to content
This repository was archived by the owner on Jul 30, 2018. It is now read-only.

CSS manipulation + tests #2

Closed
wants to merge 3 commits into from
Closed

CSS manipulation + tests #2

wants to merge 3 commits into from

Conversation

bitpshr
Copy link
Member

@bitpshr bitpshr commented May 20, 2015

This PR includes addClass, containsClass, removeClass, toggleClass, and associated unit tests.

* dom.removeClass(document.body, 'loading');
*
* @example
* dom.removeClas(document.body, 'loading', 'pending');
Copy link
Member

Choose a reason for hiding this comment

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

typo


'null class'() {
assert.doesNotThrow(function () {
dom.containsClass(targetElement, null);
Copy link
Member

Choose a reason for hiding this comment

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

I assume this test case isn't testing passing nothing (as opposed to others with the same description for the other APIs) because TS makes that impossible without suppressing a warning. I'd vote to just remove this test case for this function then.

The way this is written, it actually will throw (this test fails in IE9), but see previous remark in that null shouldn't actually throw to be consistent with native implementations.

Copy link
Member Author

Choose a reason for hiding this comment

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

I updated all tests to be consistent with native implementations and to have clearer descriptions.

@bitpshr
Copy link
Member Author

bitpshr commented May 20, 2015

This should be ready for another review. Thanks!

@kfranqueiro
Copy link
Member

Merged via 8d5def0. I removed SVG testing and made the functions expect HTMLElement, since this really doesn't support SVG elements (created via createElementNS).

@eheasley eheasley modified the milestone: Milestone 1 May 22, 2015
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