0

I am attempting to assign a keypress event to an element within a for loop. I know that there are issues with assigning events while in a for loop dynamically, and I have solved that for the "click" event however I am at a loss for how it should work for the keypress. (probably because I don't really understand how the "click" one works to begin with... closure avoidance is not something I fully get)

The basic setup is that there is a for loop that will print out a number of different textareas and a div underneath them. Pressing the div will send the text in the text area to the right person. What I would like to have happen is that the same message should be sent if the enter button is pressed within the text area.

for( var i in people){

 var message = $('<textarea></textarea>').appendTo(container);
 message.on( "keypress",  function(e) {
  if(e.keyCode==13){
   // code does make it in here ...
   sendMessage(people[i].name); // but this never gets run
  }
 });

 var messageButton= $('<div>Send</div>').appendTo(container);
 messageButton.on( "click", sendMessage(people[i].name) );                

}

var sendMessage = function(to) {
 return function(){
  /* do the sending of the message to the right person */
 }
}

Can anyone help me understand the following?

Why does the click function work in the first place? I am not understanding why we have to put return around the function block.

Why doesn't the keypress function work similarly?

On a more general level, how does keypress work to begin with. The function(e) should not work because 'e' isn't anything, where does that even get set?

  • Wrapped inside of a function due to iteration, otherwise you would be only invoking the message sent for the last item in the array. sendMessage probably gets ran, however the inner-method that is wrapped is probably not being invoked. – Nijikokun Jun 12 '14 at 21:48
  • If you are using jQuery anyway, you might want to use `$.each` instead of a for loop. Avoids the whole problem. There are other event better solutions, but that would be a very simple one. If you want to learn more about the general solution: http://stackoverflow.com/questions/750486/javascript-closure-inside-loops-simple-practical-example . – Felix Kling Jun 12 '14 at 21:50

3 Answers3

0

The problem with keypress in the code is that it will always send the message to latest person in people as at the moment when it is executed, i will have the latest value in it.

I probably would use forEach instead:

people.forEach(function (person) {
    var message = $('<textarea></textarea>').appendTo(container);

    // you can use keypress - http://api.jquery.com/keypress/#keypress-eventData-handler
    // see the examples in the reference
    message.keypress(function (e) {                     
      if (e.which === 13) {
          // here you should invoke the function returned by the sendMessage
          sendMessage(person.name)();
      }
    });

    var messageButton= $('<div>Send</div>').appendTo(container);

    messageButton.click(sendMessage(person.name));                
  });

with this approach you do not need to wrap the function in the sendMessage and can just call the original function in the corresponding event handler.

Alex Netkachov
  • 13,172
  • 6
  • 53
  • 85
  • I have implemented this approach and found it to be a success. Can you help me understand (or link me to a resource which explains) the syntax of sendMessage(person.name)(); (two sets of paranthesis...) what is that doing exactly? –  Jun 12 '14 at 22:22
  • Well, `()` is just executing the function (if the left-side expression returns it). Consider the following example: `var f = function () { }; f();`. In your code the `sendMessage` returns a function so it can be written as `var f = sendMessage(...); f();` or in a short form just `sendMessage(...)();` – Alex Netkachov Jun 12 '14 at 22:34
0

Clean example using jQuery. You should read more about jQuery and closures for iterations so you can easily understand what is going on.

$.each(people, function (person) {
  var $message = $('<textarea></textarea>').appendTo(container);
  var $button = $('<div>Send</div>').appendTo(container);
  var send = sendMessage(person.name);

  // Keypress handler
  $message.keypress(function (e) {                     
    if (e.which === 13) { // on enter do the following
      send();
    }
  });

  $button.click(send);                
});
Nijikokun
  • 1,514
  • 1
  • 15
  • 22
0

Here's another solution using a handwritten closure:

http://jsfiddle.net/M5NsS/1/

var people = {
    'p1': {
        name: 'john'
    },
        'p2': {
        name: 'bob'
    },
        'p3': {
        name: 'jim'
    }
};

var container = $('#container');

for (var i in people) {

    (function (name) {
        var message = $('<textarea></textarea>').appendTo(container);
        message.keypress(function (e) {
            if (e.keyCode == 13) {
                sendMessage(name);
            }
        });

        var messageButton = $('<div>Send</div>').appendTo(container);
        messageButton.click(function () {
            sendMessage(name)
        });
    })(people[i].name);

}

function sendMessage(to) {
    console.log(to);
}

As others have stated, the issue is that the event is bound with the last reference to 'i' in the loop. Using a closure solves this issue while still allowing you to use your for..in loop.

Another thing to note is that if you are not dynamically appending these elements to the DOM after binding, there is no reason to use jquery's .on(). You can directly bind .keypress() and .click() handlers to the elements, as seen in my fiddle and on @AlexAtNet's answer.

But it's clunky, and I would just use jquerys $.each as others have already suggested.

Faris M
  • 540
  • 4
  • 10