.replace on array element not working in a loop javascript

Question

I wanted to write a function that gets all the dom elements with the class active and remove that class. I am trying to use the replace method by looping through the array obtained from getElementsByClassName('active') and using replace to replace it with a blank space. Two questions: why isn't the replace method removing the active class and what would be a better alternative to remove class with vanilla js.

function removeItem(){
  var activeItems = document.getElementsByClassName('active');
  for (i=0; i<activeItems.length; i++){
    var activeClass = activeItems[i].className;
    activeClass.replace('active', '')
  }
}

[This is what you want?](http://stackoverflow.com/questions/2155737/remove-css-class-from-element-with-javascript-no-jquery) — Tomás Cot, Aug 05 '14 at 23:17
Using `activeClass.replace('active', '')` will also modify classes like `reactive` and `notactive` to `re` and `not` respectively. — RobG, Aug 05 '14 at 23:21

vol7ron · Accepted Answer · 2014-08-06T00:14:14.320

2

.replace() returns a string, which you must store, it doesn't overwrite the variable.

activeClass = activeClass.replace('active','');  // this will still need to be set as the classname

Something like this:

function removeClass(){
  var activeItems = document.getElementsByClassName('active');

  for (var i=0,n=activeItems.length; i<n; i++){
     var classes = activeItems[i].className.split(' ') || [];
     for (var j=0,l=classes.length; j<l; j++){
        if(classes[j] === 'active') {
           classes.splice(j,1);
           j--;l--; // we popped a value off the stack
        }
     }
     activeItems[i].className = classes.join(' ');
  }
}

or as zzzzBov suggests, go native:

activeItems[i].classList.remove('active');

edited Aug 06 '14 at 00:14

answered Aug 05 '14 at 23:19

vol7ron

40,809
21
119
172

never mind the fact that that's the wrong way to remove a class from a list of classes. – zzzzBov Aug 05 '14 at 23:20
`activeItems[i].classList.remove('active');` is a better choice. But, if you're going to split, be sure to split with `/ \t\f\r\n/g` as it includes all CSS whitespace characters. – zzzzBov Aug 05 '14 at 23:27
Very good point on the white space. Feel free to update the answer. There's a lot I'd change about his function, which I didn't include. I was more focused on giving him a reason why `active` still existed, after the `replace()` and let him figure out what his site needs. Perhaps I should point him to a jQuery or Scriptaculous example so he can see how those libraries are doing it. They often have considerations that users overlook, even if the whole library is not needed. – vol7ron Aug 05 '14 at 23:35
Jquery would have definitely made things easier but I am trying to learn vanilla js. Good call on the classlist, I can add that to the repertoire. – Brian Aug 05 '14 at 23:57
@Brian: I'm not suggesting to use jQuery, I would suggest to look how they do it (the internals). jQuery is JS, thus, they have Vanilla under the sheets :), but what you may find is they may do it one way that's either more efficient, or supports multiple browsers. Generally when a group of people work on something they consider options that one person isn't going to think about on their own – vol7ron Aug 05 '14 at 23:58
@vol7ron—jQuery uses `\s` (search the source for `rspace =`, which is used with *split* in the *addClass* funtion). *element.classList* is not well supported (e.g. requires IE 10+ so bout 30% of browsers in use don't support it), I wouldn't rely on it. – RobG Aug 06 '14 at 00:59
I also wouldn't implement the function I gave. I only made little changes to his base function, perhaps too many. I probably have answers or code in github somewhere that has more complete changes. – vol7ron Aug 06 '14 at 00:59

score 0 · Answer 2 · answered Aug 05 '14 at 23:21

0

Strings are passed by value in Javascript. You must set the changed string. Also, .replace doesn't change the string but returns a new one.

activeItems[i].className = activeItems[i].className.replace('active', '');

answered Aug 05 '14 at 23:21

PurkkaKoodari

6,703
6
37
58

StackSlave · Answer 3 · 2014-09-23T00:28:07.120

You forgot to assign the className again.

activeItems[i].className = activeClass.replace(/(?:\s*)(active\s*)/, '');

Note that getElementsByClassName() might not work in older Browsers.

Raw JavaScript idea:

var doc = document;
function getClass(class){
  if(doc.getElementsByClassName){
    return doc.getElementsByClassName(class);
  }
  else{
    var tags = doc.getElementsByTagName('*'), l = tags.length, r = [];
    if(l < 1){
      return false;
    }
    else{
      var x = new RegExp('(^|\\s+)'+class+'(\\s+|$)');
      for(var i=0; i<l; i++){
        var tag = tags[i];
        if(tag.className.match(x)){
          r.push(tag);
        }
      }
      return r;
    }
  }
}
function removeClass(class){
  var cls = getClass(class), x = new RegExp('(^|\\s+)'+class+'(\\s+|$)');
  for(var i=0,l=cls.length; i<l; i++){
    var c = cls[i];
    c.className = c.className.replace(x, '');
  }
}

`'foo active bar'` would turn into `'foobar'` in your first example. — zzzzBov, Sep 22 '14 at 20:05

RobG · Answer 4 · 2014-08-06T00:12:02.877

-1

You can use the following set of functions to add and remove class names. Plain js, no library.

// Return true or false if el has className or not
function hasClassName (el, cName) {
    var re = new RegExp('(^|\\s+)' + cName + '(\\s+|$)');
    return el && re.test(el.className);
}

// Add class to element if it doesn't have it already
function addClassName (el, cName) {
    if (!hasClassName(el, cName)) {
        el.className = trim(el.className + ' ' + cName);
    }
}

// Remove className from element if it has it
function removeClassName(el, cName) {
    if (hasClassName(el, cName)) {
        var re = new RegExp('(^|\\s+)' + cName + '(\\s+|$)','g');
        el.className = trim(el.className.replace(re, ''));
    }
}

// Remove leading and trailing whitespace, reduce remaining whitespace to
// a single character
function trim(s) {
  return s.replace(/(^\s+)|(\s+$)/g,'').replace(/\s+/g,' ');
}

To keep zzzzBov happy, here are the same function using a space instead of \s (also removed reliance on the RegExp constructor so should be faster too). I'd recommend sticking with \s as I haven't seen any convincing argument not to, but make you're own choice.

// Return true or false if el has className or not
function hasClassName (el, cName) {
    return (' ' + el.className + ' ').indexOf(' ' + cName + ' ') != -1;
}

// Add class to element if it doesn't have it already
function addClassName (el, cName) {
    if (!hasClassName(el, cName)) {
        el.className = trimSpaces(el.className + ' ' + cName);
    }
}

// Remove className from element if it has it
function removeClassName(el, cName) {
    if (hasClassName(el, cName)) {
        el.className = trimSpaces((' ' + el.className + ' ').replace(' ' + cName + ' ', ' '));
    }
}

// Remove leading and trailing whitespace, reduce remaining whitespace to
// a single character
function trimSpaces(s) {
  return s.replace(/(^ +)|( +$)/g,'').replace(/ +/g,' ');
}

edited Aug 06 '14 at 00:12

answered Aug 05 '14 at 23:29

RobG

142,382
31
172
209

`\s` is not appropriate for matching space characters within a class attribute. – zzzzBov Aug 05 '14 at 23:34
@zzzzBov—please explain. The class value separator is supposed to be a space but may be tab or other whitespace depending on the host. `\s` matches spaces and any other whitespace character that's likely to be encountered. Is there a reason to believe it will fail? `\s` is widely used in libraries, I don't see reports of failures. – RobG Aug 05 '14 at 23:44
The class value separator must be a [whitespace character as defined in the spec](http://www.w3.org/TR/css3-selectors/#whitespace), which includes only space, tab, carriage return, line feed, and form feed. [`\s` on the other hand matches a much wider range of characters](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#special-white-space), and is therefor inappropriate. – zzzzBov Aug 05 '14 at 23:48
@zzzzBov—and every browser in use is utterly standards compliant? I acknowledged that `\s` matches more than just a space, but some browsers (and possibly other UAs) tolerate other characters. There is certainly no case that I know of where `\s` will fail. If it matters, jQuery uses `\s` too. – RobG Aug 05 '14 at 23:59
`\s` will match non breaking space characters and other such characters where it shouldn't. All modern browsers are compliant with this specification (IE9+). – zzzzBov Aug 06 '14 at 00:04
additionally, string building a `RegExp` object without properly escaping input is a bad idea. – zzzzBov Aug 06 '14 at 00:06
@zzzzBov—IE8 is still more than 20% of desktop browsers if you believe [*netmarketshare*](http://www.netmarketshare.com). Also a variety of browsers on various devices that aren't anywhere near up–to–date with all applicable standards and specifications. `\s` is compliant with the standard (it matches space) and also matches other whitespace characters that some browsers are known to support (such as tab). Non–breaking whitespace has never been an issue as far as I know, so I don't see why it's an issue for you. Using a space in the above functions is more likely to cause issues than `\s`. – RobG Aug 06 '14 at 00:52
after re-reading my last statement i can see where it sounds like IE8 and below don't support whitespace properly, but that was not my intent. I'd have load up some old VMs to see just how far back proper whitespace support goes, and if I had to guess I'd guess that it goes back beyond IE6 because it's so easy to match `/ \t\f\n\r/`. `\s` is **by the definition in the standard** not compliant with the standard, so you're wrong in that regard. – zzzzBov Aug 06 '14 at 03:27
@zzzzBov—`\s` is compliant in that it matches the allowed whitespace characters (ok, so does `/.+/`). The additional whitespace it matches is extremely unlikely to ever be used in a class value—would anyone seriously use, say, `0x2009` or ` ` or equivalent in a class value? I understand your point, I just don't agree that it's necessary. If you feel so strongly about it, post an answer that complies. No one else has, because it would mean allowing other whitespace and not messing with existing `\f\n\r\t` characters when removing a value, which seems more trouble than it's worth. – RobG Aug 06 '14 at 04:11

.replace on array element not working in a loop javascript

4 Answers4