Wrong object referenced when click event is activated

I'm attempting to create an accordion effect for a document where when you click on an <h1> the rest of the document (the <div.container>) with toggle sliding up and sliding d开发者_C百科own. I keep running into a problem however. Here is the code:

HTML

<article>
    <h1>Title</h1>
    <div class="container">
        //...
    </div>
</article>

<article>
    //...
</article>

<article>
    //...
</article>

CoffeeScript:

articles = $('article').toArray()

for article in articles
    #console.log $('.container', article).parent().attr('id')
    $('h1', article).click ->
        $('.container', article).slideToggle 'slow'

When I use the article variable in say... the console.log It rotates through the articles and prints back their ids. But when I go to click any of the <h1> elements, it always collapses the last <article>s <div.container>.

I think this is because the article variable is stored outside of the scope of the for loop in CoffeeScript and the click doesn't execute until after the loop has already completed.

If this is true, how do I guarantee that the right object is being referenced when the click event is executed? Would it be better just to use a for i in [0...3] loop and just reference the array directly? Is the problem something else entirely? Thank you for your help!

For those who may not be familiar with coffeeScript, here's the javaScript that is compiled (just ignore the _results variable):

var articles
articles = $('article').toArray();
_results = [];

for (_i = 0, _len = articles.length; _i < _len; _i++) {
  article = articles[_i];
  _results.push($('h1', article).click(function() {
    return $('.container', article).slideToggle('slow');
  }));
}

---

The reason you are seeing this behavior is because the article referenced in the click events is not evaluated until the handler is triggered, and at that point in time, it is set to the last article which was evaluated in your loop.

This may work a bit better for you (sorry, not sure of the coffee script implementation):

$('article h1').click(function() {
    $(this).next('.container').slideToggle('slow');
});

---

Rocco's answer is correct. Let me expand on it:

This is a common area of confusion: Only functions create scope in JavaScript (and CoffeeScript), which means that when you do something asynchronous within a loop, you have to remember to "capture" the variable. The preferred way to do that in CoffeeScript is with the do syntax, which lets you write

for article in articles
  do (article) ->
    $('h1', article).click ->
      $('.container', article).slideToggle 'slow'

which compiles to the equivalent of

for article in articles
  ((article) ->
    $('h1', article).click ->
      $('.container', article).slideToggle 'slow'
  )(article)

That way, each click callback sees its own iteration of article, not the one that changes value as you go through the loop. I talk about this briefly in my PragPub article A CoffeeScript Intervention.

---

Here's another way to do it (in javascript/jQuery) by:

1. Modifying the initial selector to pick all the h1 tags under article
2. Using this.parentNode to get the context from the click
3. Then look for the .container object under that common parent.

This way, we don't have to store any state across the click as we can just find the container that shares the same parent with the item that is clicked on. This method also doesn't rely on the exact position of the container object relative to the h1 that is clicked on - it just needs the container to share the same parent so it's more flexible in future layout.

$('article h1').click(function() {
    $('.container', this.parentNode).slideToggle('slow');
});

---

I don't know anything about Coffeescript, but the compiled Javascript has a pretty obvious bug.

This line:

article = articles[_i];

Should read like this:

var article = articles[_i];

With this change, the article variable will be considered part of the calling scope and reference the appropriate element when it comes time to call your handler (rather than referencing an implicit global variable). That's all you need to fix your bug.

Hopefully there's a way to specify that in Coffeescript, but it seems pretty horrendous that for article in articles doesn't do that automatically.