views:

793

answers:

3

The Objective

I want to dynamically assign event handlers to some divs on pages throughout a site.

My Method

Im using jQuery to bind anonymous functions as handlers for selected div events.

The Problem

The code iterates an array of div names and associated urls. The div name is used to set the binding target i.e. attach this event handler to this div event.

While the event handlers are successfully bound to each of the div events, the actions triggered by those event handlers only ever target the last item in the array.

So the idea is that if the user mouses over a given div, it should run a slide-out animation for that div. But instead, mousing over div1 (rangeTabAll) triggers a slide-out animation for div4 (rangeTabThm). The same is true for divs 2, 3, etc. The order is unimportant. Change the array elements around and events will always target the last element in the array, div4.

My Code - (Uses jQuery)

var curTab, curDiv;
var inlineRangeNavUrls=[['rangeTabAll','range_all.html'],['rangeTabRem','range_remedial.html'],
          ['rangeTabGym','range_gym.html'],['rangeTabThm','range_thermal.html']];
        for (var i=0;i<inlineRangeNavUrls.length;i++)
        {
         curTab=(inlineRangeNavUrls[i][0]).toString();
         curDiv='#' + curTab;
         if  ($(curDiv).length)
         {
          $(curDiv).bind("mouseover", function(){showHideRangeSlidingTabs(curTab, true);} );
          $(curDiv).bind("mouseout", function(){showHideRangeSlidingTabs(curTab, false);} );
         }
        }

My Theory

I'm either not seeing a blindingly obvious syntax error or its a pass by reference problem. Initially i had the following statement to set the value of curTab:

curTab=inlineRangeNavUrls[i][0];

So when the problem occured i figured that as i changed (via for loop iteration) the reference to curTab, i was in fact changing the reference for all previous anonymous function event handlers to the new curTab value as well.... which is why event handlers always targeted the last div.

So what i really needed to do was pass the curTab value to the anonymous function event handlers not the curTab object reference.

So i thought:

curTab=(inlineRangeNavUrls[i][0]).toString();

would fix the problem, but it doesn't. Same deal. So clearly im missing some key, and probably very basic, knowledge regarding the problem. Thanks.

+1  A: 

I think you're making this more complicated than it needs to be. If all you're doing is assigning a sliding effect on mouseover/out then try the hover effect with jquery.

$("#mytab").hover(function(){
    $(this).next("div").slideDown("fast");},
  function(){
    $(this).next("div").slideUp("fast");
});

If you posted your full HTML I could tell you exactly how to do it :)

Jason
true, but that's not what's causing the problem.
CrazyJugglerDrummer
...so i get a downvote? for providing a simpler, clearer answer to the OP's problem? really?
Jason
Your answer doesn't solve the OP's problem.
Breton
Jason,et al. - dont read into the upvote on the comment, it was by accident and can't be undone
akf
Hi Jason, your tip about the hover event is appreciated but it doesn't actually solve the problem. It will clean the code up though so thanks. I also have click handlers that i omitted for the sake of clarity that also need to be bound. So the slide in out/hover etc is neither here nor there, its the targeting of the events applied, that is the problem. The closures theory offered by other answers does specifically address the problem and is also "reuseable" in the sense that regardless of what Im trying to do with the anon function i will always have to account for the scope of its params.
rism
+4  A: 

You need to create a new variable on each pass through the loop, so that it'll get captured in the closures you're creating for the event handlers.

However, merely moving the variable declaration into the loop won't accomplish this, because JavaScript doesn't introduce a new scope for arbitrary blocks.

One easy way to force the introduction of a new scope is to use another anonymous function:

for (var i=0;i<inlineRangeNavUrls.length;i++)
{
  curDiv='#' + inlineRangeNavUrls[i][1];
  if ($(curDiv).length)
  {
    (function(curTab)
    {
      $(curDiv).bind("mouseover", function(){showHideRangeSlidingTabs(curTab, true);} );
      $(curDiv).bind("mouseout", function(){showHideRangeSlidingTabs(curTab, false);} );
    })(inlineRangeNavUrls[i][0]); // pass as argument to anonymous function - this will introduce a new scope
  }
}

As Jason suggests, you can actually clean this up quite a bit using jQuery's built-in hover() function:

for (var i=0;i<inlineRangeNavUrls.length;i++)
{
  (function(curTab) // introduce a new scope
  {
  $('#' + inlineRangeNavUrls[i][1])
    .hover(
      function(){showHideRangeSlidingTabs(curTab, true);},
      function(){showHideRangeSlidingTabs(curTab, false);} 
    );
  // establish per-loop variable by passsing as argument to anonymous function
  })(inlineRangeNavUrls[i][0]); 
}
Shog9
Great stuff, thanks.
rism
+1  A: 

what's going on here is that your anonmymous functions are forming a closure, and taking their outer scope with them. That means that when you reference curTab inside your anomymous function, when the event handler runs that function, it's going to look up the current value of curTab in your outer scope. That will be whatever you last assigned to curTab. (not what was assigned at the time you binded the function)

what you need to do is change this:

$(curDiv).bind("mouseover", function(){showHideRangeSlidingTabs(curTab, true);} );

to this:

$(curDiv).bind("mouseover", 
    (function (mylocalvariable) { 
        return function(){
            showHideRangeSlidingTabs(mylocalvariable, true);
        } 
    })(curTab) 
);

this will copy the value of curTab into the scope of the outer function, which the inner function will take with it. This copying happens at the same time that you're binding the inner function to the event handler, so "mylocalvariable" reflects the value of curTab at that time. Then next time around the loop, a new outer function, with a new scope will be created, and the next value of curTab copied into it.

shog9's answer accomplishes basically the same thing, but his code is a little more austere.

it's kinda complicated, but it makes sense if you think about it. Closures are weird.

edit: oops, forgot to return the inner function. Fixed.

Breton
Thanks a bunch, bit of a coin toss for accepted answer.
rism