How equality works in javascript

Question

I am making a mobile menu. I want to change class in . The first step works and it removes 'fa-bars' class and adds 'fa-times'. But the second step does not work.

<div class="menuToggle">
       <i id='test' class="fas fa-bars fa-2x"></i>
    </div>     

        let menuActive = document.querySelectorAll('.menuToggle i')
        var element = document.getElementById('test');

        if(element.classList.contains('fa-bars')){
            function Activemenu(){
                menuActive.forEach((item)  =>
                item.classList.remove('fa-bars'));
                this.classList.add('fa-times');
            } 
        }else if(element.classList.contains('fa-bars')){
            function Activemenu(){
                menuActive.forEach((item)  =>
                item.classList.remove('fa-times'));
                this.classList.add('fa-bars');
            }  
        }
        menuActive.forEach((item) =>
            item.addEventListener('click',Activemenu));

Because it is not magically going to bind the other function. You are just going to get what first function you define. Not sure why you are not just doing the check inside the click method. — epascarello, Jan 06 '22 at 18:15

score 2 · Accepted Answer · answered Jan 06 '22 at 18:24

There are a couple of problems there:

You're using function declarations within if/else blocks. Those are standardized now in certain limited situations (which your code doesn't match), but it's complicated enough that you really just want to avoid using function declarations in blocks. Use function expressions assigned to variables instead.
You're using an arrow function for the event handler, but expecting this to be set by how the function is called. Arrow functions don't work that way.
You're checking for fa-bars in both classList.contains checks. You probably wanted fa-times in the second one.
You're only binding the first event handler to that element. Nothing updates the handler when the condition changes.

And just as a side note, I'd suggest using for-of loops over forEach callbacks:

const menuActive = document.querySelectorAll(".menuToggle i");
const element = document.getElementById("test");
function activeMenuClickHandler() {
    if (element.classList.contains("fa-bars")) {
        for (const item of menuActive) {
            item.classList.remove("fa-bars"));
        }
        this.classList.add("fa-times");
    } else if (element.classList.contains("fa-times")) {
        for (const item of menuActive) {
            item.classList.remove("fa-times"));
        )
        this.classList.add("fa-bars");
    }
}
for (const item of menuActive) {
    item.addEventListener("click", activeMenuClickHandler);
}

(Or similar, you may have to do some further tweaking.)

If this code only ever toggles fa-bars/fa-times, it may well be simpler to use the toggle function of classList:

const menuActive = document.querySelectorAll(".menuToggle i");
const element = document.getElementById("test");
function activeMenuClickHandler() {
    this.classList.toggle("fa-bars fa-times");
    for (const item of menuActive) {
        item.classList.toggle("fa-bars fa-times");
    }
}
for (const item of menuActive) {
    item.addEventListener("click", activeMenuClickHandler);
}

Thank you so much. Your first code is working. But I couldn't run the second code, which is short. — Sinan Sarıkaya, Jan 06 '22 at 18:46
@SinanSarıkaya - Must not be a simple toggle then. Glad the first thing is working! :-) — T.J. Crowder, Jan 06 '22 at 18:57

How equality works in javascript

1 Answers1