Use classList API if available #8636
Conversation
dhoko
force-pushed
the
feat/ClassListAPI
branch
from
814bc59
to
016f5fe
Compare
Inverse doc
dhoko
force-pushed
the
feat/ClassListAPI
branch
from
016f5fe
to
06d65b7
Compare
|
|
||
| // For modern browsers cf http://caniuse.com/#search=classList | ||
| if(node.classList) { | ||
| return node.classList.contains(klassName); |
There was a problem hiding this comment.
Do all browsers that implement node.classList also return a DOMTokenList that includes the contains() method?
A problem is that our tests will only cover one of both ways. Possibly we should split the method into two methods and test them separately @mattab ? Or we need to find a way to run these tests in an old browser eg IE6 regularly and on newer ones
There was a problem hiding this comment.
Yup, via can I use, we have only two issues for the partial support:
- toggle second arg
- svg support
To test, why don't you use browserstack ?
There was a problem hiding this comment.
We're using saucelabs partially for manual testing but we'd need to run them automated on each commit via travis. This would be doable but it'd need to be implemented / configured to run them automatically via saucelabs (or browserstack).
Also a problem is, I think, that some of the tests currently do not work eg on IE6. Not because piwik.js is not compatible with IE6 but I think some tests aren't.
So those are the two problems so far
There was a problem hiding this comment.
I think just to be safe we should also check for node.classList && node.classList.contains
|
Moving to 3.0.0 for now |
|
@dhoko I'd be happy to merge this PR but we would need this PR for the "3.x-dev" branch. The PR points to the "master" brach which is Piwik 2 which is in LTS mode and we merge only critical fixes for master. I'll close this issue for now but be cool if you could recreate the PR for 3.x-dev |
We can use the ClassList API if available on modern browsers cf Compat-table
There is a bug with IE11 (multiples arguments, with add/remove), but it's not problem