meyerweb.com

Skip to: site navigation/presentation
Skip to: Thoughts From Eric

Need Help With Table Row Events

Here’s a late-week call for assistance in the JavaScript realm, specifically in making IE do what I need and can make happen in other browsers.  I’d call this a LazyWeb request except I’ve been trying to figure out how to do it all [censored] afternoon, and it doesn’t [censored] work no matter how many [censored] semi-related examples I find online that work just [censored] fine, but still don’t [censored] help me [censored] fix this [censored] problem.  [doubly censored]!

I have a table.  (Yes, for data.)  In the table are rows, of course, and each row has a number of cells.  I want to walk through the rows and dynamically add an ‘onclick’ event to every row.  The actual event is slightly different for each row, but in every case, it’s supposed to call a function and pass some parameters (which are the things that change).  Here’s how I’m doing it:

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);
}

Okay, so that works fine in Gecko.  It doesn't work at all in IE.  I changed el.setAttribute('onclick',val); to el.onclick = val; per some advice I found online and that completely failed in everything.  Firebug told me "el.onclick is not a function".  Explorer just silently did nothing, like always.

So how am I supposed to make this work in IE, let alone in IE and Gecko-based and WebKit-based and all other modern browsers?

Oh, and do not tell me that framework X or library Q does this so easily, because I'm trying to learn here, not have someone else's code hand-wave the problem away.  Pointing me directly to the actual code block inside a framework or library that makes this sort of thing possible:  that's totally fine.  I may not understand it, but at least there will be JS for me to study and ask questions about.  Ditto for pointing me to online examples of doing the exact same thing, which I tried to find in Google but could not: much appreciated.

Help, please?

Update: many, many commenters helped me see what I was missing and therefore doing wrong---thank you all!  For those wondering what I was wondering, check out the comments.  There are a lot of good examples and quick explanations there.

45 Responses»

    • #1
    • Comment
    • Thu 29 May 2008
    • 1715
    Matt W. wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1720
    Justin wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1723
    David Lindquist wrote in to say...

    I believe you want something like this:

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

    • #4
    • Comment
    • Thu 29 May 2008
    • 1725
    Mike Pirnat wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1735
    Mike wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1736
    Pete B wrote in to say...

    That javascript looks very unusual,

    try this:

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

    • #7
    • Comment
    • Thu 29 May 2008
    • 1738
    Randy Saldinger wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1738
    Mike Pirnat wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1738
    Brooks wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1749
    David Lindquist wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1749
    JP LeClair wrote in to say...

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

    • #12
    • Comment
    • Thu 29 May 2008
    • 1755
    Julian Grey wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1755
    Peter Bex wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1757
    Julian Grey wrote in to say...

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

    • #15
    • Comment
    • Thu 29 May 2008
    • 1757
    Borgar wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1801
    Daniel Yokomizo wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1802
    Peter Bex wrote in to say...

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

    • #18
    • Comment
    • Thu 29 May 2008
    • 1806
    Frode Danielsen wrote in to say...

    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
    • Thu 29 May 2008
    • 1813
    • #20
    • Comment
    • Thu 29 May 2008
    • 1815
    Jeremy wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1830
    Eric Meyer wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 1856
    Jeremy wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 2001
    Borgar wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 2006
    Eric Meyer wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 2016
    Justin Makeig wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 2017
    rmaksim wrote in to say...


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

    • #27
    • Comment
    • Thu 29 May 2008
    • 2033
    Alan Gresley wrote in to say...

    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
    • Comment
    • Thu 29 May 2008
    • 2126
    Peter Wilson wrote in to say...

    @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
    • Comment
    • Fri 30 May 2008
    • 0010
    Horus Kol wrote in to say...

    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
    • Comment
    • Fri 30 May 2008
    • 0435
    ProggerPete wrote in to say...

    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
    • Comment
    • Fri 30 May 2008
    • 0436
    Peter Bex wrote in to say...

    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
    • Comment
    • Fri 30 May 2008
    • 0850
    Eric DeLabar wrote in to say...

    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
    • Comment
    • Fri 30 May 2008
    • 1248
    Eric Meyer wrote in to say...

    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
    • Comment
    • Fri 30 May 2008
    • 1532
    Tuan wrote in to say...

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

    facepalm

    • #35
    • Comment
    • Fri 30 May 2008
    • 1617
    Eric Meyer wrote in to say...

    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
    • Comment
    • Fri 30 May 2008
    • 2116
    Tuan wrote in to say...

    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
    • Comment
    • Fri 30 May 2008
    • 2131
    Eric Meyer wrote in to say...

    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
    • Comment
    • Sat 31 May 2008
    • 1356
    John Larsen wrote in to say...

    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
    • Comment
    • Sat 31 May 2008
    • 1415
    Remy Sharp wrote in to say...

    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
    • Comment
    • Sat 31 May 2008
    • 2053
    Eric Meyer wrote in to say...

    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
    • Comment
    • Sat 31 May 2008
    • 2329
    Jason Penney wrote in to say...

    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
    • Comment
    • Sun 1 Jun 2008
    • 0000
    Eric Meyer wrote in to say...

    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
    • Comment
    • Wed 4 Jun 2008
    • 2355
    Andrew Gregory wrote in to say...

    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
    • Comment
    • Tue 29 Jul 2008
    • 1001
    Rahul wrote in to say...

    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
    • Fri 16 Jan 2009
    • 2018
    Received from 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 [...]

Leave a Comment

Line and paragraph breaks automatic, e-mail address required but never displayed, HTML allowed: <a href="" title=""> <abbr title=""> <acronym title=""> <b> <blockquote cite=""> <cite> <code> <del datetime=""> <em> <i> <q cite=""> <strike> <strong>



Remember to encode character entities if you're posting markup examples! Management reserves the right to edit or remove any comment—especially those that are abusive, irrelevant to the topic at hand, or made by anonymous posters—although honestly, most edits are a matter of fixing mangled markup. Thus the note about encoding your entities. If you're satisfied with what you've written, then go ahead...


May 2008
SMTWTFS
April June
 123
45678910
11121314151617
18192021222324
25262728293031

Sidestep

Feeds

Extras