Is there a better way than this to create a toggle function just with JavaScript (no JQuery at all)?

Question

I have a nav bar in the form of a ul with several li elements and with the last li (named 'more') having its own sub-menu in the form of another ul. I was trying (and was successful) in making it so that the sub-menu's original state is visibility:visible; and then when the user clicks on li name 'more' it would toggle between visibility: visible; and visibility: hidden; I used JavaScript and a counter with an if statement. The reason why I used the counter was because when I tried:

if(document.querySelector('#subMenu').style.visibility == "hidden")...;

But it wouldn't toggle.

My questions are:

Would this method of creating the toggle function be deemed acceptable in a professional front end developer workplace?
Is there a better way to toggle between visible and hidden on clicking an element using JavaScript ONLY (trying to get better at JavaScript)?

The code is as follows(I have only included the relevant code):

HTML

<ol id = "leftNav" class = "bothNavs">
    <li class = "navs" id = "more">More<div class = "arrow"></div>
        <ul class = "subMenu">
            <li>One</li>
            <li>Two</li>
            <li>Three</li>
        </ul>
    </li>
</ol>

CSS

.subMenu {
    width: 160%;
    padding: 5px 0px;
    padding-left: 7px;
    margin-left: 6px;
    position:  absolute;
    top: 100%;
    left:0px;
    visibility: hidden;
    box-shadow: 0px 2px 3px rgba(0,0,0,0.2);
    background: #2D2D2D;
    list-style: outside none none;
    z-index: 1001;
}

JavaScript

var more = document.querySelector('#more');
var subMenu = document.querySelector('.subMenu');
var counter =0;

more.addEventListener("click", toggle);

function toggle () {
    if(counter === 0){
        subMenu.style.visibility = "visible";
        counter += 1;
    } else {
        subMenu.style.visibility = "hidden";
        counter -= 1;
    }
};

Thank you in advance for your answers.

From the code snippet shown, using `document.querySelector('#subMenu').style` probably didnt work because `subMenu` is a class not an id, should have been `('.subMenu')` — Patrick Evans, Jan 23 '16 at 16:31
The simple way to toggle visibility of the element is such: `element.hidden=!element.hidden;` — Dmytro, Jan 23 '16 at 17:29

score 5 · Accepted Answer · answered Jan 23 '16 at 16:36

5

I feel a better way of toggling visibility (or any style) is to toggle a class.

Consider something like this in your CSS:

.subMenu.is-visible {
    visibility: visible;
}

Then your function just needs to look like this:

var menu = document.querySelector('.subMenu');
document.getElementById('more').addEventListener('click', function () {
    menu.classList.toggle('is-visible');
}, false);

answered Jan 23 '16 at 16:36

James Long

4,629
1
20
30

Both this answer and cynicalJoys answers worked but I chose this one because it taught me about the classList.toggle method which seems very useful in a variety of situations – Obi-wan Jan 23 '16 at 17:35

score 2 · Answer 2 · edited May 23 '17 at 10:27

2

For a toggle you should use a boolean value rather than int:

var more = document.querySelector('#more');
var subMenu = document.querySelector('.subMenu');
var clicked = false;

more.addEventListener("click", function(evt) {
  // This is called a ternary operator, it's basically
  // a really simple if/else statement
  subMenu.style.visibility = (clicked) ? "visible " : "hidden";

  // This will set clicked to the opposite (not) value of what
  // it currently is. Being that we're using a boolean 
  // it will toggle true/false
  clicked = !clicked;
});

You should also read (and vote for) this - What is the difference between visibility:hidden and display:none?

edited May 23 '17 at 10:27

Community

1
1

answered Jan 23 '16 at 16:28

cynicaljoy

2,047
1
18
25

1

While this may work, there is a possibility that if some other code modifies the visibility of the element this will become reversed, ie the element will be shown when it was meant to be hidden. – Patrick Evans Jan 23 '16 at 16:33
This answer as well as the above worked for me and it gave me a reason to look more into the ternary operator, thanks! – Obi-wan Jan 23 '16 at 17:37

score 0 · Answer 3 · answered Jan 23 '16 at 16:43

You should not embed a <ul> inside a <li> element. There's all sorts of reasons why, but mostly it messes with the hierarchy and makes the HTML less meaningful.

Also the last time I had to tackle this problem it ended up looking something like this:

var subMenu = document.querySelector('.subMenu');
if (clicked) {
    subMenu.style.visibility = "visible";
} else {
    subMenu.style.visibility = "hidden";
}

Consider making it id instead of class incase you have other submenus it interferes with.

What alternative do you suggest for making the drop down menu if not an embedded
? — Obi-wan, Jan 24 '16 at 03:22

score 0 · Answer 4 · answered Jan 23 '16 at 16:43

If you don't really need a counter, use a boolean:

var more = document.querySelector('#more');
var subMenu = document.querySelector('.subMenu');

more.addEventListener("click", toggle);

function toggle () {    
    subMenu.style.visibility = subMenu.style.visibility != "hidden" ? "hidden" : "visible";
    
};

.subMenu {
    width: 60%;
    background: #2D2D2D;
    list-style: outside none none;
    color: white
}

<ol id = "leftNav" class = "bothNavs">
    <li class = "navs" id = "more">More<div class = "arrow"></div>
        <ul class = "subMenu" style="visibility: hidden">
            <li>One</li>
            <li>Two</li>
            <li>Three</li>
        </ul>
    </li>
</ol>

Ideally, you'd modify this code to toggle a class instead of an inline CSS style, as James suggested in his answer.

Is there a better way than this to create a toggle function just with JavaScript (no JQuery at all)?

4 Answers4