Debugging:
When running your code in Firefox, I get the following error:
TypeError: Argument 1 of Window.getComputedStyle does not implement
interface Element.
This is because button.nextSibling()
is a text node. Use nextElementSibling()
instead.
Suggestions:
I recommend attaching an "active" class to the dropdown itself instead of its content. The semantics are clearer and it provides you with more flexibility. E.g. you could style an active toggle button differently.
Also, instead of querying the computed display
style (slow!) of the button in order to infer whether a dropdown is active or inactive via
if (getComputedStyle(this.nextSibling).display === 'none') {
...
}
you could simply check for the dropdown's "active" class via
if (dropdown.classList.contains('.active') {
...
}
which is faster and agnostic to the specific CSS properties of an "active" dropdown button.
Additionally, instead of relying on a fixed order of dropdown toggle button and dropdown content by querying for the dropdown button first and then getting the content via nextSibling()
or nextElementSibling()
, I recommend querying for the dropdown
first and then separately querying its children for a dropdown-toggle
and the dropdown-content
.
Finally, I recommend simplifying the deeply nested structure of your code by moving the reusable deeply nested code into a toggleDropdown
function.
Style:
While some people consider the following idiomatic:
[].forEach.call(document.querySelectorAll('...'), function(element) {
...
});
I prefer using the simpler for ... of iteration:
for (let element of document.querySelectorAll('...')) {
...
}
Assigning the query results to a variable with a self-documenting name might improve readability:
let elements = document.querySelectorAll('...');
for (let element of elements) {
...
}
Enhancements:
You might want to listen to the "blur" event and deactivate the dropdown when its focus is transferred to an outside element.
function toggleDropdown(dropdown) {
if (dropdown.classList.contains('active')) {
dropdown.classList.remove('active');
} else {
let activeDropdowns = document.querySelectorAll('.dropdown.active');
for (let activeDropdown of activeDropdowns) {
activeDropdown.classList.remove('active');
}
dropdown.classList.add('active');
}
}
let dropdowns = document.querySelectorAll('.dropdown');
for (let dropdown of dropdowns) {
// Toggle when dropdown toggle clicked:
let toggle = dropdown.querySelector('.dropdown-toggle');
toggle.addEventListener('click', function() {
toggleDropdown(dropdown);
});
// Deactivate when focus leaves dropdown:
dropdown.addEventListener('blur', function(event) {
if (!this.contains(event.relatedTarget)) {
this.classList.remove('active')
}
}, true);
}
.dropdown {
display: inline-block;
}
.dropdown .dropdown-toggle {
background-color: #fff;
border: 0;
color: #424242;
cursor: pointer;
padding: 5px 0 5px 0;
}
.dropdown .dropdown-content {
display: none;
}
.dropdown.active .dropdown-content {
margin-top: 5px;
display: block;
background-color: #fff;
border: 1px solid #eee;
box-shadow: 0 0 2px 1px #dcdcdc;
position: absolute;
border-radius: 3px;
z-index: 10;
}
.dropdown .dropdown-content a {
display: block;
width: 100%;
text-decoration: none;
color: #3d3d3d;
padding: 4px;
box-sizing: border-box;
font-size: 0.9em;
}
.dropdown .dropdown-content a:hover {
background-color: #f0f0f0;
}
.dropdown.menu1,
.dropdown.menu2 {
margin: 0 0 0 20px;
}
.dropdown.menu1 .dropdown-toggle {
border-bottom: 2px solid #00BCD4;
}
.dropdown.menu2 .dropdown-toggle {
border-bottom: 2px solid #4CAF50;
}
.dropdown.menu3 .dropdown-toggle {
border-bottom: 2px solid #9FA8DA;
}
<div class="dropdown menu1">
<button class="dropdown-toggle">Menu 1</button>
<div class="dropdown-content">
<a href="#">Option 1</a>
<a href="#">Option 2</a>
</div>
</div>
<div class="dropdown menu2">
<button class="dropdown-toggle">Menu 2</button>
<div class="dropdown-content">
<a href="#">Option 1</a>
</div>
</div>
<div class="dropdown menu3">
<button class="dropdown-toggle">Menu 3</button>
<div class="dropdown-content">
<a href="#">Option 1</a>
<a href="#">Option 2</a>
<a href="#">Option 3</a>
</div>
</div>
Edit:
You could move the declaration of both event listeners out of the loop body. However, the dropdown toggle's 'click' event listener needs access to the dropdown element it belongs to. So you would still need a closure or alternatively bind the event listener's this
to the dropdown:
function onDropdownBlur(event) {
if (!this.contains(event.relatedTarget)) {
this.classList.remove('active')
}
}
function onDropdownToggle(event) {
toggleDropdown(this);
}
let dropdowns = document.querySelectorAll('.dropdown');
for (let dropdown of dropdowns) {
// Toggle when dropdown toggle clicked:
let toggle = dropdown.querySelector('.dropdown-toggle');
toggle.addEventListener('click', onDropdownToggle.bind(dropdown));
// Deactivate when focus leaves dropdown:
dropdown.addEventListener('blur', onDropdownBlur, true);
}