How do I trigger onclick event with only one click?

Question

It's me again! I've been slaving to get an accordion menu to work properly and I managed to get it to work... the main problem is, it only triggers on the double click of a button.

I want to make this so the user only needs to click on the button once. I have other usages of buttons in my code that work with only one click and this one does follow their formats, but I don't know whats making it trigger only on double click.

Here is the Javascript -- I could only make it work if i put it in a function(), just leaving out the function would not allow the script to work.

function accordion() {

const acc = document.getElementsByClassName("levelTab");
var a;

for (a = 0; a < acc.length; a++) {
    acc[a].addEventListener('click', function() {
        this.classList.toggle('active');
        var levelContain = this.nextElementSibling;
        if (levelContain.style.maxHeight) {
            levelContain.style.maxHeight = null;
        } else {
            levelContain.style.maxHeight = levelContain.scrollHeight + "px";
        }
    });
};
}

And the HTML

<button onclick="accordion()" class="levelTab">Cantrips</button>
                <div class="levelContain">
                    <p>
                        insert Spells here.
                    </p>
                </div>

Answer 1

The issue here seems to be with the way you're attaching the onclick event to your button.

You're invoking the accordion() function on a button click, and then within that function, you're attaching an additional click event listener to the levelTab class. This means that the first time you click the button, the event listener is added. On the second click, the event listener's action is executed.

You can use windows.onload to only add the event listeners when the page loads the first time.

Example on how you can fix:

Javascript

window.onload = function() {
    const acc = document.getElementsByClassName("levelTab");
    Array.from(acc).forEach((element) => {
        element.onclick = function() {
            this.classList.toggle('active');
            var levelContain = this.nextElementSibling;

            if (levelContain.style.maxHeight) {
                levelContain.style.maxHeight = null;
            } else {
                levelContain.style.maxHeight = levelContain.scrollHeight + "px";
            }
        }
    });
}

HTML

<button class="levelTab">Cantrips</button>
<div class="levelContain">
    <p>
        Insert Spells here.
    </p>
</div>

Answer 2

1. Event listeners are executed on every click. When added with addEventListener however, adding a new listener does not automatically remove previoiusly added listeners. When the total number of listeners is even they cancel each other out. When odd, the cumulative result should toggle the container - the reason the post says two clicks are needed.

The best solution to this is to only add the listeners once. I.E. don't use code that adds the listeners as the click handler. Although using element.onclick = handlerFunction does remove a previous click handler, but only if it was added the same way, I wouldn't recommend setting the buttons' click handlers every time they are clicked.

2. Setting

    levelContain.style.maxHeight = null;
   

will not create a toggle - it has the same effect as removing the maxHeight property from the element's style property, hence letting the browser render levelContain in normal fashion.

With care you might be able to use the text string "0px" instead of null, but a more common method would be to toggle the display property of the container between none and its default state:

 // execute after HTML for buttons has been parsed

const acc = document.getElementsByClassName("levelTab");

for (let a = 0; a < acc.length; a++) {
  const button = acc[a];
  const levelContain = button.nextElementSibling;
  levelContain.style.display = "none";
  button.addEventListener('click', function() {
    this.classList.toggle('active');
    const levelContain = this.nextElementSibling;
    if (levelContain.style.display == "none") {
      levelContain.style.display = "revert";
    } else {
      levelContain.style.display = "none";
    }
  });
}

.active { color:green}

<button class="levelTab">Cantrips</button>
  <div class="levelContain">
     <p>
       insert Spells here.
     </p>
  </div>

Note you could set the initial display of level containers to "none" in HTML and/or CSS if you prefer.