Accordion class not removing correctly with javascript when object is closed-CodePudding

Not the best with javascript and I'm currently stuck on something. I've managed to put an accordion together that expands and collapses with an :after icon that transforms when the object is opened. It also only allows for one accordion to be open at a time which is what is needed.

My issue is that the :after icon is not functioning correctly because once the "active" class is added, it only gets removed if another object is opened, not if the same object is closed. Does anyone have a solution that could fix this? I need the class to be removed if another accordion is opened, as well as if the object is clicked again to close it.

As mentioned above I'm not the best with JS and i've really only achieved what I have with this accordion from other answers I've found on here. Here is my current code. Any help is greatly appreciated!

Javascript

var acc = document.getElementsByClassName("accordion");
var i;

for (i = 0; i < acc.length; i  ) {
acc[i].addEventListener("click", function() {

var panel = this.nextElementSibling;
if (panel.style.maxHeight){
  panel.style.maxHeight = null;
} else {
  let active = document.querySelectorAll(".accordion-div .accordion.active");
  for(let j = 0; j < active.length; j  ){
    active[j].classList.remove("active");
    active[j].nextElementSibling.style.maxHeight = null;
  }
  this.classList.toggle("active");
  panel.style.maxHeight = panel.scrollHeight   "px";
}
});
}

HTML

<div class="accordion-div">
<h5 class="accordion">Heading</h5>
<div class="panel">
<p>Some Text</p>
<h5 class="accordion">Heading 2</h5>
<div class="panel">
<p>Some Text</p>
<h5 class="accordion">Heading 3</h5>
<div class="panel">
<p>Some Text</p>
</div>
</div>

CSS

h5.accordion {
  cursor: pointer;
  padding: 25px!important;
  padding-left:2px!important;
  width: 100%;
  line-height:13px;
  text-align: left;
  outline: none;
  transition: 0.4s;
  font-size:13px!important;
  vertical-align:center;
  font-family:"Avenir Next", sans-serif;
  font-weight:300;
  margin:0;
  margin-top:2px;
  border-bottom:0!important;
}

h5.accordion:after {
  content: '\002B';
  line-height:13px;
  font-size:22px;
  float: right;
  color:#5f5f5f;
  margin-left: 5px;
  transition: transform .5s;
  transform-origin: 50% 60%;
}

.active:after {
  transform: rotate(-225deg);
}

.panel {
  padding: 0 2px;
  max-height: 0;
  overflow: hidden;
  transition: max-height 0.2s ease-out;
  border-bottom:solid 1px #f1f1f1;
}

CodePudding user response：

There was a typo that you were not closing div for your headings in your HTML please check the snippet, Also to open and close, I have added a line of JS to first remove the active class if present and then toggle based on this

Please run the snippet and see

var acc = document.getElementsByClassName("accordion");
var i;

for (i = 0; i < acc.length; i  ) {
acc[i].addEventListener("click", function() {
this.classList.remove("active");
var panel = this.nextElementSibling;
if (panel.style.maxHeight){
  panel.style.maxHeight = null;
} else {
  let active = document.querySelectorAll(".accordion-div .accordion.active");
  for(let j = 0; j < active.length; j  ){
    active[j].classList.remove("active");
    active[j].nextElementSibling.style.maxHeight = null;
  }
  this.classList.toggle("active");
  panel.style.maxHeight = panel.scrollHeight   "px";
}
});
}

h5.accordion {
  cursor: pointer;
  padding: 25px!important;
  padding-left:2px!important;
  width: 100%;
  line-height:13px;
  text-align: left;
  outline: none;
  transition: 0.4s;
  font-size:13px!important;
  vertical-align:center;
  font-family:"Avenir Next", sans-serif;
  font-weight:300;
  margin:0;
  margin-top:2px;
  border-bottom:0!important;
}

h5.accordion:after {
  content: '\002B';
  line-height:13px;
  font-size:22px;
  float: right;
  color:#5f5f5f;
  margin-left: 5px;
  transition: transform .5s;
  transform-origin: 50% 60%;
}

.active:after {
  transform: rotate(-225deg);
}

.panel {
  padding: 0 2px;
  max-height: 0;
  overflow: hidden;
  transition: max-height 0.2s ease-out;
  border-bottom:solid 1px #f1f1f1;
}
javascript
jquery

<div class="accordion-div">
<h5 class="accordion">Heading</h5>
<div class="panel">
<p>Some Text</p>
</div>
<h5 class="accordion">Heading 2</h5>
<div class="panel">
<p>Some Text</p>
</div>
<h5 class="accordion">Heading 3</h5>
<div class="panel">
<p>Some Text</p>
</div>
</div>

CodePudding user response：

First, remove all active classes when the h5 tag is clicked

const removeActiveClasses = (e) => {
    const target = e.targrt || e.currentTarget;
    const active = target.querySelector(".accordion.active");
    active.classList.remove("active");
}

Then you need to activate the current element

const activeElement = (e) => {
    const target = e.target.nextElementSibling;
    target.classList.add("active");
}

At the End

const accordion = e => {
    removeActiveClasses(e);
    activeElement(e);
}

for (i = 0; i < acc.length; i  ) {
    acc[i].addEventListener("click", accordion);
}

Notice: You do not need to use a loop because you only want one active element