1

I've been trying to make a dropdown button that it disactivates when clicking outside of it but I have not find good solution that has good performance, I am making the website with the Astro Framework

so this is an example of what I want it to be like: example

and this is what I have:my website

this is my code:


<nav
class="hidden items-center lg:relative  lg:mt-0 lg:!flex lg:basis-auto"
>
<!-- Desktop Navigation -->
<ul
class="list-style-none mr-auto flex flex-col pl-0 lg:flex-row"
>
    {navData.map(data => {
        if(data.dropdown === true) {
            return (
                <li data-title={data.title} class="sm:block relative dropdowns mx-2 lg:flex ">
                    <button data-title={data.title} class="dropdown text-sm rounded flex gap-1  items-center">
                        {data.title}
                        <svg viewBox="0 0 1024 1024" class="icon w-3" version="1.1" xmlns="http://www.w3.org/2000/svg" fill="currentColor"><g id="SVGRepo_bgCarrier" stroke-width="0"></g><g id="SVGRepo_tracerCarrier" stroke-linecap="round" stroke-linejoin="round"></g><g id="SVGRepo_iconCarrier"><path d="M903.232 256l56.768 50.432L512 768 64 306.432 120.768 256 512 659.072z" ></path></g></svg>
                    </button>
                    <div data-title={data.title} class="absolute right-0 top-7 mt-2 w-48 bg-gray-800 rounded-md overflow-hidden z-10 hidden">
                        {data.subcat.map(el => (
                            <a href={el.slug} class="block px-4 py-2 text-sm text-gray-200 hover:bg-gray-700">
                                {el.title}
                            </a>
                        ))}
                    </div>
                </li>
        )}
        else {
            return (
                    <li class="mb-4 pl-2 lg:mb-0 lg:pl-0 lg:pr-1" data-te-nav-item-ref>
                        <a
                        class="p-0 text-sm transition duration-200  hover:ease-in-out   motion-reduce:transition-none lg:px-2"
                        href={`${data.slug}`}
                        data-te-nav-link-ref
                        >
                            {data.title}
                        </a>
                    </li>
                )
            }
        })}
    </ul>
</nav>

const dropdownToggle = document.querySelectorAll('.dropdown');
const dropdownMenu = document.querySelectorAll('.dropdowns > div');


    dropdownToggle.forEach(button => button.addEventListener('click', () => {
            dropdownMenu.forEach((el) => {
                if(button.getAttribute('data-title') === el.getAttribute('data-title')){
                    el.classList.toggle('hidden')
                }
            });     
    }))

I've tried to use focus-within but didn't work as expected

.dropdowns:focus-within > div {
    display: block;
}

and I think this solution:

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

is bad for performance

nicoiscoding
  • 21
  • 3
  • You do not have to do a `.contains()` test before removing the class. – Pointy Aug 23 '23 at 13:04
  • Also unless your page has thousands of dropdown structures, that should not be any sort of performance issue. – Pointy Aug 23 '23 at 13:05
  • @Pointy the window.onclick solution I found it on Google but I thought it was bad for performance because it's executing code everytime there is a click,but how could I implement it for my code? – nicoiscoding Aug 23 '23 at 13:19
  • Are you actually *noticing* a performance problem? – Pointy Aug 23 '23 at 13:25
  • No, I said I Thought it had a performance problem but because I didn't know that it doesn't have a performance problem but now I found out how to do it, thanks – nicoiscoding Aug 23 '23 at 13:49
  • 1
    In your dropdownToggle.foreach you can get the parent "dropdowns" element via button.parentNode (ie button.parentNode.classList.toggle('hidden'). There is no need to search out the matching data-title (probably don’t need the data-title at all unless it’s displayed) – James Aug 23 '23 at 13:59
  • If you want good performance with anything DOM related I suggest you use a framework actually designed for component render performance. Make your dropdown a React or Preact component and use that in your Astro component. – user9993 Aug 23 '23 at 19:10

1 Answers1

0

It is not bad for performance, because you need a click handler that will notice whenever a dropdown is to be closed. You should not optimize what's not slow, because there are chances that you will overcomplicate your code with something that will be difficult to maintain and possibly slower too.

But, if you indeed notice some performance issues (like in the case when you have many thousands of dropdowns), then you can simply create some resource, let's call for the sake of simplicity currentDropdown and:

  • whenever a click occurs, if there was a previously set currentDropdown and that does not match to the dropdown you just opened (if you opened a dropdown), then collapse it
  • whenever a dropdown is opened, just set currentDropdown to it
  • whenever a click happened outside of any dropdown, make sure you do not forget to set currentDropdown to undefined
  • if you need to close a dropdown once an item inside of it is chosen, then also set currentDropdown to undefined

So, instead of looping all dropdowns, you will have a single dropdown to handle at most.

If you really really want to avoid having a click event outside the dropdowns when none of them are open (but I don't recommend this), then you can removeEventListener whenever you click outside of any dropdown so that clicks outside of dropdowns will not trigger event listeners when you do not need it. But, again, I do not recommend this, because it's code complication with virtually no gain. Avoiding the loop as described above should be enough.

Lajos Arpad
  • 64,414
  • 37
  • 100
  • 175