Search code examples
jquerytogglestoppropagation

Better way to toggle dropdown div on clicking on and off?


I'm currently toggling a dropdown div menu with the following code:

$(function() {
  function toggleMenu(show) {
    $(".dropdownInfo").toggle(show);
    $('.userNavBar').css('background-color', show ? '#444' : '#333');
    $('.upperBar').css('border-top-color', show ? '#ff556f' : '#333');
  };
  $('.userNavBar').click(function(e) {
    toggleMenu(true);
    e.stopPropagation();
  });
  $("body").click(function(e) {
    toggleMenu(false);
  });
});

Is there a faster/better/more efficient way to toggle the div? This seems like a very large chunk of code..


Solution

  • Use more CSS in a stylesheet, and less inline. Then you can just toggle a class, and stand around looking ...classy. One other thing: you can bind just a single click event listener, and in the event handler, check to see whether or not you should open or close the menu.

    CSS

    .userNavBar {
        background-color: #333;
    }
    
    .userNavBar.active {
        background-color: #444;
    }
    
    .upperBar {
        border-top-color: #333;
    }
    
    .upperBar.active {
        border-top-color: #ff556f;
    }
    

    JavaScript

    $(function() {
      function toggleMenu(show) {
        $('.dropdownInfo').toggle(show);
        $('.userNavBar, .upperBar').toggleClass('active', show);
      };
    
      $('body').on('click', function(e) {
        var show = $(e.target).hasClass('userNavBar');
        if (show) e.preventDefault(); // not sure this is even necessary anymore
        toggleMenu(show);
      });
    });
    

    Also, please don't mix single- and double-quotes. Pick one, and be consistent.