Slider won't close

Question

I have the following:

// Menu
function openNav() {
  document.querySelector("div.slider").style.width = "250px";
    document.querySelector("div.slider").style.boxShadow = "-2px 0px 5px -1px rgba(0,0,0,0.75)";
}
function closeNav() {
  document.querySelector("div.slider").style.width = "0";
    document.querySelector("div.slider").style.boxShadow = "";
}

<nav>
            <a href="index.html"><div class="logo"></div></a>
            <div class="menu" onclick="openNav()">
                <div class="slider">
                    <a href='#Home' onclick="closeNav()">Home</a>
                    <a href="#Services" onclick="closeNav()">Services</a>
                    <a href="#Portfolio" onclick="closeNav()">Portfolio</a>
                    <a href="#Team" onclick="closeNav()">Team</a>
                    <a href="#Contact" onclick="closeNav()">Contact</a>
                </div>
            </div>
        </nav>

The slider will not close. I have also tried setting it up so that if I click outside of the div it will close but it isn't working. If I run 'closeNav()' in the console the nav closes fine but I can't seem to get the onclick to work.

Clicking on any link within `.menu` is still firing a click event on `.menu` which means your `openNav` code overrides your `closeNav` code. — zzzzBov, May 27 '20 at 17:32

Balastrong · Accepted Answer · 2020-05-27T17:34:20.147

What's wrong in your code is that you're calling both method at the same time. When you click a menu item, both closeNav and openNav are called in this order, hence you'll always see it open. You can notice by adding a console.log("open") and console.log("close") in your methods. This happens because if you click on a menu item, you're also clicking the menu behind :)

You should consider using a CSS class and classList.toggle("open-nav"). See the example below:

function toggleNav() {
  document.querySelector("div.slider").classList.toggle("open-nav");
}

.open-nav {
  width: 250px;
  box-shadow: -2px 0px 5px -1px rgba(0,0,0,0.75);
}

<nav>
            <a href="index.html"><div class="logo"></div></a>
            <div class="menu" onclick="toggleNav()">
                <div class="slider">
                    <a href='#Home'>Home</a>
                    <a href="#Services">Services</a>
                    <a href="#Portfolio">Portfolio</a>
                    <a href="#Team">Team</a>
                    <a href="#Contact">Contact</a>
                </div>
            </div>
        </nav>

Scott Marcus · Answer 2 · 2020-05-27T17:44:41.897

Events propagate or "bubble" so when you click a link, you call the closeNav function, but then that click event bubbles up to the parent menu where openNav is called.

A few things to rework here:

Don't do your event handling with HTML attributes. Separate your event handling into JavaScript.
Don't set inline styles if you can help it, they become difficult to override later. Separate your CSS from your HTML.
You only need one function here. Start with the menu items hidden as their default styling and then just toggle that as needed with a toggle function.
You can leverage event bubbling by doing event delegation. Just set your event handler on the parent of all the possible clicked items and let the event bubble up to that parent. Then check the event.target to see if the source was something that you need to act upon.

// Get your DOM references just once
let slider = document.querySelector("div.slider");

// Do all your event binding in JavaScript, not with HTML attributes
document.querySelector("div.menu").addEventListener("click", toggleNav);
slider.addEventListener("click", toggleNav);

function toggleNav(event) {
  // Check to see if the click came from a link or the menu
  if(event.target.classList.contains("link")){
    slider.classList.add("hidden");  // Link: reinstate the hidden class
  } else {
    slider.classList.remove("hidden"); // Menu: remove the hidden class
  }
}

.slider {
  width:250px;
  box-shadow: -2px 0px 5px -1px rgba(0,0,0,0.75)
}
.hidden { display:none; }

<nav>
  <a href="index.html"><div class="logo"></div></a>
  <div class="menu">MENU
     <div class="slider hidden">
        <a class="link" href='#Home'>Home</a>
        <a class="link" href="#Services">Services</a>
        <a class="link" href="#Portfolio">Portfolio</a>
        <a class="link" href="#Team">Team</a>
        <a class="link" href="#Contact">Contact</a>
     </div>
  </div>
</nav>

With this I receive the following error `script.js:10 Uncaught TypeError: Cannot read property 'addEventListener' of null` — Mac, May 27 '20 at 18:12
@Mac Whatever DOM query you called `.addEventListener()` on didn't return a DOM node. Make sure the `script` is placed just before the closing `body` tag in your HTML so that by the time it runs, all elements have been parsed into memory. — Scott Marcus, May 27 '20 at 19:52

Slider won't close

2 Answers2