Is there a cleaner way to call multiple toggles & elements during a click event listener with vanilla JavaScript?

user_14331407 0
Viewed 113

I have multiple toggles being called at the same time during a click /eventListener and it works but I'm wondering if there is a cleaner way to write the code where it doesn't look so repetitive.

Here is a snippet of the code I'm trying to refactor/clean up. Any help or point in the right direction is greatly appreciated. Thanks!

document.getElementById("menu-toggle").addEventListener("click", mobileMenuToggle);

function mobileMenuToggle() {
    var hideMenu = document.getElementById("mobile-menu-container");
        hideMenu.classList.toggle("hide-menu");

    var hideMenuIcon = document.getElementById("menu-toggle");
        hideMenuIcon.classList.toggle("hide-menu-icon");

    var closeMenuIcon = document.getElementById("close-menu-toggle");
        closeMenuIcon.classList.toggle("show-close-icon");

    var closeMenuIcon = document.getElementById("container-bg-toggle");
        closeMenuIcon.classList.toggle("show-icon-bg");
}
javascript
3 Answers

I would write something like this, put all element info in a object, iterate the object and do DOM manipulation. This way a new element-class pair can be added in the object, rest of the code will be untouched

function mobileMenuToggle() {
     // Object to store  id-class name pair
      var elements = {"mobile-menu-container" : "hide-menu", 
                        "menu-toggle": "hide-menu-icon", 
                        "close-menu-toggle" : "show-close-icon", 
                        "container-bg-toggle": "show-icon-bg"};
    

    // iterate and toggle class names
    for (var elementId in elements ) {
        var el = document.getElementById(elementId );
        el.classList.toggle(elements[elementId]);
        
    }
}
user_11883220

Few other options

  1. To make it more concise, you can get rid of references, which you are not reusing

    function mobileMenuToggle() {
  document.getElementById("mobile-menu-container").classList.toggle("hide-menu");
  document.getElementById("menu-toggle").classList.toggle("hide-menu-icon");
  document.getElementById("close-menu-toggle").classList.toggle("show-close-icon");
  document.getElementById("container-bg-toggle").classList.toggle("show-icon-bg");
}

  2. In case if these elements are nested, You can just write a css rule and do one toggle

    #mobile-menu-container.hide-menu .menu-toggle {
  display:none;
}

#mobile-menu-container.hide-menu .close-menu-toggle {
  display:block;
}

    And you can just toggle the parent container alone

     function mobileMenuToggle() {
   document.getElementById("mobile-menu-container").classList.toggle("hide-menu");
 }

  3. In case if you are using jquery, you can use the jquery toggle, which is bit short

    $('#mobile-menu-container').toggleClass('hide-menu');
user_81871490

There are couple of ways of doing it based on how clean you want and how many repetitive elements are there.

If you have just few elements, then this should be clean enough.

var menuToggle = document.getElementById("menu-toggle");

function mobileMenuToggle() {
  var hideMenu = document.getElementById("mobile-menu-container");
  var hideMenuIcon = document.getElementById("menu-toggle");
  var closeMenuIcon = document.getElementById("close-menu-toggle");
  var closeMenuBgIcon = document.getElementById("container-bg-toggle");

  hideMenu.classList.toggle("hide-menu");
  hideMenuIcon.classList.toggle("hide-menu-icon");
  closeMenuIcon.classList.toggle("show-close-icon");
  closeMenuBgIcon.classList.toggle("show-icon-bg");
}

menuToggle.addEventListener("click", mobileMenuToggle);

If you have many elements and they need to be dynamic then you can convert it to an array and loop over them.

var menuToggle = document.getElementById("menu-toggle")
var elementList = [
  { id: "mobile-menu-container", className: "hide-menu" },
  { id: "hide-menu", className: "hide-menu-icon" },
  { id: "close-menu-toggle", className: "show-close-icon" },
  { id: "container-bg-toggle", className: "show-icon-bg" }
];

function mobileMenuToggle() {
  elementList.forEach(function (props) {
    var element = document.getElementById(props.id);
    
    element.classList.toggle(props.className);
  });
}

menuToggle.addEventListener("click", mobileMenuToggle);
user_14040760
Related