current community

your communities

more stack exchange communities

Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 6 down vote favorite

I've written a function that wraps the classList API, allowing you to pass an array of classes to add/remove, or the variable arguments already supported by classList.

The methods are the following:

var flatten = function(array) {
    return Array.prototype.concat.apply([], array);
};

var classMethod = function(method) {
    return function(element /*, classes... */) {
        return flatten(Array.prototype.slice.call(arguments, 1)).forEach(function(klass) {
            element.classList[method](klass);
        });
    }
};

var addClass = classMethod('add');
var removeClass = classMethod('remove');

Usage:

var elem = document.querySelector('.box');

addClass(elem,     'class1');
addClass(elem,     'class2', 'class3');
addClass(elem,    ['class2', 'class3']);

removeClass(elem,  'class1');
removeClass(elem,  'class2', 'class3');
removeClass(elem, ['class2', 'class3']);

I'm wondering if there's some better way to implement this function, say without using the forEach call and instead using Function.prototype.apply, but I get a TypeError: Illegal invocation when trying to refactor this into, say:

var classMethod = function(method) {
    return function(element /*, classes... */) {
        element.classList[method].apply(element, flatten(Array.prototype.slice.call(arguments, 1)));
    }
};

Is it possible to write this more succinctly, whilst keeping all of the use cases?

share|improve this question

1 Answer 1

up vote 2 down vote accepted

Interesting question,

I've been looking at this for a while, I dont think there is a more succinct way. I've been playing with reconstructing className from the add or remove function calls but it became a less succinct and terrible hack.

The core reason for that is default JS does not have enough convenience methods for arrays. If you were open to use something like LoDash, you could do this:

var addClass = function addClass(element /*, classes... */){
  element.className = _.uniq( element.className.split(' ').concat( _.flatten( _.rest( arguments ) ) ) ).join(' ');  
};

var removeClass = function removeClass(element /*, classes... */){
  element.className = _.difference( element.className.split(' '),  _.flatten( _.rest( arguments ) )  ).join(' ');
};

Other than that I really like your code; proper naming, indenting, size of functions, etc.

Update on illegal invocation, you are passing element as this to a function that is attached to classList, if you pass classList, then there will be no exception.

var classMethod = function(method) {
    return function(element /*, classes... */) {
        var classList = element.classList;
        element.classList[method].apply(classList,flatten(Array.prototype.slice.call(arguments, 1)));
    };
};

And then that works.. My mind is still boggling at that, but there you go.

share|improve this answer
    
Do you have any ideas on the Illegal invocation? To be honest I don't really know why the refactor I tried didn't work as expected. –  Ben Nov 24 '14 at 20:03
    
Found it, updated answer. –  konijn Nov 25 '14 at 13:11
1  
Brilliant. Indeed, I didn't realise that you could (and in this case needed to) apply with a property of an object. So we can also do apply(element.classList), but I think I prefer your idea of caching it in a variable. Note that we can simplify this a little further by removing element. from the start of of the call to apply (we can just use the cached value). Thanks! :-) –  Ben Nov 25 '14 at 13:46

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.