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")
    });
  });