Function not repeating after the first event-CodePudding

I've been trying to learn how to work with HTML, CSS and JavaScript with no prior experience with a any sort of programming, and I'm trying some things out as I learn.

I'm trying to get a click event where multiple divs on a list (two in this case) to either lose a specific class if they have it, and gain said class if they don't, all on the same click.

The first time it's clicked, it runs properly, but how do I get it to do this on every click? I know there's easier ways to do this, but I really just want to experiment with this setup for now. If anybody has an idea on why it won't switch classes again, or any tips(or insults) whatsoever, please feel free.

Here's the code:

const SignList = document.querySelectorAll('.sign')

console.log(SignList)

SignList.forEach((div) => {
  div.addEventListener('click', () => {
    if (div.classList.contains('active')) {
      SignList.forEach((div) => {
        div.classList.remove('active');
        SignList.forEach((div) => {
          if (!div.classList.contains('active')) {
            div.classList.add('active')
          }
        })
      })
    }
  })
})

html {
  margin: 0px;
  padding: 0px;
  box-sizing: border-box;
  height: 100%;
}

body {
  display: flex;
  justify-content: center;
  align-items: center;
  height: 100%;
}

.sign {
  position: absolute;
  height: 100px;
  width: 100px;
  border: 5px solid black;
  border-radius: 20px;
}

.red {
  background-color: red;
  display: none;
}

.blue {
  background-color: blue;
  display: none;
}

.active {
  display: block;
}

<div ></div>
<div ></div>

CodePudding user response：

I feel like your question and code are a little confusing in terms of exactly what behavior you want from the click. Also, you probably shouldn't nest so much and reuse the same variable name [like div here] - instead, it might be better to break it down by defining a function.

If you want to toggle a certain class in each div when any div is clicked, you might change your js to something like this:

const SignList = document.querySelectorAll('.sign')
console.log(SignList)

function toggleClass(tClass) {
    //incase more div.sign might be added dynamically:
    const sList = document.querySelectorAll('.sign')

    sList.forEach((div) => {
        if (div.classList.contains(tClass)) {
            div.classList.remove(tClass);
        } else {
            div.classList.add(tClass);
        }
    })
}

SignList.forEach((div) => {
    div.addEventListener('click', () => { toggleClass('active') })
})

On the other hand, if you want to toggle that class only if the clicked div contain the class, you can change the above code to :

function toggleClass(clickTarget, tClass) {
    if (clickTarget.classList.contains(tClass)) {
        const sList = document.querySelectorAll('.sign');
        sList.forEach((div) => {
            if (div.classList.contains(tClass)) {
                div.classList.remove(tClass);
            } else {
                div.classList.add(tClass);
            }
        })
    }
}
SignList.forEach((div) => {
    div.addEventListener('click', (evt) => { toggleClass(evt.target, 'active') })
})

Then, it will only toggle if an "active" element is clicked on. It won't make a difference here, because the non-"active" elements are not visible, and there are only 2 elements affected, but if the class (for example) changed border color to green, only clicking on divs with green border would do anything....

(evt.target passes the object [that the event happened to] to the function in the listener and is more reliable than passing it as div like in your code. You could pass on the evt object itself to give the function more information about the event, but it's not necessary in this case.)

Hope this helps, and best of luck with your coding journey!

CodePudding user response：

You should iterate each of the .sign and toggle it. That's how the code now reflects that.

const SignList = document.querySelectorAll('.sign')

// console.log(SignList)

SignList.forEach((div) => {
  div.addEventListener('click', () => {

    SignList.forEach((div) => {
      if (div.classList.contains('active')) {
        div.classList.remove('active');
      } else {
        div.classList.add('active')
      }
    })
  })

})

html {
  margin: 0px;
  padding: 0px;
  box-sizing: border-box;
  height: 100%;
}

body {
  display: flex;
  justify-content: center;
  align-items: center;
  height: 100%;
}

.sign {
  position: absolute;
  height: 100px;
  width: 100px;
  border: 5px solid black;
  border-radius: 20px;
}

.red {
  background-color: red;
  display: none;
}

.blue {
  background-color: blue;
  display: none;
}

.active {
  display: block;
}

<body>

  <div ></div>
  <div ></div>


</body>