How can I make this javascript function not use a global variable?

Question

This function toggles the active state of a hamburger icon when clicking on it. Also clicking anywhere on the document does the same but only if the dropdown is open.

var dropdownOpen = false;

$(".hamburger").click(function () {
    $(this).toggleClass('is-active');
    dropdownOpen = !dropdownOpen;
});

$(document).ready(function(){
    $(document).click(function(e){
        if ($(e.target).is('.hamburger')) {
            return;
        }
        else if (dropdownOpen === true)
        {
            $(".hamburger").toggleClass('is-active');
            dropdownOpen = false;
        }
    });
});

How would I go about combining two click events so I don't have to use a global variable?

I've tried this:

$(document).ready(function(){
    var dropdownOpen = false;

    $(document).click(function(e){
        if ($(e.target).is('.hamburger')) {
            $('.hamburger').toggleClass('is-active');
            dropdownOpen = !dropdownOpen;
        }
        else if (dropdownOpen === true)
        {
            $(".hamburger").toggleClass('is-active');
            dropdownOpen = false;
        }
    });
});

..but it didn't work, any ideas?

The function you are passing, give it a name. Attach a prototype property and use that each time you want to toggle — binariedMe
– binariedMe, Commented Mar 4, 2018 at 8:52
The second version is missing the $(".hamburger").click() handler. If you add that into the document.ready function it should work. — Barmar
– Barmar, Commented Mar 4, 2018 at 8:54
Why are you doing if (dropdownOpen === true)? Just use if (dropdownOpen), because if the expression is not true, it is false. — Markus Zeller
– Markus Zeller, Commented Mar 4, 2018 at 8:54
Why do you need the global variable? Just use if ($(".hambuger").hasClass("is-active")) — Barmar
– Barmar, Commented Mar 4, 2018 at 8:59
or get rid of the if entirely, and just do $(".hamburger").removeClass("is-active"). — Barmar
– Barmar, Commented Mar 4, 2018 at 8:59

void · Accepted Answer · 2018-03-04 08:51:45Z

1

You can wrap all your JS in an Immediately Invoked Function Expression. All the JS variables are not scoped to this function expression instead of being available globally.

(function() {
  var dropdownOpen = false;

  $(".hamburger").click(function() {
    $(this).toggleClass('is-active');
    dropdownOpen = !dropdownOpen;
  });

  $(document).ready(function() {
    $(document).click(function(e) {
      if ($(e.target).is('.hamburger')) {
        return;
      } else if (dropdownOpen === true) {
        $(".hamburger").toggleClass('is-active');
        dropdownOpen = false;
      }
    });
  });
})();

answered Mar 4, 2018 at 8:51

void

36.7k10 gold badges71 silver badges111 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

dan Over a year ago

Thanks, that works. I've not heard of an Immediately Invoked Function Expression before. Will be useful to know in the future.

Barmar · Accepted Answer · 2018-03-04 09:02:33Z

1

There's no need for the global varable at all.

$(document).click(function(e) {
    if ($(e.target).is(".hamburger")) {
        $(e.target).toggleClass("is-active");
    } else {
        $(".hambuger").removeClass("is-active");
    }
}

There's no harm in calling removeClass() if the class isn't there.

answered Mar 4, 2018 at 9:02

Barmar

789k57 gold badges554 silver badges669 bronze badges

Collectives™ on Stack Overflow

How can I make this javascript function not use a global variable?

2 Answers 2

1 Comment

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

1 Comment

Comments

Your Answer

Sign up or log in

Post as a guest

Related