Toggle button not working. Not having display: none; at first click

Question

I'm trying to do a toggle button with js, but the first time it's clicked it is not working. Since the nav.display.display has no value although it has display: none; in the CSS code.

I guess there should be an issue on how I'm using onload... as if the css wasn't loaded yet. But I've read that window.onload waits for all to load. So I don't really know what's happening.

This is how I'm loading it, in real code (outside jsfiddle that handles onload)

window.onload = function() {
    var el = document.querySelector('.nav-toggle');
    el.addEventListener("click", toggleNav, false);
};

NO jQuery please.

Here's the jsfiddle:

function toggleNav() {
  var nav = document.querySelector('#navigation');
  alert(nav.style.display);
  
  if (nav.style.display == 'none') {
    nav.style.display = 'flex';
  } else {
    nav.style.display = 'none';
  }
}
var el = document.querySelector('.nav-toggle');
el.addEventListener("click", toggleNav, false);

#navigation {
    display: none;
}

<button class="nav-toggle">Toggle</button>
<nav id="navigation">
    <ul>
        <li><a href="#">Item 1</a></li>
        <li><a href="#">Item 2</a></li>
        <li><a href="#">Item 3</a></li>
        <li><a href="#">Item 4</a></li>
        <li><a href="#">Item 5</a></li>
    </ul>
</nav>

Yeah, that way the DOM has the display property. And it works. But what If I don't want to use style="" attribute ? — Jeflopo, Dec 15 '16 at 06:15
If you want to keep the "display:none" in the css you can add a check for offsetWidth and offsetHeight as seen here: http://stackoverflow.com/a/19808107/407526 — Adrian, Dec 15 '16 at 06:15
Just for fun, here is a somewhat hacky, pure CSS version: https://jsfiddle.net/k5yym7ng/ — Jon P, Dec 15 '16 at 06:45
@Jeflopo if you don't want to use the style attribute then you shouldn't be using JavaScript because JS is adding this attribute to your element. This is the reason that it doesn't work first time. JavaScript can't find the attribute so it needs to set it first. — , Dec 15 '16 at 09:39

score 2 · Answer 1 · answered Dec 15 '16 at 06:28

Thanks to @Aliester comment I figured it out :)

function toggleNav() {
  var nav = document.getElementById('navigation');
  
  if (nav.offsetWidth == 0 && nav.offsetHeight == 0) {
    nav.style.display = 'flex';
  } else {
    nav.style.display = 'none';
  }
}
var el = document.querySelector('.nav-toggle');
el.addEventListener("click", toggleNav, false);

#navigation {
    display: none;
}

<button class="nav-toggle">Toggle</button>
<nav id="navigation">
    <ul>
        <li><a href="#">Item 1</a></li>
        <li><a href="#">Item 2</a></li>
        <li><a href="#">Item 3</a></li>
        <li><a href="#">Item 4</a></li>
        <li><a href="#">Item 5</a></li>
    </ul>
</nav>

Undecided Dev · Answer 2 · 2016-12-15T06:19:23.790

0

Add Disply:none on your #navigation hope this one works.

function toggleNav() {
  var nav = document.getElementById('navigation');
  
  
  if (nav.style.display == 'none') {
    nav.style.display = 'block';
    
  } else {
    nav.style.display = 'none';
  }
}
var el = document.querySelector('.nav-toggle');
el.addEventListener("click", toggleNav, false);

#navigation {
    display: none;
}

<button class="nav-toggle">Toggle</button>
<nav id="navigation">
    <ul>
        <li><a href="#">Item 1</a></li>
        <li><a href="#">Item 2</a></li>
        <li><a href="#">Item 3</a></li>
        <li><a href="#">Item 4</a></li>
        <li><a href="#">Item 5</a></li>
    </ul>
</nav>

edited Dec 15 '16 at 06:19

answered Dec 15 '16 at 06:16

Undecided Dev

840
6
16

It works. But I would prefer to not use style="" attribute. – Jeflopo Dec 15 '16 at 06:18
The current answer doesn't works. The first time you click display has no value. So we have to click twice to show. – Jeflopo Dec 15 '16 at 06:26
**First time** click not working its working after **twice** click. – Raeesh Alam Dec 15 '16 at 06:45

score 0 · Accepted Answer · answered Dec 15 '16 at 07:14

You can access style via getComputedStyle() instead of style

var btn = document.querySelector('.nav-toggle'),
    nav = document.getElementById('navigation');
    
btn.addEventListener('click', function (e) {
  e.preventDefault()
  if (window.getComputedStyle(nav, null).display == 'none') {
    nav.style.display = 'flex';
  } else {
    nav.style.display = 'none';
  }
});

#navigation {
    display: none;
}

<button class="nav-toggle">Toggle</button>
<nav id="navigation">
    <ul>
        <li><a href="#">Item 1</a></li>
        <li><a href="#">Item 2</a></li>
        <li><a href="#">Item 3</a></li>
        <li><a href="#">Item 4</a></li>
        <li><a href="#">Item 5</a></li>
    </ul>
</nav>

Kudos for you, I think it's better to use `getComputedStyle()` than relying on elements size. Thank you too. — Jeflopo, Dec 16 '16 at 04:50

Toggle button not working. Not having display: none; at first click

3 Answers3