efficient
efficient

Reputation: 93

Is there a more efficient way to toggle multiple classes on click than this?

What is a better way to implement this toggle class using JS?

What this is doing is simply this:

When you click either "Services", "Gallery", or "Customer", it removes the class "hidden" in the div below, and adds the class "hidden" to all other lists within the same div.

A live demo of what is happening can be seen here: http://www.cyberbytesdesign.com/WIP2 (it's the main header navigation).

Here is the code that I am using (definitely not an efficient way as this is less than half of all the code, but gets the point across).

<nav>
  <ul class="menu-option-set">
    <li>
      <a href="javascript:;" onmouseup="
        document.getElementById('services').classList.toggle('hidden');
        document.getElementById('gallery').classList.add('hidden');
        document.getElementById('customer').classList.add('hidden');
        ">Services</a>
    </li>
    <li>
       <a href="javascript:;" onmouseup="
         document.getElementById('gallery').classList.toggle('hidden');
         document.getElementById('services').classList.add('hidden');
         document.getElementById('customer').classList.add('hidden'); ">Gallery</a>
    </li>
    <li>
       <a href="javascript:;" onmouseup="
         document.getElementById('gallery').classList.toggle('hidden');
         document.getElementById('services').classList.add('hidden');
         document.getElementById('customer').classList.add('hidden'); ">Customer</a>
    </li>
  </ul>
</nav>
<div id="header-subnav">
  <nav>
    <ul id="services" class="hidden">
      <li <?php echo $bathroom ?>><a href="bathroom">Bathroom</a></li>
      <li <?php echo $kitchen ?>><a href="index.php">Kitchen</a></li>
      <li <?php echo $accessibility ?>><a href="index.php">Accessibility</a></li>
    </ul>
    <ul id="gallery" class="hidden">
      <li <?php echo $photo ?>><a href="gallery.php">Photo Gallery</a></li>
      <li <?php echo $project ?>><a href="project.php">Project Gallery</a></li>
    </ul>
    <ul id="customer" class="hidden">
      <li <?php echo $coupons ?>><a href="coupons.php">Coupons</a></li>
      <li <?php echo $testimonials ?>><a href="testimonials.php">Testimonials</a></li>
    </ul>
  <nav>
</div>

I'm assuming that you have to do something similar to using this somehow, but modified:

$('.menu-option-set a').click(function()
{
    // if clicked item is selected then deselect it
    if ($('#header-subnav').hasClass('hidden'))
    {
        $('#header-subnav').removeClass('hidden');
    }

    // otherwise deselect all and select just this one
    else
    {
        $('.menu-option-set a').removeClass('hidden');
        $('#header-subnav').addClass('hidden');
    }
});

Any ideas?

Upvotes: 3

Views: 2017

Answers (2)

Mics
Mics

Reputation: 1430

Less modification version:

Add data attribute to main menu and set its value to sub menu's id.

<nav>
  <ul class="menu-option-set">
    <li>
      <a href="#" data-subid="services">Services</a>
    </li>
    <li>
       <a href="#" data-subid="gallery">Gallery</a>
    </li>
    <li>
       <a href="#" data-subid="customer">Customer</a>
    </li>
  </ul>
</nav>

Then attach event handler to main menu. It hides all sub menus, and shows proper sub menu. (Also you don't need 'hidden' class.)

$(function() {
    $("#header-subnav ul").hide();
    $('.menu-option-set a').on('click', function() {
        var $s = $("#" + $(this).attr('data-subid'));                
        if($s.is(':hidden')) {
            $("#header-subnav ul").hide();
            $s.show();
        } else {
            $s.hide();
        }
    });
});

​ That's all! Here's example: http://jsfiddle.net/dT225/1/

...But I suggest this style personally:

Put sub menus next to each main menu.

<nav id="menu">
  <ul>
    <li>
      <a href="#">Services</a>
      <ul>
        <li <?php echo $bathroom ?>><a href="bathroom">Bathroom</a></li>
        <li <?php echo $kitchen ?>><a href="index.php">Kitchen</a></li>
        <li <?php echo $accessibility ?>><a href="index.php">Accessibility</a></li>
      </ul>
    </li>
    <li>
      <a href="#">Gallery</a>
      <ul>
        <li <?php echo $photo ?>><a href="gallery.php">Photo Gallery</a></li>
        <li <?php echo $project ?>><a href="project.php">Project Gallery</a></li>
      </ul>
    </li>
    <li>
      <a href="#">Customer</a>
      <ul>
        <li <?php echo $coupons ?>><a href="coupons.php">Coupons</a></li>
        <li <?php echo $testimonials ?>><a href="testimonials.php">Testimonials</a></li>
      </ul>
    </li>
  </ul>
</nav>​​​​​​​​​​​​​

When main menu clicked, event handler will show just next element of source element.

$(function() {
    $('#menu li ul').hide();
    $('#menu a').on('click', function() {
        var $s = $(this).next();           
        if($s.is(':hidden')) {
            $('#menu li ul').hide();
            $s.show();
        } else {
            $s.hide();
        }
    });
});

​It's more simple, maintainable, and semantic I think. http://jsfiddle.net/dKtVK/1/

Upvotes: 2

Christoph
Christoph

Reputation: 51201

First of all thumbs up for trying to do this with plain javascript. Note however, that using things like classList is not supported in every browser - your code will fail in IE.

To your question: You almost answered your question by yourself. It is strongly advised to NOT use those event attributes an the elements. Instead put it into (a separated) js (file). This makes your code clean and maintainable.

So write this instead:

<a class="services" href="javascript:;">Services</a>

and then add an eventhandler in your <script> Tag or separate JS-File:

var gallery  = document.getElementById('gallery');
var services = document.getElementById('services');
var customer = document.getElementById('customer');

document.querySelector(".services").addEventListener(function(){
    /* Do your stuff here */
    gallery.classList.toggle('hidden');
    /*...and so on...*/
});

I used querySelector here, because it is quite similar to jQuery, thus familiar to lot of people, and supported in IE >=8. But feel free to use whatever suits you (and is needed for browser-support).

Using jQuery (the second codesnippet you posted) makes this quite a lot easier and is working fine in all browsers, so feel free to use it and save you some work.

Upvotes: 1

Related Questions