Dropdown menu that opens on click using plain Javascript

Question

I made a simple dropdown menu which opens when clicked, and closes when the user clicks anywhere outside the menu. The following is the codebase:

function deactivateAllDropdownTriggers() {
 document.querySelectorAll('.dropdown-trigger.active').forEach(function(elem) {
 elem.classList.remove('active');
 }) 
}
function handleDropdownClicks(event) {
 if (event.target.matches('.dropdown-trigger')) {
 if (event.target.classList.contains('active')) {
 event.target.classList.remove('active');
 } else {
 deactivateAllDropdownTriggers();
 event.target.classList.add('active');
 }
 } else {
 if (!event.target.matches('.dropdown-menu *')) {
 deactivateAllDropdownTriggers();
 }
 }
}
document.addEventListener('click', handleDropdownClicks, false);

.dropdown {
 display: inline-block;
 position: relative;
}
.dropdown .dropdown-menu {
 display: none;
 position: absolute;
 -moz-box-shadow: 0 2px 4px rgba(0, 0, 0, 0.15);
 -webkit-box-shadow: 0 2px 4px rgba(0, 0, 0, 0.15);
 box-shadow: 0 2px 4px rgba(0, 0, 0, 0.15);
 width: auto;
 border: 1px solid rgba(0, 0, 0, 0.15);
 border-radius: var(--border-radius);
 background-color: #ffffff;
 z-index: 1;
}
.dropdown .dropdown-menu ul {
 padding: 0;
 margin: 0;
}
.dropdown .dropdown-menu ul > li {
 list-style: none;
 margin: 0;
}
.dropdown .dropdown-menu ul > li.dropdown-menu-content {
 padding: 0.6rem 1.2rem;
 white-space: nowrap;
}
.dropdown .dropdown-menu ul > li.dropdown-menu-divider {
 height: 1px;
 background-color: rgba(0, 0, 0, 0.05);
}
.dropdown .dropdown-menu ul > li > a {
 display: block;
 text-decoration: none;
 padding: 0.6rem 1.2rem;
 color: rgba(0, 0, 0, 0.8);
 cursor: pointer;
 white-space: nowrap;
 min-width: 12rem;
}
.dropdown .dropdown-menu ul > li > a:hover {
 background-color: rgba(0, 0, 0, 0.025);
}
.dropdown .dropdown-trigger.on-click.active + .dropdown-menu {
 display: block;
}

<div class="dropdown">
 <button class="dropdown-trigger on-click">Pick your weapon</button>
 <div class="dropdown-menu">
 <ul>
 <li class="dropdown-menu-content">
 Weapons
 </li>
 <li class="dropdown-menu-divider"></li>
 <li><a href="#">Sword</a></li>
 <li><a href="#">Lance</a></li>
 <li><a href="#">Axe</a></li>
 <li><a href="#">Bow</a></li>
 </ul>
 </div>
</div>
<div class="dropdown">
 <button class="dropdown-trigger on-click">Pick your class</button>
 <div class="dropdown-menu">
 <ul>
 <li class="dropdown-menu-content">
 Classes
 </li>
 <li class="dropdown-menu-divider"></li>
 <li><a href="#">Fighter</a></li>
 <li><a href="#">Archer</a></li>
 <li><a href="#">Thief</a></li>
 <li><a href="#">Ninja</a></li>
 </ul>
 </div>
</div>

One thing that concerns me is the open event listener. I could not create a system where an event listener is added to the document when a menu is opened, and the event listener is removed when the menu is closed. Perhaps one of you could help me with that.

Anyway, would appreciate a review of this code.

Answer 1

Overall the code looks fine. I just have a couple suggestions about the JavaScript and CSS.

JS

querySelectorAll vs getElementsByClassName

Generally document.getElementsByClassName will be quicker than document.querySelectorAll (see this post for more information) and the former also returns a live HTMLCollection so it wouldn't need to be queried each time. Bearing in mind that deactivateAllDropdownTriggers() looks for elements with both class names dropdown-trigger and active, only the latter is really needed. If active applies to other elements, then perhaps a name like active-dropdown would help narrow down elements. In order to iterate over the items in that collection, they would need to be put into an array - that can be done with the spread operator ...

const activeElements = document.getElementsByClassName('active');
function deactivateAllDropdownTriggers() {
 [...activeElements].forEach(elem => elem.classList.remove('active'));
}

CSS

Useless class on-click

It appears that the on-click class is only utilized in the last selector (i.e. .dropdown .dropdown-trigger.on-click.active + .dropdown-menu) but that class name could be removed since it doesn't appear to be used anywhere else

default values

Some rules set values to what should be the default values - e.g. margin: 0 for the unordered list and list items, and cursor: pointer for the anchors. Those shouldn't be needed unless there are other styles matching the selectors that would add different values - e.g. for other page contents or a browser/plugin stylesheet

Answer 2

Instead of adding the event listener to the document element you could add it to every element that has a class of dropdown-trigger. Then you could make use of javascripts classList toggle function. That way you don't need the if else statement in handleDropdownClicks.

Example:

const dropdownTriggers = document.querySelectorAll(".dropdown-trigger")
 dropdownTriggers.forEach( item => {
 item.addEventListener("click", () => {
 item.classList.toggle("active")
 });
 });

• darkhorse: Your solution would not let the users close the dropdown menu by clicking outside of the dropdown menu though.
• Moglash: Your absolutely right. Sorry, I overlooked it...