Why does this javascript onclick load code run without me explicitly telling it to?

Question

I have some javascript that changes a # hash URL and loads some HTML into an element using AJAX.

// Manually trigger a hashchange to start the app.
$(window).trigger('hashchange');


function makeContentView() {
  document.location.hash = "content";
  return false; // we don't want event propagation - just AJAX call
}


$(function () {

  $(window).on('hashchange', function () {
    // On every hash change the render function is called with the new hash.
    // This is how the navigation of our app happens.
    render(window.location.hash);
  });

  function render(url) {
    // This function decides what type of page to show 
    // depending on the current url hash value.

    switch (url) {
      case "#home":
        $("#AJAX_Parent").load("body/index_body.html #AJAX_Child");
        break;
      case "#grid":
        $("#AJAX_Parent").load("body/grid_body.html #AJAX_Child", function () {

            var contents = document.getElementsByClassName("thumbnailImage");

              for (i = 0; i < contents.length; i++) {

                // THIS FUNCTION GETS CALLED WITHOUT ME CLICKING
                contents.item(i).onclick = makeContentView();
              }

          }); // makeContentView IS ALWAYS BEING CALLED IMMEDIATELY
    ... // More switch options

^ Note that in the above example, the "contents.item(i).onclick = makeContentView();" assigns the makeContentView function to the onclick handler of my page's thumbnail images, but this makeContentView function is triggered unconditionally.

I only want makeContentView to be called when I click on one of the 6 thumbnail images with class=thumbnailImage

It's getting called because you are calling it. `makeContentView()` calls the function. What more is there to say other than... "duh"? — Niet the Dark Absol, Jan 05 '16 at 19:07
Possible duplicate of [Event binding on dynamically created elements?](http://stackoverflow.com/questions/203198/event-binding-on-dynamically-created-elements) — John Dvorak, Jan 05 '16 at 20:11
Oh my god. It's calling the function and then returning the result. -_- — Michael Lafayette, Jan 05 '16 at 20:11

score 5 · Accepted Answer · answered Jan 05 '16 at 19:03

5

makeContentView();

That is a function invocation. Leave off the () to assign the function reference.

answered Jan 05 '16 at 19:03

Taplar

24,788
4
22
35

score 1 · Answer 2 · answered Jan 05 '16 at 19:04

1

because you are invoking it, simply make it

contents.item(i).onclick = makeContentView; //remove the parenthesis from the end

answered Jan 05 '16 at 19:04

gurvinder372

66,980
10
72
94

score 1 · Answer 3 · answered Jan 05 '16 at 19:04

1

By putting () right after function name you call it immediately. Remove them to assign a function as onclick handler (you're assigning its result now).

answered Jan 05 '16 at 19:04

xersiee

4,432
1
14
23

score 1 · Answer 4 · answered Jan 05 '16 at 19:04

You want contents.item(i).onclick = makeContentView without the parentheses. The reason is that Javascript basically runs any function it finds with () at the end of it as soon as it reads it.

You don't want Javascript to run this function as soon as it reads it. Instead, you want to just tell Javascript about the function -- that is, you just want to reference the function, rather than call it.

Why does this javascript onclick load code run without me explicitly telling it to?

4 Answers4