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>
2
Answers
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 additionalclick
event listener to thelevelTab
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
HTML
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.Setting
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 renderlevelContain
in normal fashion.With care you might be able to use the text string
"0px"
instead ofnull
, but a more common method would be to toggle thedisplay
property of the container betweennone
and its default state:Note you could set the initial display of level containers to "none" in HTML and/or CSS if you prefer.