Need Help With Table Row Events

var event = '5'; // in the real code this is passed into the surrounding function
var mapStates = getElementsByClassName('map','tr');
for (x = 0; x < mapStates.length; x++) {
	var el = mapStates[x];
	var id = el.getAttribute('id');
	var val = "goto('" + id + "','" + event + "');";
	el.setAttribute('onclick',val);
}

Comments (45)

1. How about

   var event = ‘5’; // in the real code this is passed into the surrounding function
   var mapStates = getElementsByClassName(‘map’,’tr’);
   for (x = 0; x < mapStates.length; x++) {
   var el = mapStates[x];
   var id = el.getAttribute(‘id’);
   var val = function(){
   goto(‘” + id + “‘,'” + event + “‘);
   }
   el.onclick = val;
   }

2. Have you tried using closures to call goto() with the appropriate arguments? That way you can use the el.onclick syntax:

   el.onclick = function() { goto(id, event); };

3. I believe you want something like this:

   var val = function() { goto(id, event); }

4. Well, I can understand the “el.onclick is not a function” — because it’s a string instead of a function. Maybe something like (untested/talking outta my butt):

   el.setAttribute('onclick', function() { goto(id, event); });

5. If you know the number of rows your table has initially, why not assign them all an ID dynamically like row0, row1, rowX when you build the page? Then you can loop through all of the elements by saying something like (this will probably be bad code, but hopefully will point you in the right direction):

   for( i = 0; i < rows; i++) {
   var currentRow = document.getElementById(“row” + i);
   /* do other stuff involving currentRow */
   }

   It won’t be as elegant as getElementsByClassName, but I thought that was only supported on the latest version of Gecko.

6. That javascript looks very unusual,

   try this:

   for (x = 0; x < mapStates.length; x++) {
   mapStates[x].onclick = function () {
   goto(this.id, event);
   };
   }

7. Not sure about setAttribute(), but I know that the value assigned to el.onclick needs to be a function object, not just a string containing code. For example:

   el.onclick = function() { goto( id, event ); }

   However, this probably won’t suffice here, because it makes a “closure,” such that the actual (last) value of the id variable will always get used in the call to goto(). (event doesn’t cause a problem because it’s value won’t change in a single call of the enclosing function.)

   My usual way of hacking around this is to move the loop body inside its own function, but there must be a better way. Hopefully someone will recommend something better, and we’ll both learn. ;-)

8. Okay, that’s pretty close actually, and happily fires onclicks in IE6 for me. The only problem is that the id and event have scoping issues and will be bound to the last values they encountered as we set up all the onclicks… But that’s fixable with some monkeying. (Unfortunately have to run — it’s carpool time — so I can’t help further right now.)

9. Are there a lot of elements in that array? There’s some issue with IE7 and javascript array sizes. Apparently you can’t have an array in js that has a lot of elements. Broke a number of our sites in the corporate intranet before we *finally* figured it out.

10. Ah, yes Randy is right. I forgot about the closure/for loop issue. Can you instead call a function to create the handler? Something like this:

    
function makeHandler(id, event) {
    return function() { goto(id, event); }
}

    for (x = 0; x < mapStates.length; x++) {
    ...
    var val = makeHandler(id, event);
}


11. why not check for browser type (window.browser) and use .attachEvent (ie) or .addEventListener (non ie)?

12. Eric if you adopt the closure can’t you call this.id (since the attribute has already been assigned) and then simply store the table rows id=event map in an accessible assc. array?

    
el.onclick = function() { goto( this.id, events_arr[id] ); }


13. One way that should work (I’ve not tested it) is to create a closure with its own variable, like so:

    el.onclick = (function(id_copy) { return function() { goto( id_copy, event ); }; })(id);

    Because the loop actually manipulates the variable id in-place, it will indeed overwrite the value. By creating a function which you directly call, you create a new closure with its own copy of id.

    Hope this helps and good luck!

    By the way, it’s better to use proper event handling using addEventListener. This will allow you to define multiple handlers, whereas setting the onclick property will simply overwrite any existing handlers.

14. events_arr[id] should read events_arr[this.id]

15. As stated above, the assigned value must be a function. Given the example I would probably do something like this:

    
var event = '5'; // in the real code this is passed into the surrounding function
var mapStates = document.getElementsByTagName('tr');
for (x = 0; x < mapStates.length; x++) {
mapStates[x].onclick = function () {
goto( this.getAttribute('id'), event );
};
}


    The function here is run with the node as context and I use a closure to remember the event variable. As a bonus, this delays processing the id attribute until actually needed.

    Also: event and goto are both reserved words in Javascript (ahem)

16. IIUC your problem can be solved by meditating upon jquery-1.2.6.js, particularly lines starting at 1972, the definition of the trigger method of the jQuery object.

17. Also, Borgar is right that goto is a reserved word. event is not, though.

18. Ok, that was annoying – submitting the form with an error wrecked the “back” button, so I just lost a complete answer…

    Anyway, Borgar has provided a working solution for the problem at hand. But here’s another solution which would solve the problem if the event variable changed during the loop, or if the id variable couldn’t be fetched through the event context element.

    el.onclick = function (id, event) {
return function () {
goto(id, event);
};
}(id, event);

    It creates a closure on both variables through a surrounding anonymous function which gets immediately called, returning the inner function as the value of the onclick property.

19. Pingback :: Thursday, May 29th, 2008 6:13pm

    Need Help With Table Row Events

    […] http://meyerweb.com/eric/thoughts/2008/05/29/need-help-with-table-row-events/ asks Hoosgot, […]

20. This is sort of a tangent, but you are hooking up your events in a very old-school way. You said you’re anti-framework, so I won’t try to convince you to use Mochikit’s excellent Signal library, but you should at least consider using something like:

    function connect(element, eventName, func) {
    if (element.addEventListener) // If the browser has proper DOM support
    element.addEventListener(eventName.substr(2), func, false); // Hook up the event the proper way
    else if (src.attachEvent) // If instead we’re dealing with that crappy IE browser
    element.attachEvent(eventName, func); // Hook up the event the screwed up MS way
    }

    to gain all the benefits of modern even hook ups.

21. Hallelujah! Thank you, everyone, who showed me how to do this and explained what Firebug was telling me– that the problem was that what I was assigning onclick wasn’t a function, as required. I thought it was telling me that onclick itself was not a valid function, which I thought it was. The definitions of functions and methods are not very separate in my mind. Like PHP and the English language, just about everything I know about JS I picked up by absorption and intuition. Unlike the English language, both my PHP and JS fluency is… partial. At best.

    And I did not say I was anti-framework. I said I wanted to learn, and using a framework would block reaching that goal.

22. My apologies, I misunderstood your opinion re:frameworks. And since I did, I might as well take a second to recommend Mochikit, as it would let you turn this:
    el.setAttribute(‘onclick’,val);
    in to:
    connect(el, “onclick”, val);
    so that you could:
    A) hook up multiple onclick events to that element
    B) have a way of unhooking the events later (connect returns and object that you can pass to “disconnect” to clear the event … something browsers aren’t always smart enough to do on their own)

    Also, you could use another Mochikit function called partial, which let’s you turn a function in to a new function with one of it’s arguments “pre-filled”, to solve your problem in yet another way:

    var gotoFunc = function(event, id) {
    goto(“‘” + id + “‘”,”‘” + event + “‘”);
    }

    var gotoForThisEvent = partial(gotoFunc, event);

    // and then inside the loop …
    var val = partial(gotoForThisEvent, id);
    connect(el, “onclick”, val);

23. If anyone is curious as to why the demo code works in some browsers and not others, I’ve made an attempt to explain it.

    Oh, and yes, event is not a reserved word. Sorry. It’s a “system” global in IE and highlights in my editor. Which are reasons enough for me to not use it.

24. You’re still misunderstanding what I said. I am not against the use of frameworks. I just don’t want people telling me to use one in this case, because I won’t learn that way, and examples based on a framework don’t help either.

25. Wouldn’t this be a good use case for event bubbling? Just attach a click handler to the enclosing table. In the handler you’ll be able to figure out the tr that fired event from a property of the event object. (Which property and how you get the event object are, of course, browser-specific.) This technique is also helpful when the number of rows is dynamic.
    With most modern frameworks, attaching the handler and determining the row are one line of code.

26. 
eval('el.onclick = function() { gotoFunc("'+id+'","'+event+'"); }');


27. Hi Eric

    I would love to maybe learn JS someday, at the momment it’s just CSS. Do you often swear when you code or is that just when you working with a language that you don’t quite understand? My only JS ever created is this. It was pointed our very quickly how it could have been coded better. I was just trilled it actuality worked.

    BTW. What has happen to CSS discuss? Is it coming back?

    Just to be cheeky. What’s the difference between a JS framework and a CSS framework? Why would you not want to use one but promote the other?

28. @Alan Gresley Swearing is an essential part of programming, there is nothing as effective as yelling “you ******* useless *****” at an inanimate object to assist with bug fixing; I find this particularly useful when coding css for those instances that I”ve used my native centre rather than the American center.

29. Isn’t this a problem related to how IE works with tr in the HTML DOM?
    
<table id="tableid">
<tr id="1">
<td>Row 1</td>
</tr>

    <tr id="2">
    <td>Row 2</td>
    </tr>
    </table>

    <script type="text/javascript">

    function some_function()
    {

    alert(this.id);

    }

    var table = document.getElementById('tableid');

    for (r in table.rows) {

    if (!isNaN(r)) {

    tRow = table.rows[r];

    tRow.onclick = some_function;

    }

    }

    </script>


    the only problem with the script above is that IE starts indexing the table row array from 1 in the for loop, instead of 0, and misses adding the function to the first row…
    couldn’t think of a quick way to fix that.

30. I think the ideal solution would be to use event delegation instead.

    Assign a click handler to the table itself that looks something like.

    function callGoto(e)
    {
    if (!e) e = event;
    var src = e.target || e.srcElement;
    while (src.nodeName != “TABLE”)
    {
    if (src.nodeName == “TR”)
    {
    var id = src.id;
    var event = this.idEventMap(id)
    return goto(id, event);
    }
    src = src.parentNode;
    }
    }

    Then all the function that is currently initialising your event handler has to do is maintain an idEventMap object attached to the table.

31. In common parlance, “method” just means a function on an object, but the difference between function and method is not made in Javascript. You can assign any function to any property on an object and it “becomes” a method.

    One real difference you can notice in Javascript is that when the function is called as a method (ie, like obj.propertyname(arg1, arg2);), there is a special variable called this that has the value of the object that the method was called on (obj in the example). If a function is called directly (not “on an object”), this has the value of the “global object”, which is window in Javascript.

32. Glad you found your solution, but here’s a little constructive criticism on performance. In general, with JavaScript, when using a for loop it’s better to set a variable for the length than to call the length property on every iteration through the loop.

    e.g.
...
var size = mapStates.length;
for (x = 0; x < size; x++) {
...


    or for brevity’s sake:
    
...
for (x = 0, size = mapStates.length; x < size; x++) {
...


    In JavaScript the value of length could change for each iteration, so it’s evaluated each time through instead of just being checked like a variable would be. With all of the JavaScript being written for today’s “Web 2.0” sites every millisecond counts, and since this is a learning exercise it’s better to learn the best practice!

33. Thanks for that, Eric— I had no idea you could set up multiple variables in the first term of a ‘for’ loop’s parameters! Makes sense that it would be a performance win, and I always like to learn best practices. I’m adding that as soon as I post this comment.

    Jeremy, I’ve no doubt I’m coding old-school, and I did try some of the functionality-testing code you suggest. It wasn’t helping (due to my botching the whole ‘onclick’ thing) so I took it out. Now that I’ve got things actually working thanks to all you awesome folks, I’ll start adding that kind of feature-detected code back in. I’ll also look into delegation and bubbling as suggested by ProggerPete and Justin.

34. oh man, it took 30 posts for someone to suggest event delegation. It should have been the first!

    facepalm

35. Unfortunately, while I get the principles behind event delegation, I don’t know how to add a click handler to the table such that it will take advantage of the routine ProggerPete posted. The actual routine I understand. Just not how to properly invoke it.

    Oh wait, no, I don’t understand one part of it, which is the ‘idEventMap’ part. Sorry.

    Oh, or how to do it so it works in IE7, which it doesn’t when I try it in such a way that it works in Gecko. Sorry again.

36. here’s some pseudo code for you. Only thing you have to worry about is getting the event target from the event object. The rest is pretty simple. If you’re using a library like Prototype, you could do all this with just a few lines.

    var handleClick = function(event) {
    // get the element inside the table that fired the event
    var el = event.target; // add logic here for cross browser

    // check if the element is a TR, if not then keep going up the
    // DOM tree til you reach one and stop when you’re at the table
    while(el.tagName != ‘TR’) {
    el = el.parentNode;
    if(el.tagName == ‘TABLE’) { // reached the table itself, passed any TR element, bail out
    return;
    }
    }

    // if you’re here, you got the TR and assuming it has an id, just call your function
    goTo(el.id, event);
    }

    ….

37. Um, you just posted more or less what ProggerPete did, except not in actual JS, and nothing about the parts that I said elude me. I do appreciate the effort, but unfortunately it seems to have gone to repeating an existing answer and not answering the existing questions.

    Update: thanks to PPK (who else?), I worked out how to add the table handler in a cross-browser fashion:

    if (t.addEventListener) {t.addEventListener('click',callGoto,true);}
    if (t.attachEvent) {t.attachEvent('onclick',callGoto);}
    

    Not the most robust, perhaps, but it works well enough. I also figured out how to handle the event determination in a way that made sense to me, which was to set up a global variable and give it the needed value in my onLoad routine.

    It was at that point I realized that event delegation probably won’t work well in my case, because not every row of the table should be clickable—in my current working file, about three out of ~35 rows are non-clickable. So it actually makes more sense for me to collect all the rows I want to make clickable and assign them their own event handlers, I think. The only way around it I can see is to do delegation and then have more code in ‘gotoCall’ that checks against a class or a list of IDs that should be ignored, and that seems kind of ugly. This table isn’t ever going to have more than 60 or 70 rows, and that’s a very padded estimate.

    But I did learn a lot more about events and event handling, for which I thank you all!

38. How about iterating over the rows and setting a property to flag whether the row should be ignored? Then use delegation with an initial test of the flag. If the flag is false just return.

39. By way of pushing best practises a little further, add this code to your JS snippets:

    addEvent = (function () {
  return document.body.addEventListener ? function (e, ev, fn) {
    e.addEventListener(ev, fn, false);
  } : function () {
    e.attachEvent("on" + ev, fn);
  };
})();

    // usage:
addEvent(element, 'click', function () {
  // do stuff
});

    It’s a pretty common design pattern – it looks for the method to attach the events and creates the appropriate function.

    Re: delegation to solve your problem. The way you’ve suggested, i.e. checking for a class to ignore particular rows would be the best way to do it (until we get data attributes in HTML5…) – but it wouldn’t be in the gotoCall function – it would be in the delegation code.

    However, for the size of the table – there’s definitely isn’t going to be a measurable performance benefit. Go with what makes more sense to you now :-)

40. Remy: if by the “delegation code” you mean the actual function gotoCall, then that’s what I meant. Something like:

    function callGoto(e) {
      if (!e) e = event;
      var src = e.target || e.srcElement;
      while (src.nodeName != "TABLE") {
        if (src.nodeName == "TR") {
          if (src.class == "one" || src.class == "two" || src.id != "blah") {
            var id = src.id;
            return goto(id, conferenceID);
          }
        }
        src = src.parentNode;
      }
    }
    

    …though there’s probably a less clumsy way to roll all the innermost “if”s together, and that innermost “if” is probably pseudocode as it stands (I don’t think el.class is really valid, is it?). But that’s basically what I was meaning. Good to know that brute-forcing a click event onto every row of tables this size isn’t going to be a big performance loss.

    Of course, absolutely none of this would be necessary if HTML5 restored the href attribute to all elements, which was one of the few things I thought XHTML2 got absolutely totally 110% correct. Then all I’d have to do is slap the necessary href on each tr element and be finished with this really stupidly simple requirement in twenty seconds instead of juggling a dozen script eggs in an attempt to make stuff clickable.

41. I totally agree on the XHTML2/href thing. I remeber thinking how much that would simplify things. Was there a reason that was dropped or was it just overlooked?

42. I dunno, Jason, but I’m just now most of the way done with a detailed rant on that very topic. I expect to post it Sunday or Monday (probably the latter).

43. Eric, if you can, please, please, please change the third parameter of addEventListener in your comment #37 to false! Having true there will set up what’s called a “capturing” event handler (explained on PPKs page) which is NOT what you want, while the IE compatible attachEvent you’ve got on the next line implements non-capturing.

    Unintended event capturing breaks pages for the minor browsers that support it (currently Opera and Safari, but soon Firefox 3).

44. Hello Eric,

    Thanks for posting this question. I was facing the same problem. Your post and the comment did help me out. Thanks.

    But going further, I am facing same problem as mentioned in comment #8. i.e “The only problem is that the id and event have scoping issues and will be bound to the last values they encountered as we set up all the onclicks…”

    You certainly must have faced same problem. It would be great if you post how you work around that issue as well.

    Thanks in advance..

    Rahul

45. Pingback :: Friday, January 16th, 2009 8:18pm

    Borgar.net » Node members or Node attributes

    […] given it a bit of thought I think that this problem stems from a slight confusion as to how DOM nodes’ attributes […]

Add Your Thoughts

• Trackback URL
• Comments RSS feed'

Name

E-mail (required but not displayed)

URI (optional)

Meyerweb dot com reserves the right to edit or remove any comment, especially when abusive or irrelevant to the topic at hand.

HTML allowed: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <em> <i> <q cite=""> <s> <strong> <pre class=""> <kbd>

---

Your Comment (Markdown supported)

if you’re satisfied with it.

Comment Preview

Δ