Unobtrusive Javascript: always acts on the last value

Forum Moderators: open

Message Too Old, No Replies

Unobtrusive Javascript: always acts on the last value

Clicking _any_ <a> gives the expected behavior for the _last_ <a>

MichaelBluejay

2:38 am on Apr 17, 2009 (gmt 0)

I'm brand-new to unobtrusive Javascript, and clearly I'm doing something wrong. I'm trying to set up some links to automatically show a hidden list, but when I click either link, it always shows the last list. This is the fewest number of lines with which I can show the problem.

<html> 
<style type=text/css>ul {display:none}</style> 
  
<div id=blogmenu>  
  <p><a href=category1.html>Category 1</a> 
  <ul id=category1.html> 
   <li>Details 1 
  </ul> 
  
  <p><a href=category2.html>Category 2</a> 
  <ul id=category2.html> 
   <li>Details 2 
  </ul> 
</div> 
  
<script type=text/javascript> 
linkies=document.getElementById("blogmenu").getElementsByTagName("a"); 
for(var i=0,l=linkies.length;i<l;i++) {  
   linky=linkies[i]; 
   detailsID = linky.href; 
   detailsID = detailsID.replace(/.*\//,""); 
   linky.onclick = function() { 
     document.getElementById(detailsID).style.display="block"; 
     return false; 
   } 
} 
</script> 
</html>

[edited by: MichaelBluejay at 2:39 am (utc) on April 17, 2009]

daveVk

4:46 am on Apr 17, 2009 (gmt 0)

Your code has linky and detailsID as global variables hence problem.

var linkies = document.getElementById("blogmenu").getElementsByTagName("a");
for(var i=0,l=linkies.length;i<l;i++) {
linkies[i].onclick = function() {
var detailsID = this.href; // this = linky
detailsID = detailsID.replace(/.*\//,"");
document.getElementById(detailsID).style.display="block";
return false;
}
}

Fotiman

1:35 pm on Apr 17, 2009 (gmt 0)

MichaelBluejay, as daveVK pointed out, linky and detailsID ar global variables, though it's only the detailsID that causes a problem in your particular case. daveVK moved the processing part into the handling of the onclick event. Another alternative would have been to use closures:


var i, l, linky, detailsId, linkies = document.getElementById("blogmenu").getElementsByTagName("a"); 
for (i = 0, l = linkies.length; i < l; i++) { 
  linky = linkies[i]; 
  detailsID = linky.href; 
  detailsID = detailsID.replace(/.*\//,""); 
  linky.onclick = (function(d) {
    return function() {
      document.getElementById(d).style.display = "block"; 
      return false;
    };
  })(detailsID);
}

In other words, you are executing a function, passing in detailsID, and the return value of that function is an anonymous function which which had the detailsID in it's scope as the variable d. That returned function is what gets assigned to your onclick event handler.

Either method should work. daveVK's method will get the href and perform the replace each time the link is clicked, and the closure approach will do it only once when you assign the event handler. One method might yield slightly better performance than another depending on how many links you are doing this for, but both should get the job done.

[edited by: Fotiman at 1:37 pm (utc) on April 17, 2009]

MichaelBluejay

3:02 pm on Apr 17, 2009 (gmt 0)

Fotiman, that's a hell of an answer, thanks! I now see two ways of doing it and the differences between them. I only barely understand the second version, so I'm going to use the first one for now, since the processing time should be negligible. But I've saved both of them to my scrapbook for future reference. By the way, the semicolon in the third-to-last line seems superfluous (and seems to kill the script), if you want to owner-edit that out for reference. Thanks again for your help.

Fotiman

4:26 pm on Apr 17, 2009 (gmt 0)

The semicolon in the third to last line is not superfluous and does not kill the script. It is the semicolon for the return statement on the 7th line. I've verified that the script works in IE, Firefox, and Chrome.

MichaelBluejay

2:23 am on Apr 18, 2009 (gmt 0)

I see. I had reformatted the script to try to make it more understandable, and I mistakenly had the semicolon after the second-to-last } rather than the third-to-last instance (which is the 4th-to-last line for me, 3rd-to-last line in your example). FWIW, it does work without the semicolon, since JavaScript is more forgiving than other languages about missing semicolons, but I know it's bad practice to not include them.

daveVk

3:03 am on Apr 18, 2009 (gmt 0)

Defining the function inline as in both these solutions will result in a new function being created for each link, if the number of links is large it would be more efficient to use the same function on each link, set onclick=show

function show() {
var detailsID = this.href.replace(/.*\//,"");
document.getElementById(detailsID).style.display="block";
return false;
}

If using closures you should reset the onclick to null on page unload to avoid memory leakage.