How to use toggle function in JavaScript?

Question

So I am running into a couple of problems when I try to use JavaScript to toggle between classes on click.

What is going on.. Also when I run it on my localhost it runs but only on the first link. not on the second. I tried recreating the same problem but here it becomes even more a mess..

Now for some reason if I use this code even in the snippet, when you click the event happends, then dissapears completely.

var pill = document.querySelector(".navpill");
var sub = document.querySelector(".submenu");

pill.onclick = () => {
    sub.classList.toggle("collapse");
}

.mainmenu {
  background-color: #1f1f1f;
}
.navpill {
  padding: 15px;
}
.navpill a {
  text-decoration: none;
  color: white;
}
.submenu {
  display: none;
}
.submenu.collapse {
  display: block;
}

<div>
  <ul class="mainmenu">
    <li class="navpill"><a href="">Link collapse 1</a>
        <ul class="submenu">
            <li class="navpill"><a href="">sub Link 1</a></li>
            <li class="navpill"><a href="">sub Link 1</a></li>
            <li class="navpill"><a href="">sub Link 1</a></li>
            <li class="navpill"><a href="">sub Link 1</a></li>
        </ul>
    </li>
    <li class="navpill"><a href="">Link collapse 2</a>
        <ul class="submenu">
            <li class="navpill"><a href="">sub Link 1</a></li>
            <li class="navpill"><a href="">sub Link 1</a></li>
            <li class="navpill"><a href="">sub Link 1</a></li>
            <li class="navpill"><a href="">sub Link 1</a></li>
        </ul>
    </li>
    <li class="navpill"><a href="">no link</a></li>
    <li class="navpill"><a href="">no link</a></li>
  </ul>
</div>

ADyson · Accepted Answer · 2021-02-02 19:47:43Z

1

href="" makes it try to navigate to another page. Either try href="#" or just don't user hyperlinks if they're not actually intended to be real links. <li class="navpill">sub Link 1</li> ought to be just fine for you I'd expect.

Also, querySelector only selects the first element in the selected set. So it only acts on your first link. You need to use querySelectorAll, loop through the results and add a click to each.

Identifying the specific sublink to act on needs to be done within the click callback, so you can find the one within the menu item which has just been clicked. Also you can't use navpill as the class selector for handling the expand/collapse clicks, as it'll crash if you click on one of the sublinks with that class. So I added a new class for the outer, clickable items.

Demo:

var pills = document.querySelectorAll(".expand");

pills.forEach(function(pill) {
  pill.onclick = () => {
    var sub = pill.querySelector(".submenu");
    sub.classList.toggle("collapse");
  }
});

.mainmenu {
  background-color: #1f1f1f;
}

.navpill {
  padding: 15px;
  color: white;
}

.submenu {
  display: none;
}

.submenu.collapse {
  display: block;
}

<div>
  <ul class="mainmenu">
    <li class="navpill expand">Link collapse 1
      <ul class="submenu">
        <li class="navpill">sub Link 1</li>
        <li class="navpill">sub Link 1</li>
        <li class="navpill">sub Link 1</li>
        <li class="navpill">sub Link 1</li>
      </ul>
    </li>
    <li class="navpill expand">Link collapse 2
      <ul class="submenu">
        <li class="navpill">sub Link 1</li>
        <li class="navpill">sub Link 1</li>
        <li class="navpill">sub Link 1</li>
        <li class="navpill">sub Link 1</li>
      </ul>
    </li>
    <li class="navpill">no link</li>
    <li class="navpill">no link</li>
  </ul>
</div>

answered Feb 2, 2021 at 19:47

ADyson

62.9k16 gold badges85 silver badges99 bronze badges

Sign up to request clarification or add additional context in comments.

11 Comments

RaRa Ritalin Over a year ago

He Thanks for the answer. Worked great. however i run into the next problem. wich is i need to change more then just 1 variable. In your code that doesnt seem to work. ( or i dont know how )

ADyson Over a year ago

Sorry but I'm not really sure what you mean? Your code doesn't have any variables. Anyway...I answered the question as it was given. I think you should accept the answer. if you have a new and different problem, you should ask a new and different question about it :-)

RaRa Ritalin Over a year ago

I mean i have another class to add to a different object ( class ). You can see in the snippet. It shows the menu, and changes the background color of the UL. in your code it only toggles the submenu.

ADyson Over a year ago

You only inserted that after I wrote the answer. If you change the question substantially after people have written answers, that's not really on. People put time and effort into answering. In return they expect the odd upvote and accepted answer. They don't expect to spend their evening changing the answer because the person asking decided to move the goalposts after the fact. Revert your changes and accept this answer please. Like I said, if you have a new requirement or a new problem, you can ask a new question about it. Put a link to it here, and I might just take a look. Thanks.

ADyson Over a year ago

(For reference, stackoverflow.com/posts/66016490/revisions shows what time you added your new parts to the question, compared with when I - and others - wrote answers to your original question. If your original question didn't reflect your real situation...well that's not our fault.)

|

imvain2 · Accepted Answer · 2021-02-02 19:46:17Z

By using the same class for all of your nav, both main and sub you will need to change how you are calling your nav.

using QuerySelectorALL and > you can get just the main nav. Then using nextElementSibling you can get the next navigation.

I'm also looking for an already open menu and hiding it.

var pills = document.querySelectorAll(".mainmenu > .navpill");

pills.forEach((pill) => {
  pill.onclick = (e) => {
    sub = e.target.nextElementSibling;
    open = document.querySelector(".submenu.collapse")
    if(open) open.classList.remove("collapse");
    if(sub) sub.classList.toggle("collapse");
    e.preventDefault();
  }
});

.mainmenu {
  background-color: #1f1f1f;
}

.navpill {
  padding: 15px;
}

.navpill a {
  text-decoration: none;
  color: white;
}

.submenu {
  display: none;
}

.submenu.collapse {
  display: block;
}

<div>
  <ul class="mainmenu">
    <li class="navpill"><a href="">Link collapse 1</a>
      <ul class="submenu">
        <li class="navpill"><a href="">sub Link 1</a></li>
        <li class="navpill"><a href="">sub Link 1</a></li>
        <li class="navpill"><a href="">sub Link 1</a></li>
        <li class="navpill"><a href="">sub Link 1</a></li>
      </ul>
    </li>
    <li class="navpill"><a href="">Link collapse 2</a>
      <ul class="submenu">
        <li class="navpill"><a href="">sub Link 1</a></li>
        <li class="navpill"><a href="">sub Link 1</a></li>
        <li class="navpill"><a href="">sub Link 1</a></li>
        <li class="navpill"><a href="">sub Link 1</a></li>
      </ul>
    </li>
    <li class="navpill"><a href="">no link</a></li>
    <li class="navpill"><a href="">no link</a></li>
  </ul>
</div>

entitycs · Accepted Answer · 2021-02-02 21:26:08Z

querySelector selects just one element. Use querySelectorAll.

Then to limit the submenu items affected to just the ones under their parent menu item, query from that element as opposed to from the document. You'll also want to limit querySelectorAll to just choose elements with the navpill class which are direct children of the mainmenu class (done using the > symbol between selectors).

let pill = document.querySelectorAll(".mainmenu > .navpill");

pill.forEach ((element) => {
  let sub = element.querySelector(".submenu");
  let mainItem = element.firstChild;
  /* if you want your menu to collapse when clicking a submenu item
   * remove the line above, and on the line below, replace "mainItem"
   * with "element". */
  mainItem.onclick = (e) => {
    if (sub){
      sub.classList.toggle("collapse");
    }
  }
});

Note: Because your submenu items are encompassed in / children of your main menu items (per the selectors originally chosen), clicking on a submenu item will also set off the click event handler for the main menu item above it (the <li>). If each submenu item has a link, you'll likely not care - but clicking on a submenu item will also collapse the submenu. The comment in the code above explains how to obtain either behavior, defaulting with the option of leaving the submenu open unless explicitly closed by again clicking on the main menu item.

As far as the difference between href="#" vs href="", as others suggested, you can use <a> for your main menu items which are themselves links, and use something else, for example, <span> to hold your main menu items meant to open a submenu. That way, you don't send users to the top of your page any time they click a main menu item (using href="#"). You can then change your css for main item entries to the following, so that you get the same cursor behavior for both <a> and <span>:

.navpill span, .navpill a {
  text-decoration: none;
  color: white;
  cursor: pointer;
}

That's a quite understandable way to handle it. In the case, for example, another developer has to one day edit the site, they'd quickly understand your intentions and not get tripped up in e.preventDefault or other relatively obscure code.

Collectives™ on Stack Overflow

How to use toggle function in JavaScript?

3 Answers 3

11 Comments

Comments

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

11 Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related