Simple Jquery function won't work

Question

I have following jQuery code

$(document).ready(function() { 
    for(var i=1;i<4;i++) { 
        var img= ".img"+i; 
        var p= ".p"+i;
        $(img).on("click", function(){
            $(p).hide(); 
        }); 
    } 
});

Applied on the following html:

<div>
    <article>
         <h1>Over mezelf <img src='images/plus.png' class='img1'/></h1>

        <p class='p1'>Test</p>
    </article>
    <article>
         <h1>Contact <img src='images/plus.png' class='img2'/></h1>

        <p class='p2'>Test</p>
    </article>
    <article>
         <h1>Website <img src='images/plus.png' class='img3'/></h1>

        <p class='p3'>Test</p>
    </article>
</div>

When I click on the images, the last <p> disappears, it doesn't work on the other ones.

Answer 1

Milind Anantwar's answer is definitely the way to go, but here are a few more notes:

You may need the selectors to be more specific, so they are less dependent on layout changes:

JSFiddle: http://jsfiddle.net/TrueBlueAussie/XhRPC/2/

$(function () {
     $('article h1 img').on('click', function(){
         $(this).closest('article').find('p').last().hide(); 
     });
});

This finds the closest article ancestor, then finds the last paragraph (.children('p').last() does the same thing only faster than find as it search only one level)

*Note: $(function(){YOUR CODE HERE}); is just a shortcut for $(document).ready(function(){YOUR CODE HERE});

You should not have unique class names unless you actually mean to style each element specifically. Unique identifiers belong in id= and shared classes belong in class=.

Dynamic elements?: http://jsfiddle.net/TrueBlueAussie/XhRPC/3/

Should you introduce dynamically loaded elements, of the same type you have shown, you can easily change it to use a delegated version of on():

$(function () {
     $(document).on('click', 'article h1 img', function(){
         $(this).closest('article').find('p').last().hide(); 
     });
});

Delegated events listen at an ancestor, then apply the selector, then apply the function to any matching elements that generated the event.

The preference is to use the first non-changing ancestor in your DOM. Failing that (you do not show the rest of the page) you can use document which receives all bubbled events on the page.

Answer 2

The problem is due to the scope of p. When it comes round to running the event handler code it will always use the most recent value of 'p', which in this case would be ".p3" (see here for more information)

Quick Fix

You could use a closure instead:

$(document).ready(function() { 
    for(var i = 1; i < 4; i++) { 
        var img= ".img" + i;
        var p = ".p" + i;
        (function(p){
            $(img).on("click", function(){
                $(p).hide();
            });
        })(p);
    }
});

Here is a working example

---

Recommended

Personally I would prefer to change your HTML to use a common class, this will avoid the need for a loop. You can then navigate your way to the correct p tag...

HTML:

<div>
    <article>
         <h1>Over mezelf <img src='images/plus.png' class='img'/></h1>

        <p class='p1'>Test</p>
    </article>
    <article>
         <h1>Contact <img src='images/plus.png' class='img'/></h1>

        <p class='p2'>Test</p>
    </article>
    <article>
         <h1>Website <img src='images/plus.png' class='img'/></h1>

        <p class='p3'>Test</p>
    </article>
</div>

(note that you can have multiple classes if you still need the numbered ones for something else. e.g: "img img1")

Javascript:

$(document).ready(function() {
    $(".img").on("click", function () {
        $(this).closest("article").find("p").hide();
    });
});

Here is a working example

Answer 3

You can simply use:

 $('img').on("click", function(){
     $(this).parent().next().hide(); 
 }); 

JSFiddle: http://jsfiddle.net/TrueBlueAussie/XhRPC/1/

Answer 4

You just need this

$(img).on("click", function(){
    $(this).closest('article').find('p').hide(); 
 }); 

Fiddle

Docs

closest()

find()