0

so i'm a bit confused as why the code im working off isn't working as expected, so any help would be good.

so let me explain im working on an expanding/ collapsing menu, when the a tag is clicked a class is added and css is added to make this happen, and also the class is removed from the other links.

CSS code:

a {
max-height: 0;
overflow: hidden;
transition: 0.5s ease-in-out
}
a.clicked {
max-height: 600px;
}

html code:

<ul>

<li><a onclick='mobileMenu(event)'>link 1</a><li>

<li><a onclick='mobileMenu(event)'>link 2</a><li>

<li><a onclick='mobileMenu(event)'>link 3</a><li>

<li><a onclick='mobileMenu(event)'>link 4</a><li>

</ul>

for this post i kept the html pretty simple but if it needs to be more detailed let me know.

Each link has a onclick attached.

Javascript code:

function mobileMenu(event) {

        event.currentTarget.classList.toggle("clicked");

        [].forEach.call(document.querySelectorAll('ul li a'), function(a){

            a.classList.remove('clicked');

        });

}

so this code mostly works where you click a link is adds 'clicked' class and also removes the 'clicked' class from any other links except the current link, which is good because im using this as a collapse-able menu so you click another link it opens it while also closing any other link currently open.

My problem is is the js code above to add the clicked class i'm using a '.toggle' but this only adds the 'clicked' class but does not toggle it. I want to be able to toggle the 'clicked' class and also remove it from others links. so im not sure if its a simple oversight on my part but im not sure where im going wrong and why the '.toggle' isn't actually toggling the class.

Thanks.

ddd
  • 79
  • 1
  • 1
  • 8

2 Answers2

2

As per comment the right way to add event listeners is using .addEventListener().

In the event listener you can search for an anchor with a class clicked and if any remove the class. After you can add the class to the current element:

document.querySelectorAll('ul li a').forEach(function(ele, idx) {
    ele.addEventListener('click', function(e) {
        var clickedEle = document.querySelector('ul li a.clicked');
        if (clickedEle != null)
            clickedEle.classList.remove('clicked');
        this.classList.add('clicked');
    })
});
a {
    max-height: 0;
    overflow: hidden;
    transition: 0.5s ease-in-out
}
a.clicked {
    max-height: 600px;
    color: blue;
}
<ul>
    <li><a>link 1</a><li>
    <li><a>link 2</a><li>
    <li><a>link 3</a><li>
    <li><a>link 4</a><li>
</ul>
gaetanoM
  • 41,594
  • 6
  • 42
  • 61
  • Also, I'll add that to keep your list dynamic, you might not want to set a hard value for max-height in the 'clicked' class. Any part of the list added that is longer than 600px will be cut off. That is why it's better to set the maxHeight's value to the event's scroll height using Javascript. – rguttersohn Sep 01 '20 at 12:51
2

You can try a if statement. It will always remove the checked from all links, and if the event.target is not checked, it will add the class checked.

function mobileMenu(event) {
    
  const checked = event.currentTarget.classList.contains('clicked')
  
  [].forEach.call(document.querySelectorAll('ul li a'), function(a){
    
    a.classList.remove('clicked');
    
  });

  if (checked) return;
  
  event.currentTarget.classList.toggle("clicked");
}