1

I am trying to make a page with tag buttons that can show/hide elements according with their tag. This is my first version without loops:

https://jsfiddle.net/Pokipsy/uu7y3x2x/

$(document).ready(function(){
    $(".showall").click(function(){
        $(".item").show();
        $(".filter").text("All elements")
    });
    $(".show.a").click(function(){
      $(".item").hide();
        $(".item.a").show();
        $(".filter").text("Tag: a")
    });
    $(".show.b").click(function(){
      $(".item").hide();
        $(".item.b").show();
        $(".filter").text("Tag: b")
    });
    $(".show.c").click(function(){
      $(".item").hide();
        $(".item.c").show();
        $(".filter").text("Tag: c")
    });
});
.clickable:hover {
    cursor: pointer;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<ul>
<li class="clickable showall">all</li>
<li class="clickable show a">a</li>
<li class="clickable show b">b</li>
<li class="clickable show c">c</li>
</ul>

<h3 class="filter">
All elements
</h3>
<ul>
<li class="a item">first a</li>
<li class="b item">second b</li>
<li class="a b item">third a b</li>
<li class="c item ">fourth c</li>
<li class="c b item">fifth c b</li>
</ul>

It works but this strategy would produce a very long code if the tags are too many so I tried to use loops to make a shorter code that works with an array of tags:

https://jsfiddle.net/Pokipsy/f9uqetnn/1/

$(document).ready(function(){
    $(".showall").click(function(){
        $(".item").show();
        $(".filter").text("All elements")
    });
    var tags = [".a",".b",".c"]; 
    
    for(i = 0; i < 3; i++) {
    x=tags[i];
    $(".show".concat(x)).click(function(){
      $(".item").hide();
        $(".item".concat(x)).show();
        $(".filter").text("Tag: ".concat(x))
      });
    }
});
.clickable:hover {
    cursor: pointer;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<ul>
<li class="clickable showall">all</li>
<li class="clickable show a">a</li>
<li class="clickable show b">b</li>
<li class="clickable show c">c</li>
</ul>

<h3 class="filter">
All elements
</h3>
<ul>
<li class="a item">first a</li>
<li class="b item">second b</li>
<li class="a b item">third a b</li>
<li class="c item ">fourth c</li>
<li class="c b item">fifth c b</li>
</ul>

but it doesn't work: apparently it always recognise a click to the last element of the array even if I clicked the first. Is there a problem with jQuery that I am unable to see?

Marco Disce
  • 234
  • 2
  • 13
  • http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example – naortor Sep 11 '16 at 16:57

4 Answers4

3

Variable x inside the for loop is overridden in each iteration so you need to lock in inside another closure. Also, I don't know what concat() was supposed to do, I've never seen it used with string. Just use + in JavaScript :).

See your update demo: https://jsfiddle.net/eddd36ma/1/

for (i = 0; i < tags.length; i++) {
    (function(x) {
        $(".show" + x).click(function() {
            $(".item").hide();
            $(".item" + x).show();
            $(".filter").text("Tag: " + x)
        });
    })(tags[i]);
}

Btw, there's also forEach method that takes a function as an argument, so it'd even easier to use that than for because the tag variable is scoped in each iteration:

tags.forEach(function(tag) {
    $(".show" + tag).click(function() {
        $(".item").hide();
        $(".item" + tag).show();
        $(".filter").text("Tag: " + tag)
    });
});

See demo: https://jsfiddle.net/eddd36ma/3/

martin
  • 93,354
  • 25
  • 191
  • 226
  • Concat will combine two or more arrays. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Array/concat – Darkrum Sep 11 '16 at 17:09
1

you can try something like this

1- Instead of showall class use show instead and make click event for all li.show

2- make array of classes name you have

3- loop through array and break if class founded

$(document).ready(function(){
    var ClassesArray = ["a" , "b" , "c"];
    $("li.show").click(function(){
        for ( var i = 0; i < ClassesArray.length; i++ ){
            if ( $(this).hasClass( ClassesArray[i] ) ){
              $(".item").hide();
              $(".item."+ClassesArray[i]).show();
              $(".filter").text("Tag: "+ ClassesArray[i]);
              break;
            }else{
              $(".item").show();
              $(".filter").text("All elements");
            }
        } 
    });
});
.clickable:hover {
    cursor: pointer;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<ul>
<li class="clickable show">all</li>
<li class="clickable show a">a</li>
<li class="clickable show b">b</li>
<li class="clickable show c">c</li>
</ul>

<h3 class="filter">
All elements
</h3>
<ul>
<li class="a item">first a</li>
<li class="b item">second b</li>
<li class="a b item">third a b</li>
<li class="c item ">fourth c</li>
<li class="c b item">fifth c b</li>
</ul>

Note: With this code you can add another classes as you want var ClassesArray = ["a" , "b" , "c" , "d" , "e" , ....]; and the code will work fine .

Mohamed-Yousef
  • 23,946
  • 3
  • 19
  • 28
0

The problem is not with jQuery, the problem is with logics and for loop. As OP noticed the click handler always use the last element which is not surprising. By the moment of click the loop is finished and x has the last element value. Right way to solve it is to wrap x=tags[i] inside a function. Something like this.

$(document).ready(function(){
    $(".showall").click(function(){
        $(".item").show();
        $(".filter").text("All elements")
    });
    var tags = [".a",".b",".c"]; 
    
    for(i = 0; i < 3; i++) {
    //x=tags[i];
    (function(x){
    $(".show".concat(x)).click(function(){
      $(".item").hide();
        $(".item".concat(x)).show();
        $(".filter").text("Tag: ".concat(x))
      });
    }(tags[i]));
    }
});
.clickable:hover {
    cursor: pointer;
}
<script src="https://ajax.googleapis.com/ajax/libs/jquery/2.1.1/jquery.min.js"></script>

<ul>
<li class="clickable showall">all</li>
<li class="clickable show a">a</li>
<li class="clickable show b">b</li>
<li class="clickable show c">c</li>
</ul>

<h3 class="filter">
All elements
</h3>
<ul>
<li class="a item">first a</li>
<li class="b item">second b</li>
<li class="a b item">third a b</li>
<li class="c item ">fourth c</li>
<li class="c b item">fifth c b</li>
</ul>
Alex Kudryashev
  • 9,120
  • 3
  • 27
  • 36
-1

you can do it this way too

HTML:

<ul class="clickableParent">
  <li data-tag="selectAll" class="clickable">all</li>
  <li data-tag="a" class="clickable">a</li>
  <li data-tag="b" class="clickable">b</li>
  <li data-tag="c" class="clickable">c</li>
</ul>

<h3 class="filter">
All elements
</h3>
<ul class="filterItemsParent">
  <li class="a item">first a</li>
  <li class="b item">second b</li>
  <li class="a b item">third a b</li>
  <li class="c item ">fourth c</li>
  <li class="c b item">fifth c b</li>
</ul>

JQuery

$(document).ready(function(){
    var filterItemsParent = $(".filterItemsParent");
    var clickableParent = $(".clickableParent");
    var filter = $(".filter");


  clickableParent.on("click", ".clickable", function () {
    var self = $(this);
    var tag = self.data("tag");
    filter.text("Tag: " +  tag);
    tag = (tag !== "selectAll")? "." + tag : ".item";  
    filterItemsParent.children().hide().end()
        .children(tag).fadeIn();
  });
});

DEMO at JSFIDDLE!

InTry
  • 1,169
  • 3
  • 11
  • 35