efficient
efficient

Reputation: 93

How do I shorten up this overly long JavaScript function?

I have a simple script that selects a class's and adds a class of selected to it and remove it from all other 's with the similar class. It all works perfectly, but I know that there has to be a better way to write this out, I just can't figure it out.. Does it involve something like this?

$('.adv-option-set a, .adv-option-set2 a, .adv-option-set3 a, .adv-option-set4 a, .adv-option-set5 a, .adv-option-set6 a, .adv-option-set7 a').click(function()
{
    // if clicked item is selected then do nothing
    if ($(this).hasClass('selected')){}

    // otherwise deselect all and select just this one
    else
    {
        $('.adv-option-set a').removeClass('selected');
        $(this).addClass('selected');
    }

    {
        $('.adv-option-set2 a').removeClass('selected');
        $(this).addClass('selected');
    }

    {
        $('.adv-option-set3 a').removeClass('selected');
        $(this).addClass('selected');
    }
});

So here is the code that I currently have and is working:

$('.adv-option-set a').click(function()
{
    // if clicked item is selected then do nothing
    if ($(this).hasClass('selected')){}

    // otherwise deselect all and select just this one
    else
    {
        $('.adv-option-set a').removeClass('selected');
        $(this).addClass('selected');
    }
});

$('.adv-option-set2 a').click(function()
{
    // if clicked item is selected then do nothing
    if ($(this).hasClass('selected')){}

    // otherwise deselect all and select just this one
    else
    {
        $('.adv-option-set2 a').removeClass('selected');
        $(this).addClass('selected');
    }
});

$('.adv-option-set3 a').click(function()
{
    // if clicked item is selected then do nothing
    if ($(this).hasClass('selected')){}

    // otherwise deselect all and select just this one
    else
    {
        $('.adv-option-set3 a').removeClass('selected');
        $(this).addClass('selected');
    }
});

Etc. up until '.adv-option-set7 a'

EDITED to show the DOM:

      <article id="filter">
        <ul id="filter-nav" class="option-set">
          <li>Filter: </li>
          <li><a data-categories="*" data-subid="all" class="selected">All</a></li>
          <li><a data-categories="tubs-and-showers" data-subid="tubs-and-showers">Tubs &amp; Showers</a></li>
          <li><a data-categories="countertops" data-subid="countertops">Countertops</a></li>
          <li><a data-categories="faucets" data-subid="faucets"><s>Faucets</s></a></li>
          <li><a data-categories="cabinetry" data-subid="cabinetry"><s>Cabinetry</s></a></li>
          <li><a data-categories="flooring" data-subid="flooring"><s>Flooring</s></a></li>
          <li><a data-categories="toilets" data-subid="toilets"><s>Toilets</s></a></li>
          <li><a data-categories="accessories" data-subid="accessories"><s>Accessories</s></a></li>
        </ul>
        <div id="advfilter" class="advfilter filter-nav hidden">
          <ul id="tubs-and-showers" class="adv-option-set1">
            <li><span class="small">ADV</span> Filter: </li>
            <li><a data-categories="modular">Modular</a></li>
            <li><a data-categories="custom">Custom</a></li>
          </ul>
          <ul id="countertops" class="adv-option-set2">
            <li><span class="small">ADV</span> Filter: </li>
            <li><a data-categories="marble">Marble</a></li>
            <li><a data-categories="solid-surface">Solid Surface</a></li>
            <li><a data-categories="laminate"><s>Laminate</s></a></li>
            <li><a data-categories="granite"><s>Granite</s></a></li>
          </ul>
          <ul id="faucets" class="adv-option-set3">
            <li><span class="small">ADV</span> Filter: </li>
            <li><a data-categories="">Coming Soon</a></li>
            <li><a data-categories="">Coming Soon</a></li>
          </ul>
          <ul id="cabinetry" class="adv-option-set4">
            <li><span class="small">ADV</span> Filter: </li>
            <li><a data-categories="">Coming Soon</a></li>
            <li><a data-categories="">Coming Soon</a></li>
          </ul>
          <ul id="flooring" class="adv-option-set5">
            <li><span class="small">ADV</span> Filter: </li>
            <li><a data-categories="">Coming Soon</a></li>
            <li><a data-categories="">Coming Soon</a></li>
          </ul>
          <ul id="toilets" class="adv-option-set6">
            <li><span class="small">ADV</span> Filter: </li>
            <li><a data-categories="">Coming Soon</a></li>
            <li><a data-categories="">Coming Soon</a></li>
          </ul>
          <ul id="accessories" class="adv-option-set7">
            <li><span class="small">ADV</span> Filter: </li>
            <li><a data-categories="">Coming Soon</a></li>
            <li><a data-categories="">Coming Soon</a></li>
          </ul>
        </div>
      </article>

FINAL EDIT: This is the other part of the JS code that I have, it seems that your answers actually take this part into effect aswell though:

    $("#advfilter ul").hide();
    $('.option-set a').on('click', function() {
        $("#advfilter ul").hide();
        $("#advfilter").removeClass('hidden');
        $("#" + $(this).attr('data-subid')).show();
        /*location.href='#filter'*/
    });

    $('.option-set a').click(function()
    {
        // if clicked item is selected then deselect it
        if ($(this).hasClass('selected'))
        {
           $('.adv-option-set1 a').removeClass('selected');
           $('.adv-option-set2 a').removeClass('selected');
           $('.adv-option-set3 a').removeClass('selected');
           $('.adv-option-set4 a').removeClass('selected');
           $('.adv-option-set5 a').removeClass('selected');
           $('.adv-option-set6 a').removeClass('selected');
           $('.adv-option-set7 a').removeClass('selected');
        }

        // otherwise deselect all and select just this one
        else
        {
           $('.option-set a').removeClass('selected');
           $(this).addClass('selected');
           $('.adv-option-set1 a').removeClass('selected');
           $('.adv-option-set2 a').removeClass('selected');
           $('.adv-option-set3 a').removeClass('selected');
           $('.adv-option-set4 a').removeClass('selected');
           $('.adv-option-set5 a').removeClass('selected');
           $('.adv-option-set6 a').removeClass('selected');
           $('.adv-option-set7 a').removeClass('selected');
        }
    });

Upvotes: 1

Views: 140

Answers (4)

Ankur Ghelani
Ankur Ghelani

Reputation: 659

you can use starts with in jQuery as below:

$('div[class^="adv-option-set"] a').click(function()
{

   // if clicked item is selected then do nothing
   if ($(this).hasClass('selected'))
   {
   }else
   {
     $('div[class^="adv-option-set"] a').removeClass('selected');
     $(this).addClass('selected');
   }

}

I hope this would help, this is called attribute start with selectors below is reference link for you to have look at: http://api.jquery.com/attribute-starts-with-selector/

this is i think shortest code, I am hopping

Thanks Ankur

Upvotes: 0

Bergi
Bergi

Reputation: 664444

Option #1 would be a simple loop:

for (var i=1; i<=7; i++) (function(){
    var links = $('.adv-option-set'+i+' a');
    links.click(function() {
        var $this = $(this);
        if (!$this.hasClass('selected')) { // could be left out
            links.removeClass('selected');
            $this.addClass('selected');
        }
    });
})();

Note that this code does not remove any classes, as your comment (not the code) to .adv-option-set3 ("…then deselect it") implies, it "do[es] nothing" when the item is already selected. If you want to enable removing the class, you might want to use this snippet in the handler:

if ($(this).toggleClass('selected').hasClass('selected'))
    links.not(this).removeClass('selected');

Option #2 would be to set up the same event handler for all links (better: use delegated events) and get the sibling links where the class should be removed dynamically. The code for this depends on your DOM setup.


OK, now that I see your dom I have some additional points.

  • The classes are really useless. You seem to use the to identify one element - which is already unique identified by its id. Use (the same) class="adv-option-set" on all of them to classify them.

    The code for option #2 - both elegant and efficient - would then look like this:

$("#advfilter").on("click", "a", function(e) {
    $(this).addClass('selected')
      .closest(".adv-option-set").find("a").not(this).removeClass('selected');
});
// or, little better but tightier bound to the DOM:
$("#advfilter").on("click", "a", function(e) {
    $(this).addClass('selected')
      .parent().siblings().find("a").removeClass('selected');
});
  • I'm not sure, but you should really take a look at traditional radio buttons to build your form with. After that is done, you might enhance it with some JavaScript.

Upvotes: 3

Phillip Schmidt
Phillip Schmidt

Reputation: 8818

Yeah. But first of all, the code doesn't make sense. You said "otherwise deselect all and select just this one", but really you're deselecting THIS one and then re-selecting it. So first off, give a common class name to all of them. Then, using jQuery, you can do:

$(".commonClassName").each(function(index, item){
    item.click(function(){
        $(".commonClassName").removeClass('selected');  //no matter what, deselect all of them
        if(!item.hasClass('selected'))
        {
            item.addClass('selected');  //then select this one if it isn't already
        }
    }
});

Upvotes: 1

Cranio
Cranio

Reputation: 9847

$('[class^=adv-option-set] a').click(
    function()
    {
       $(this).addClass('selected');
       $('[class^=adv-option-set] a').not(this).removeClass('selected');
    }
);

Upvotes: 3

Related Questions