hide open/active accordion when another accordion is clicked-CodePudding

I suspect the issue is with the line const active = document.querySelector(".accordion.active"); in JS code below. It doesn't seem to be retrieving that element. Could you please help me debug it? Or should I use something else instead of querySelector? It is also found that this.classList.add("active"); is not adding the "active class" to the accordion element when it is clicked.

document.addEventListener("DOMContentLoaded", function(event) {
  var acc = document.getElementsByClassName("accordion");
  var i;
  for (i = 0; i < acc.length; i  ) {
    acc[i].addEventListener("click", function() {
      const active = document.querySelector(".accordion.active");
      console.log(active);
      if (active) {
        active.classList.remove('active'); // remove active class from accordions
      }
      this.classList.add("active"); // add it to this one
      
      this.classList.toggle("active");
      var panel = this.nextElementSibling;
      if (panel.style.display === "block") {
        panel.style.display = "none";
      } else {
        panel.style.display = "block";
      }
    });
  }
});

.accordion {
  background-color: #eee;
  color: #444;
  cursor: pointer;
  padding: 1.5rem;
  width: 100%;
  border: none;
  text-align: left;
  outline: none;
  font-size: 15px;
  transition: 0.4s;
}

.active,
.accordion:hover {
  background-color: #ccc;
}

.panel {
  padding: 0 18px;
  display: none;
  background-color: white;
  overflow: hidden;
  background: red;
}

.active .panel {
  display: block;
}

<div >
  <button ><div ><?php echo $label; ?></div></button>
  <div >
    <p >
      <?php echo $answer; ?>
    </p>
    </br>
  </div>
</div>
<div >
  <button ><div ><?php echo $label; ?></div></button>
  <div >
    <p >
      <?php echo $answer; ?>
    </p>
    </br>
  </div>
</div>

CodePudding user response：

You don't need to manipulate the display of the active element, your CSS already does that. Also you should not both add and toggle the active class on this - that is equivalent to removing it.

I've also added an if statement to check if the clicked element is already active so that collapsing it again works.

document.addEventListener("DOMContentLoaded", function(event) {
  var acc = document.getElementsByClassName("accordion");
  var i;
  for (i = 0; i < acc.length; i  ) {
    acc[i].addEventListener("click", function() {
      const active = document.querySelector(".accordion.active");
      console.log(active);
      if (active) {
        active.classList.remove('active'); // remove active class from accordions
      }

      if (active !== this) {
        this.classList.toggle("active");
      }
    });
  }
});

.accordion {
  background-color: #eee;
  color: #444;
  cursor: pointer;
  padding: 1.5rem;
  width: 100%;
  border: none;
  text-align: left;
  outline: none;
  font-size: 15px;
  transition: 0.4s;
}

.active,
.accordion:hover {
  background-color: #ccc;
}

.panel {
  padding: 0 18px;
  display: none;
  background-color: white;
  overflow: hidden;
  background: red;
}

.active .panel {
  display: block;
}

<div >
  <button ><div >Label1</div></button>
  <div >
    <p >
      Answer goes here
    </p>
  </div>
</div>
<div >
  <button ><div >Label2</div></button>
  <div >
    <p >
      Second answer
    </p>
  </div>
</div>

CodePudding user response：

I always prefer delegation

document.addEventListener("DOMContentLoaded", function(event) {
  const acc = document.getElementById("accodionContainer");
  acc.addEventListener("click", function(e) {
    const currentButton = e.target.closest(".accordion");
    if (!currentButton) return // something else clicked
    acc.querySelectorAll(".accordion.active").forEach(acc => {
      if (acc != currentButton) {
        acc.classList.remove("active");
        acc.nextElementSibling.style.display = "none"
      }
    });
    currentButton.classList.toggle("active"); // add or remove it to this one
    const show = currentButton.classList.contains("active");
    const panel = currentButton.nextElementSibling;
    panel.style.display = show ? "block" : "none";
  });
});

.accordion {
  background-color: #eee;
  color: #444;
  cursor: pointer;
  padding: 1.5rem;
  width: 100%;
  border: none;
  text-align: left;
  outline: none;
  font-size: 15px;
  transition: 0.4s;
}

.active,
.accordion:hover {
  background-color: #ccc;
}

.panel {
  padding: 0 18px;
  display: none;
  background-color: white;
  overflow: hidden;
}

.active .panel {
  display: block;
}

.accordion.active span::after { content: "➖"; }
.accordion span::after { content: "➕"; }

<div id="accodionContainer">
  <div >
    <button ><span></span> Question 1</button>
    <div >
      <p >
        Answer 1
      </p>
    </div>
  </div>
  <div >
    <button ><span></span> Question 2</button>
    <div >
      <p >
        Answer 2
      </p>
    </div>
  </div>
</div>

CodePudding user response：

this.classList.add("active"); is not adding the "active class" to the accordion element when it is clicked.

It is adding. But immediately the class is toggled, so it is removed. That toogle class line is commented.

I have added the css for green color to active accordion, which you can see after moving the cursor off the accordion element.

document.addEventListener("DOMContentLoaded", function(event) {
  var acc = document.getElementsByClassName("accordion");
  var i;
  for (i = 0; i < acc.length; i  ) {
    acc[i].addEventListener("click", function() {
      const active = document.querySelector(".accordion.active");
      console.log(active);
      if (active) {
        active.classList.remove('active'); // remove active class from accordions
        // this is if other heading is clicked
      }
      
      
     // this.classList.toggle("active");  // not needed
      var panel = this.nextElementSibling;
      if (panel.style.display === "block") {
      this.classList.remove("active"); // remove it to this one
        panel.style.display = "none";
      } else {
      this.classList.add("active"); // add it to this one
        panel.style.display = "block";
      }
    });
  }
});

.accordion {
  background-color: #eee;
  color: #444;
  cursor: pointer;
  padding: 1.5rem;
  width: 100%;
  border: none;
  text-align: left;
  outline: none;
  font-size: 15px;
  transition: 0.4s;
}
.accordion.active{
  background:green;
}

.active,
.accordion:hover {
  background-color: #ccc;
}

.panel {
  padding: 0 18px;
  display: none;
  background-color: white;
  overflow: hidden;
  background: red;
}

.active .panel {
  display: block;
}

<div >
  <button ><div >Accordion Label 1</div></button>
  <div >
    <p >
      Accordion Answer 1
    </p>
    </br>
  </div>
</div>
<div >
  <button ><div >Accordion Label 2</div></button>
  <div >
    <p >
      Accordion Answer 1
    </p>
    </br>
  </div>
</div>

CodePudding user response：

The below answer is not related to the question, but it is good to have the html hierarchy.

const accordionToggles = document.querySelectorAll('.accordion-toggle');
const accordionItems = document.querySelectorAll('.accordion-item');
const flush = true; // set to true,if only one accordion bodyto be open. set false, to open multiple accordion-body
accordionToggles.forEach((toggleBtn)=>{
   toggleBtn.addEventListener('click', (event)=>{
   const accordionItem = event.target.closest(".accordion-item");
   if(!flush){
   accordionItem.classList.toggle('active');
   return;
   }
   if(accordionItem.classList.contains('active')){
   accordionItem.classList.toggle('active');
   return;
   }
   accordionItems.forEach((item)=>{
      item.classList.remove('active');
   })
   accordionItem.classList.add('active');
   })
   
});

.accordion{

}

.accordion-item{
border:1px solid gray;
}
.accordion-item .accordion-item{
border-top: none;
}
.accordion-header{

}

.accordion-toggle{
   width:100%;
   border:none;
   height:30px;
   border-bottom: 1px solid black;
   text-align: left;
   position:relative;
}
.accordion-toggle:after{
  position:absolute;
  top:5px;
  right: 5px;
  font-size: 18px;
  }

.accordion-body{
  padding:10px;
  display:none;
}

.accordion-item.active .accordion-body{
display:block;
}

.accordion-item .accordion-toggle:after{
  content: " "
}
.accordion-item.active .accordion-toggle:after{
  content: "-"
}

<div >
  <div >
    <div >
     <button >Title 1</button>
    </div>
    <div >
      Accordion Content 1
    </div>
  </div>
  <div >
    <div >
     <button >Title 2</button>
    </div>
    <div >
      Accordion Content 2
    </div>
  </div>
</div>