CODEWITHSUNDEEP
javascripthtmldrop-down-menuonclickbuttonclick
MJCarnahan
MJCarnahan

Reputation: 23

Simple HTML Javascript button onclick function not working

So essentially I want to keep this as simple as possible, meaning no jquery or bootstrap etc... just straight javascript, HTML and CSS. This is what I have so far

Javscript:

var menuOptions= document.getElementsByClassName("nav");

var hamburger= document.getElementById("nav-btn");

  function myFunction() {

hamburger.onclick= menuOptions.style.visibility= 'visible';

}

HTML:

<HTML>
  <button onclick="myFunction()">  
    <span id="nav-btn">
      <image src="Menugreen.png" alt="collapsable menu"/>  
    </span>
  </button>  
<div class="nav">
  <ul>
    <li id="Programs"> <a href="Programs.html"> Programs </a> </li>
    <li> <a href="Tshirts.html"> T-Shirts </a> </li>
    <li id="About"> <a href="About.html"> About </a> </li>
  </ul>
</div>
</HTML>

CSS:

.nav {
  visibility: hidden;
}

Besides just giving me a solution I would highly appreciate it if you could explain why my current method does not work and why yours does. Thanks in advance!

Upvotes: 1

Views: 3883

Answers (2)

gyre
gyre

Reputation: 16779

There are a few reasons why your current setup does not function:

  • Document#getElementsByClassName returns a collection, and you are treating the result like a DOM element. You need to access an index like [0] to get an actual element.

  • Your toggle button only works one way, because visibility is set to visible but never set back to none when clicked again.

  • In myFunction, hamburger.onclick should not be assigned to the expression you chose. I am not sure why you tried to assign another click handler, but in order to make that work you would have needed to set it to a function () { ... }.

Now for my advice:

  • Use CSS classes to control whether the menu is hidden or not, rather than messing around with the style property in your JS. You can use the classList property of DOM elements to .add(), .remove(), and .toggle() a specific class when myFunction is run. I have chosen to use toggle because I think that most suits your use case.

  • Use element.addEventListener instead of HTML attributes like onclick.

Snippet:

var menuOptions = document.getElementsByClassName("nav")[0]

var hamburger = document.getElementById("nav-btn")

hamburger.parentNode.addEventListener('click', function myFunction() {
  menuOptions.classList.toggle('hidden')
})
.nav.hidden {
  visibility: hidden;
}
<button>  
    <span id="nav-btn">
      <img src="Menugreen.png" alt="collapsable menu"/>  
    </span>
  </button>
<div class="nav hidden">
  <ul>
    <li id="Programs"> <a href="Programs.html"> Programs </a> </li>
    <li> <a href="Tshirts.html"> T-Shirts </a> </li>
    <li id="About"> <a href="About.html"> About </a> </li>
  </ul>
</div>

Upvotes: 0

nnnnnn
nnnnnn

Reputation: 150020

Two problems:

  1. getElementsByClassName() returns a list, not a single element (though the list may contain just a single element), and that list doesn't have a .style property. You can use menuOptions[0] to access the first (and in this case only) element in the list.

  2. You don't want to say hamburger.onclick= inside your function, because that would be assigning a new onclick handler but your function is already being called from the onclick attribute of your button. (Also, if you were trying to assign a new click handler you'd want hamburger.onclick = function() { /* something */ }.)

So the minimum change to your existing code to get it to work would be to change this line:

hamburger.onclick= menuOptions.style.visibility= 'visible';

...to this:

menuOptions[0].style.visibility = 'visible';

In context:

var menuOptions= document.getElementsByClassName("nav");
var hamburger= document.getElementById("nav-btn");

function myFunction() {
  menuOptions[0].style.visibility = 'visible';
}
.nav {
  visibility: hidden;
}
<HTML>
  <button onclick="myFunction()">  
    <span id="nav-btn">
      <image src="Menugreen.png" alt="collapsable menu"/>  
    </span>
  </button>  
<div class="nav">
  <ul>
    <li id="Programs"> <a href="Programs.html"> Programs </a> </li>
    <li> <a href="Tshirts.html"> T-Shirts </a> </li>
    <li id="About"> <a href="About.html"> About </a> </li>
  </ul>
</div>
</HTML>

If you want repeated clicks on the button to toggle the menu display on and off then you can test the current visibility:

  menuOptions[0].style.visibility =
    menuOptions[0].style.visibility === 'visible' ? '' : 'visible';

Expand the following to see that working:

var menuOptions= document.getElementsByClassName("nav");
var hamburger= document.getElementById("nav-btn");

function myFunction() {
  menuOptions[0].style.visibility =
    menuOptions[0].style.visibility === 'visible' ? '' : 'visible';
}
.nav {
  visibility: hidden;
}
<HTML>
  <button onclick="myFunction()">  
    <span id="nav-btn">
      <image src="Menugreen.png" alt="collapsable menu"/>  
    </span>
  </button>  
<div class="nav">
  <ul>
    <li id="Programs"> <a href="Programs.html"> Programs </a> </li>
    <li> <a href="Tshirts.html"> T-Shirts </a> </li>
    <li id="About"> <a href="About.html"> About </a> </li>
  </ul>
</div>
</HTML>

Upvotes: 1

Related Questions