JavaScript event listener is not working with if statement

Viewed 137

I have a container div with many smaller divs inside of it, each with a class of box. The container is styled as a grid. I have an event listener that changes the background color of the grid boxes when there is a mouseover event.

I want to make a psychedelic mode that makes the boxes a random color when there is a mouse over instead of a fixed color.

I have a button:

 <button class="psychMode">Psychedelic Mode</button>

It is identified in the JavaScript:

let psych = document.querySelector('.psychMode');
let psychClass = psych.getAttribute('class');

I have a toggle on the class of the button to change it to psychMode active when the button is clicked:

psych.addEventListener('click', function () {
    psych.classList.toggle('active');
})

Finally, I have a function that returns a random number and the event listeners for the boxes:

function randomColor() {
    let num= Math.floor(Math.random() * 255);
    return num;
}

let boxes = document.querySelectorAll('.box')

for (let i of boxes) {
    i.addEventListener('mouseover', () => {
        i.style['backgroundColor'] = 'black'
        i.style.border = '1px solid red'
    })
}

for (let i of boxes) {
    if (psychClass === 'active') {
        i.addEventListener('mouseover', () => {
            i.style['backgroundColor'] = `rgb(${randomColor()},${randomColor()},${randomColor()})`
            i.style.border = `1px solid rgb(${randomColor()},${randomColor()},${randomColor()})`
        })
    }
}

For some reason, the code only results in a black background and red border. It doesn't choose a random color even when the active class is toggled on. I also tried putting both conditionals in the for loop together, it still didn't work. Can someone tell me what I'm doing wrong?

2 Answers
for (let i of boxes) {
    i.addEventListener('mouseover', () => {
        if(!psych.classList.contains("active"){
          i.style.backgroundColor = 'black'
          i.style.border = '1px solid red'
        }else{
          i.style.backgroundColor = `rgb(${randomColor()},${randomColor()},${randomColor()})`
          i.style.border = `1px solid rgb(${randomColor()},${randomColor()},${randomColor()})`
        }
    })
}

use this instead of those 2 for loops.

make your code as concise as possible and avoid unnecessary loops.

You're only ever assigning psychClass once, so it always equals "psychMode".

You'd be better off ditching that variable altogether and going with Element.classList.contains (for the specifics of the contains function, see here)

function randomColor() {
    let num= Math.floor(Math.random() * 255);
    return num;
}

let boxes = document.querySelectorAll('.box')

for (let i of boxes) {
    i.addEventListener('mouseover', () => {
        i.style['backgroundColor'] = 'black'
        i.style.border = '1px solid red'
    })
}

for (let i of boxes) {
    if (psych.classList.contains('active')) {
        i.addEventListener('mouseover', () => {
            i.style['backgroundColor'] = `rgb(${randomColor()},${randomColor()},${randomColor()})`
            i.style.border = `1px solid rgb(${randomColor()},${randomColor()},${randomColor()})`
        })
    }
}
Related