jQuery click event firing immediately

Question

I have the following code in the web app I'm developing:

var bgcolor = '';
var row = 0;
for(var i = 0; i < result.data.length; i++)
{
    // Set alternating background colors
    bgcolor = ( i%2 == 0) ? ' style=\'background-color: white;\'': ' style=\'background-color: whitesmoke;\'';
    $( "div.row" ).append("\
        <div class='search_container'"+bgcolor+">\
            <div class='search_result_client_heading'>"+result.data[i]['client_name']+"</div>\
            <div class='search_result_industry_heading'>"+result.data[i]['industry_name']+"</div>\
            <div class='search_result_yield_heading'>Saved: "+result.data[i]['actual_impact']+" "+result.data[i]['impact_type_name']+" / "+formatCurrency(String(result.data[i]['actual_savings']))+"</div>\
            <div class='search_result_problem_heading'>"+nl2br( result.data[i]['problem'] )+"</div>\
            <div class='detail_button_holder'>\
                <a class='btn search_result_button' id='view_results_"+result.data[i]['record_id']+"' href='#'>View details &raquo;</a>\
            </div>\
        </div>");

    $( "#view_results_"+result.data[i]['record_id'] ).click( show_result_dialog(i, result) );
}

and then a separate named function that simply populates the created div with different values, depending on which button is clicked:

function show_result_dialog(row, result)
{
    $( "#search-result-dialog-client" ).empty().append("<div class='search_result_label'>Client</div><div class='search_result_value'>"+result.data[row]['client_name']+"</div>");
    $( "#search-result-dialog-industry" ).empty().append("<div class='search_result_label'>Industry</div><div class='search_result_value'>"+result.data[row]['industry_name']+"</div>");
    $( "#search-result-dialog-contact" ).empty().append("<div class='search_result_label'>Contact</div><div class='search_result_value'>"+result.data[row]['contact_name']+"</div>");
    $( "#search-result-dialog-journey" ).empty().append("<div class='search_result_label'>Journey</div><div class='search_result_value'>"+result.data[row]['journey_name']+"</div>");
    $( "#search-result-dialog-focus_area" ).empty().append("<div class='search_result_label'>Focus Area</div><div class='search_result_value'>"+result.data[row]['focus_area']+"</div>");
    $( "#search-result-dialog-problem" ).empty().append("<div class='search_result_label'>Problem</div><div class='search_result_value'>"+result.data[row]['problem']+"</div>");
    $( "#search-result-dialog-approach" ).empty().append("<div class='search_result_label'>Approach</div><div class='search_result_value'>"+result.data[row]['approach']+"</div>");
    $( "#search-result-dialog-tactics" ).empty().append("<div class='search_result_label'>Tactics</div><div class='search_result_value'>"+result.data[row]['tactics']+"</div>");
    $( "#search-result-dialog-delivery_date" ).empty().append("<div class='search_result_label'>Delivery Date</div><div class='search_result_value'>"+result.data[row]['delivery_date']+"</div>");
    $( "#search-result-dialog-impact_type" ).empty().append("<div class='search_result_label'>Impact Type</div><div class='search_result_value'>"+result.data[row]['impact_type_name']+"</div>");
    $( "#search-result-dialog-estimated_impact" ).empty().append("<div class='search_result_label'>Estimated Impact</div><div class='search_result_value'>"+result.data[row]['estimated_impact']+" "+result.data[row]['impact_type_name']+"</div>");
    $( "#search-result-dialog-actual_impact" ).empty().append("<div class='search_result_label'>Actual Impact</div><div class='search_result_value'>"+result.data[row]['actual_impact']+" "+result.data[row]['impact_type_name']+"</div>");
    $( "#search-result-dialog-estimated_savings" ).empty().append("<div class='search_result_label'>Estimated Savings</div><div class='search_result_value'>"+formatCurrency(String(result.data[row]['estimated_savings']))+"</div>");
    $( "#search-result-dialog-actual_savings" ).empty().append("<div class='search_result_label'>Actual Savings</div><div class='search_result_value'>"+formatCurrency(String(result.data[row]['actual_savings']))+"</div>");
    $( "#search-result-dialog-nps" ).empty().append("<div class='search_result_label'>NPS</div><div class='search_result_value'>"+result.data[row]['nps']+"</div>");
    $( "#search-result-dialog-keywords" ).empty().append("<div class='search_result_label'>Keywords</div><div class='search_result_value'>"+result.data[row]['keywords']+"</div>");
    $( "#search-result-dialog" ).dialog( "open" );
    return false;
}

in the code result is an array of data returned by an AJAX call (this code is all in the success function)

My problem is that the click handler is firing immediately upon page load. I was hoping someone could explain to me why that is, and if there is a way around that, since I need to pass that function the i value from the loop, so if I house an anonymous function in the click event, then I have problems with closure, and always end up with the last i value, so every button opens the same data. I have tried various techniques around that without success, including returning an anonymous function inside of the main anonymous function and vice versa. I've spent the last two days on Google trying to find the solution, and so far nothing has worked.

Answer 1

You have at least two problems that I can see.

The first, as identified by other posters, is not writing function() { ... } around the call to show_result_dialog().

The second, is using .click instead of .on. The former method is (confusingly) used to both register and to trigger click handlers, whereas the latter can only register. If you had used .on the problem would likely have been much easier to see.

The third is that you're trying to use a loop variable inside a closure, which never works. i will have the last value it had at the end of the loop, not the value it had when the call to .click was made.

The simplest fix for the latter problem is to use the data parameter to .on() to pass the required parameters:

$("#view_results_" + result.data[i]['record_id'] ).on('click', {
   row: i, result: result
}, show_result_dialog);

function show_result_dialog(ev) {
    var row = ev.data.row;
    var result = ev.data.result;
    ...
}

Other (more efficient) methods exist to solve the loop problem, but this will work and is pretty trivial to understand.

Answer 2

You are calling the function yourself here .click( show_result_dialog(i, result) ); There is a difference between invoking a function show_result_dialog() and passing its reference show_result_dialog.

You need to either pass your arguments as event data:

.click( {index: i, result: result}, show_result_dialog );

or simply wrap it in an anonymous function:

.click( function() { return show_result_dialog(i, result) });