onclick doesn't trigger function (button created through innerHTML)

Question

First I tried:

   document.getElementById(trade_selection[i].slug).onclick = send_trade_request(auction_a, slug_for_trading, username);

but it triggered the function before any click event took place, so I found that I need to call anonymous function in order to make it work, so I tried that:

var slug_for_trading = 'slug for trading';
var trade_selection = [{
  name: 'foo',
  slug: 'niceslug',
  selling__username: 'foo',
  price: 123
}, {
  name: 'foo1',
  slug: 'veryveryniceslug',
  selling__username: 'foo1',
  price: 123
}];

var el = document.getElementById('test');
el.innerHTML = "TRADE <br>";
for (var i = 0; i < trade_selection.length; i++) {
  var username = trade_selection[i].user_selling__username;
  var auction_a = "<a href='http://example.com/foo/" + trade_selection[i].slug + "'>" + trade_selection[i].name + "</a>";
  var trade_request_button = "<button id='" + trade_selection[i].slug + "'>Send Trade Request</button>";
  el.innerHTML = el.innerHTML + "- Auction name: " + auction_a + " | Price: " + trade_selection[i].price + trade_request_button + "<br>";
  console.log(document.getElementById(trade_selection[i].slug));
  document.getElementById(trade_selection[i].slug).onclick=function(){
  send_trade_request(auction_a, slug_for_trading, username);
  }
}

function send_trade_request(auction_a, auction_b, to) {
  alert(auction_a + " " + auction_b + " " + to);
}

<div id='test'>

</div>

and it works, except that the first button doesn't call the function at all. I can't seem to figure out why that's. Looking for some direction.

My assumption is that the element is not "in the browser" by the time you attempt to attach an event handler to it, because Javascript takes priority over DOM. Try console.log(document.getElementById(trade_selection[i].slug) before adding an event handler to see if you get undefined. If that is the case I am right. — Dellirium
– Dellirium, Commented May 4, 2017 at 10:34
@Dellirium Run my edit please, the log shows that both buttons were successfully created before the event handler. Thanks for the suggestion, and the info on Javascript priorities (learned something new). — almost a beginner
– almost a beginner, Commented May 4, 2017 at 10:43
I think I am onto what the issue is give me a moment to run it in fiddle. Ok I think I got it, rechecking and posting — Dellirium
– Dellirium, Commented May 4, 2017 at 11:03

Dellirium · Accepted Answer · 2017-05-04 11:09:54Z

4

Okay so the issue is, you are using "innerHTML" to assign the new elements to the "root" div. By saying el.innerHTML = el.innerHTML + "new element HTML". You are 'removing' the previously added event listener.

Instead use:

  var button = document.createElement('button');
  button.innerHTML = "send request: " + i;
  button.id = trade_selection[i].slug;
  el.appendChild(button);

This way you keep the previous DOM tree and append new elements. Tested and it works.

answered May 4, 2017 at 11:09

Dellirium

1,51618 silver badges32 bronze badges

Sign up to request clarification or add additional context in comments.

5 Comments

Dellirium Over a year ago

Closed it, will recreate though, so we can discuss

Dellirium Over a year ago

jsfiddle.net/3mkf6uym here it is. I think i know what you were aiming at, and that is a completly different issue then the one in question here, regardless, solved in the fiddle, so OP can take a look.

Dellirium Over a year ago

I beg to differ, you weren't mistaken, if you meant what I think you meant, which would cause issues because of function being defined within a loop and loop changing the parameters in the second iteration. It happens! That is why an externally defined registrationHandler is needed. I did not comment on that matter in the initial answer because it is not the reason the initial issue was not working but it is true that for this thing to be useful it should be in there so kudos.

Theodore K. Over a year ago

Yeah, that was exactly what I was thinking. I would have add it to the answer but I see what you're saying.

Lennholm Over a year ago

A good argument for using .forEach() instead of a for loop.

Haroldo_OK · Accepted Answer · 2017-05-04 10:45:14Z

1

The problem is that you are actually calling send_trade_request(auction_a, slug_for_trading, username); immediately.

You should wrap send_trade_request() within a function, and reference the function without actually calling it:

function myEvent(){ 
    send_trade_request(auction_a, slug_for_trading, username); 
};

document.getElementById(trade_selection[i].slug).onclick = myEvent;

answered May 4, 2017 at 10:45

Haroldo_OK

7,3033 gold badges50 silver badges86 bronze badges

4 Comments

Dellirium Over a year ago

Except that it is wrapped and is not being called. Its just anonymus

Haroldo_OK Over a year ago

That's exactly how event callbacks work: you pass for it a function without calling it, and when the event happens, only then the function is executed.

Dellirium Over a year ago

But he already did that in his example, so I do not see a point in telling him to do, what he already did. There was no immediate call, he even stated that he had to wrap it in his question.

Haroldo_OK Over a year ago

Oh, I see. He fails to do it in the first example, but In the second example, he did try wrapping it.

Collectives™ on Stack Overflow

onclick doesn't trigger function (button created through innerHTML)

2 Answers 2

5 Comments

4 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

5 Comments

4 Comments

Your Answer

Sign up or log in

Post as a guest

Related