Dealing with multiple classes in addClass, removeClass and hasClass

[regiskuckaertz]

Hi,

I too thought that was a shame the classList API wouldn't deal with multiple class names, as I needed it for a personal project. Therefore, I changed each function so the string is split into an array and then iterated over with the appropriate method.

What do you think?

[krmgns]

While dealing with multiple classes, I think current hasClass will fail. Cos, if user assigned of an element as class="abc foo bar" and if search hasClass(el, "bar foo"), hasClass will find anything (with this imp: return classReg(c).test(el.className)).

Just as a proposal for this issue;

addClass = function(el, cls) {
    cls = trim(cls).split(/\s+/);
    var c, cs = [];
    while (c = cls.shift()) {
        if (!hasClass(el, c)) {
            cs.push(c);
        }
    }
    el.className = trim(el.className +" "+ cs.join(" "));
}

[rvagg]

@regiskuckaertz & @qeremy, you should submit some code for the tests that demonstrate how the current implementation(s) fail for your particular use case and how your fix(es) deal with the problem. Remember that these internal hasClass() etc. functions are purely internal and they are wrapped around by the public hasClass() etc. methods (i.e. https://github.com/ded/bonzo/blob/master/src/bonzo.js#L593-L600) which pre-split the passed in string by whitespace before calling these internal versions.

If there is a problem with the current implementation then we need to see it via tests so we can (a) understand the problem you're trying to solve (b) prevent regressions on this particular use-case for future versions that may see refactoring in this area and (c) ensure that your fix works across all browsers, for example, you're using ES5 Array methods in your classList changes, that's probably OK because classList is only in newer browsers but it'd be good to be able to see that working via tests.

Cheers & thanks for contributing!

[ded]

whoa whoa. wait.... this should already be working.... and there's already tests for it.

[ded]

guys, https://github.com/ded/bonzo/blob/master/src/bonzo.js#L562

[krmgns]

Actually, I looked at only scoped addClass function (bonzo.js#L298), not bonzo.addClass. OK guys, it's working already as I pointed above.

[ded]

excellent

[regiskuckaertz]

sorry for the lack of feedback, and a second sorry for not seeing the Bonzo.addClass et al, I don't know why I thought multiple classes would not work. thanks for pointing this out guys.

cheers!