1

I am trying to register click on all <td> elements of my array where the class is editable :

<td class="editable"> Test1 </td>
<td class="editable"> Test2</td>
<td class="editable"> Test3 </td>
<td class="editable"> Test4 </td> 

JS

function registerClickOnEditableRow() {
    for (var i=0; i<$(".editable").length; i++) {
        var currentRow = $(".editable")[i];
        $(currentRow).click(function() {
           $("#oldValueInput").val($(currentRow).text());   
        });
    }
 }

Problem : When in click on a random td element, the value of $(currentRow).text() is always equal to the last element registered.

wawanopoulos
  • 9,614
  • 31
  • 111
  • 166
  • 3
    More info about WHY this happens http://stackoverflow.com/questions/10954053/javascript-variable-scope-inside-for-loop – drys Aug 26 '15 at 14:17

5 Answers5

4

You need to use 'this'

$(currentRow).click(function() {
    $("#oldValueInput").val($(this).text());   
});
Stephen King
  • 816
  • 4
  • 14
1

You need to use the contextual this instead of a generic selector. Using jQuery, you can do:

$("td[contenteditable]").click(function () {
  alert($(this).text());
});
Praveen Kumar Purushothaman
  • 164,888
  • 24
  • 203
  • 252
0

Here is a simples way to do this.

You DO NOT need the for loop to register the click event on all td.editable elements. :)

You can simply do,

function registerClickOnEditableRow() {
    $('td.editable').click(function() {
        $("#oldValueInput").val($(this).text());   
    });
 }
AdityaParab
  • 7,024
  • 3
  • 27
  • 40
0

The obvious problem was covered by others already, but you should avoid using loads of single event handlers (i.e. per item). That is very inefficient for large numbers of items and can add a noticeable delay to the page start-up.

Instead use a single delegated on event handler, attached to a single non-changing ancestor of the items (e.g. table in this case):

$('table').on('click', 'td.editable', function(){
    $("#oldValueInput").val($(this).text());   
});

This handles dynamically added items so never needs to be reconnected. It works by listening for the event (click) to bubble up to the connected ancestor. It then applies the jQuery selector, at event time, to only the elements in the bubble chain. It then applies the callback to only the matching elements that caused the event.

This is a lot more efficient for lots of elements. The smaller additional overhead at event time will never be noticed (unless you can click 50,000 times per second!).

iCollect.it Ltd
  • 92,391
  • 25
  • 181
  • 202
0

To use $(this) like in the following example seems to be the easiest solution.

function registerClickOnEditableRow() {
  for (var i=0; i<$(".editable").length; i++) {
    var currentRow = $(".editable")[i];
    $(currentRow).click(function() {
       console.log($(this).text());
    });
  }
}

But if you have 10000 td nodes you would also register 10000 click event handler and that is to much overhead.

Better you register only one event handler to an root element (in your example the table node) and test if the event target is one of the relevant child nodes. This approach is called event delegation.

function registerClickOnEditableRow() {
  $("table").click(function(event) {
    var target = $( event.target );
    if(target.is(".editable")) {
      console.log($(event.target).text());
    }
  });
}

And here you can see the jquery way of event delegation:

function registerClickOnEditableRow() {
  $("table").on( "click", ".editable", function() {
    console.log($(this).text());
  });
}
netzzwerg
  • 386
  • 4
  • 9