Dropdown with Javascript solution

Question

Hello all I have a problem when I use two dropdowns, when you click on one, both are opened, below is my code:

function myFunction() {
    document.getElementById("myDropdown").classList.toggle("show");
}

window.onclick = function (event) {
    if (!event.target.matches('.dropbtn')) {
        var dropdowns = document.getElementsByClassName("dropdown-content");
        var i;
        for (i = 0; i < dropdowns.length; i++) {
            var openDropdown = dropdowns[i];
            if (openDropdown.classList.contains('show')) {
                openDropdown.classList.remove('show');
            }
        }
    }
}

function newFunction() {
    document.getElementById("myDropdown1").classList.toggle("show");
}

window.onclick = function (event) {
    if (!event.target.matches('.dropbtn')) {
        var dropdowns = document.getElementsByClassName("dropdown-content");
        var i;
        for (i = 0; i < dropdowns.length; i++) {
            var openDropdown = dropdowns[i];
            if (openDropdown.classList.contains('show')) {
                openDropdown.classList.remove('show');
            }
        }
    }
}

.dropbtn {
    background-color: #ffffff;
    color: #252930;
    font-size: 18px;
    border: none;
    cursor: pointer;
}

.dropdown {
    position: relative;
    display: inline-block;
}

.dropbtn-selected {
    background-color: #ec292d;
    color: #fff!important
}

.dropdown-content {
    display: none;
    position: absolute;
    background-color: #f1f1f1;
    min-width: max-content;
    overflow: auto;
    margin-top: 15px;
    box-shadow: 0px 8px 16px 0px rgba(0, 0, 0, 0.2);
    z-index: 969999;
}

.dropdown-content a {
    color: black;
    padding: 12px 16px;
    text-decoration: none;
    display: block;
}

.dropdown a:hover {
    background-color: #ddd;
}

.show {
    display: block;
}

.dropbtn>i {
    pointer-events: none;
}

<li class="dropdown desktopNav">
    <button onclick="myFunction()" class="dropbtn">Test <i
            class="fas fa-chevron-down"></i></button>
    <div id="myDropdown" class="dropdown-content">
        <a href="/test1.html">Lorem1 </a>
        <a href="/test2.html">Lorem 2</a>
        <a href="/test3.html">Lorem 3</a>
       
    </div>
</li>

<li class="dropdown desktopNav">
    <button onclick="newFunction()" class="dropbtn">Second <i
            class="fas fa-chevron-down"></i></button>
    <div id="myDropdown1" class="dropdown-content">
        <a href="/test1.html">Lorem 4 </a>
        <a href="/test2.html">Lorem 5</a>
        <a href="/test3.html">Lorem 6</a>
       
    </div>
</li>

Is there a way to make this code more flexible? I want to achieve, when the user clicks on the dropdown that dropdown should be only opened,.

Can somebody try to help me with this? What is the best way to achieve this?

Answer 1

To have only one dropdown visible at a time you need to hide all others. The simple way to achieve this is to group the element structures by classname based on their behaviour, as opposed to giving each block its own incremental id. The latter approach leads to bloated duplicate code which becomes harder to maintain.

To fix this remove the id and give all the elements in the repeated blocks the same class. From there you can use DOM traversal to relate the elements to each other, hiding and displaying as necessary.

let dropdownContents = document.querySelectorAll('.dropdown-content');

// display the dropdown relevant to the clicked button
document.querySelectorAll('.dropbtn').forEach(btn => {
  btn.addEventListener('click', e => {
    let targetContent = e.target.parentNode.querySelector(".dropdown-content");
    targetContent.classList.toggle("show");
    dropdownContents.forEach(el => el !== targetContent && el.classList.remove('show'));
  });
});

// hide all dropdowns when the document is clicked
document.addEventListener('click', e => {
  if (!e.target.matches('.dropbtn'))
    dropdownContents.forEach(el => el.classList.remove('show'));
})

.dropbtn {
  background-color: #ffffff;
  color: #252930;
  font-size: 18px;
  border: none;
  cursor: pointer;
}

.dropdown {
  position: relative;
  display: inline-block;
}

.dropbtn-selected {
  background-color: #ec292d;
  color: #fff!important
}

.dropdown-content {
  display: none;
  position: absolute;
  background-color: #f1f1f1;
  min-width: max-content;
  overflow: auto;
  margin-top: 15px;
  box-shadow: 0px 8px 16px 0px rgba(0, 0, 0, 0.2);
  z-index: 969999;
}

.dropdown-content a {
  color: black;
  padding: 12px 16px;
  text-decoration: none;
  display: block;
}

.dropdown a:hover {
  background-color: #ddd;
}

.show {
  display: block;
}

.dropbtn>i {
  pointer-events: none;
}

<ul>
  <li class="dropdown desktopNav">
    <button class="dropbtn">Test <i class="fas fa-chevron-down"></i></button>
    <div class="dropdown-content">
      <a href="/test1.html">Lorem1 </a>
      <a href="/test2.html">Lorem 2</a>
      <a href="/test3.html">Lorem 3</a>
    </div>
  </li>
  <li class="dropdown desktopNav">
    <button class="dropbtn">Second <i class="fas fa-chevron-down"></i></button>
    <div class="dropdown-content">
      <a href="/test1.html">Lorem 4 </a>
      <a href="/test2.html">Lorem 5</a>
      <a href="/test3.html">Lorem 6</a>
    </div>
  </li>
</ul>

Note the use of addEventListener over the outdated onX properties, and also the click handler which hides the dropdowns being attached to the document instead of the window.

Answer 2

There is no seperate 'close' event, so when clicking the second, there's no reason why the first should close.

You could have a variable that is set when a dropdown is to be shown. When clicking the second dropdown, the function would check if the variable is set, and if so, toggle that one off before toggling the correct one on. Eg:

let dropdownShown = null;

function onClick(e) {
  if (dropdownShown !== e.target) {
    dropdownShown.classList.toggle("show");
  }
  e.target.classList.toggle("show");
  dropdownShown = e.target;
}