More efficient way of doing this?

[rapor]

Hey guys, I’m kinda new to javascript, and I’m just wondering if there’s more a efficient way to do this. Basically when the page loads, I want a function to do this:

Change this:
<div class="prdctfltr_add_scroll prdctfltr_down" style="display: block;">
To this:
<div class="prdctfltr_add_scroll" style="display: none;">
And change this:
<i class="prdctfltr-up"></i>
To this:
<i class="prdctfltr-down"></i>

I couldn’t find a way to just replace a class, so I’ve had to add the one I want, then delete the one I don’t want. Does anyone know if this is the best way to do this?

window.onload = function(){
    
    //Get the class "prdctfltr_add_scroll" and remove class "prdctfltr_down" and add "Display:none" to the style
    var x = document.getElementsByClassName("prdctfltr_add_scroll");
    var i;
    for (i = 0; i < x.length; i++) {
        x[i].classList.remove("prdctfltr_down");
        x[i].style.display = "none";
    }
    
    //Add "prdctfltr-down" to "prdctfltr-up" 
    var a = document.getElementsByClassName("prdctfltr-up");
    var b;
    for (b = 0; b < a.length; b++) {
        a[b].classList.add("prdctfltr-down");
    }
    
    //Remove "prdctfltr-up"
    var c = document.getElementsByClassName("prdctfltr-down");
    var d;
    for (d = 0; d < c.length; d++) {
        c[d].classList.remove("prdctfltr-up");
    }
    
}

[Paul_Wilkins]

An improved way is to use querySelectorAll to obtain all elements that match the selector, along with using CSS to manage the presentation of things.

.hide {
    display: none;
}

window.onload = function(){
    document.querySelectorAll(".prdctfltr_add_scroll").forEach(function (el) {
        el.classList.remove("prdctfltr");
        el.classList.add("hide");
    });
    document.querySelectorAll(".prdctfltr-up").forEach(function (el) {
        el.classList.remove("prdctfltr-up");
        el.classList.add("prdctfltr-down");
    });
}

[rapor]

Thanks for the fast reply, but this isn’t working. I don’t actually understand that code also, shouldn’t there be some kind of “prdctfltr_add_scroll” in there somewhere?

[Paul_Wilkins]

Yes, that should be the first one, which I’ve just updated now.

If you can supply an example page at jsfiddle or codepen with future requests, that will make it easier to develop more fully working solutions.

[m3g4p0p]

@rapor

I couldn’t find a way to just replace a class

Hi @rapor, you might do this via the className property, which is the space separated list of class names; also, as @Paul_Wilkins suggests, better use querySelectorAll() so that you can directly query for the class you want to change (without changing the query result while modifying the classes):

document.querySelectorAll('.prdctfltr_up').forEach(function (el) {
  el.className = el.className.replace(/\bprdctfltr_up\b/, 'prdctfltr_down')
})

However first adding and then removing a given class is also a perfectly fine (and way more readable) way to do this. And if there are a lot of such elements on the page, you might batch the updates in an animation frame to avoid multiple browser repaints:

window.requestAnimationFrame(function () {
  document.querySelectorAll('.prdctfltr_up').forEach(function (el) {
    el.classList.remove('prdctfltr_up')
    el.classList.add('prdctfltr_down')
  })
})

[rapor]

Thanks, kind of got this working now, but this:

<div class="prdctfltr_add_scroll" style="display: none;">

actually needs to change the inline style, not just add a new “hide” class. So is there anyway to have something like this:

    document.querySelectorAll(".prdctfltr_add_scroll").forEach(function (el) {
        el.classList.remove("prdctfltr");
        el.classList.style.display = "none";
    });

That doesn’t work, but I’m trying to figure it out.

[rapor]

Thanks for that, I’ll try that out after I’ve figured out how to get this first issue sorted.

[Paul_Wilkins]

@rapor

actually needs to change the inline style

Yes, that can be done. It’s just frowned on to add style attributes to elements with class names being preferred to adjust the presentation of elements.

[rapor]

Well it’s for a collapsible widget. So basically when you click on it, it changes from “Display:Block” to “Display:None” to like open and close it. That’s why it’s inline.

Is there a line I could put in there to do that? Or given it has to be inline is it best to just keep it how I did it initially?

window.onload = function(){
    document.querySelectorAll(".prdctfltr_add_scroll").forEach(function (el) {
        el.classList.remove("prdctfltr_down");
        el.style.display = 'none';
    });
}

This works, not sure if that’s the way to do it, but it’s working

So here’s the code for anyone else that may need it:

window.onload = function(){
    document.querySelectorAll(".prdctfltr_add_scroll").forEach(function (el) {
        el.classList.remove("prdctfltr_down");
        el.style.display = 'none';
    });
    document.querySelectorAll(".prdctfltr-up").forEach(function (el) {
        el.classList.remove("prdctfltr-up");
        el.classList.add("prdctfltr-down");
    });
}

Couldn’t get the className.replace or window.requestAnimationFrame working at the moment, but I’ll look into a little more tomorrow. Thanks for all your help guys