Should event handlers be the last statement in a method?

Question

I have a method which is called onClick of some element. In that function I have an event handler( JQuery $().click() ), that detects the click of a button and performs some action.

I have noticed that the event handler works fine as long as it is the last block of statement in the function and is skipped altogether if there lie certain code block after it. Why is that happening?

EDIT Adding code

  function launchPopUp(ID) {
            if ($('#popUp').is(':hidden')) {
                var serial = ID.id; // ID of the element or area clicked.
                var headData = 'SVG PopUp';
                var entData = 'Enter the data you want to store:';
                var ok = "No";
                var input = "";

                var header = addHeader(headData);
                var enterable = addEnterable(entData);
                var buttons = addButtons();

                $('#popUp').append(header);
                $('#popUp').append(enterable);
                $('#popUp').append(buttons);
                $('#popUp').show();

                $('#btnSubmit').click(function() {
                    input = document.getElementById('txtData').value;
                    if (input != "") {
                        ok = "yes";
                        $(ID).css('fill', 'green'); // Change colour to green only if some valid data is entered.
                        closePopUp();
                    }

                });
                var collData = { "ID": serial, "header": headData, "OK": ok, "input": input };
                collection.push(collData);
            }  
        }

Control is jumping straightaway to the code block after the .click()

@gdoron: Why did u del ur ans? If nothing, it'd act as good reference point for other users. — Sayan, Nov 02 '12 at 11:12
What do you mean by "control is jumping"? Your code executes some statements, adds an event listener, and executes some other statements. What else did you expect? — Bergi, Nov 02 '12 at 11:35
What i meant by *"control is jumping"* is that, the code adds an event listener but the listener is never fired! Control will always go ahead and there is no way u can capture the click event.....savy? — Sayan, Nov 02 '12 at 11:37
The handler will be executed when someone clicks #btnSubmit. If you want something to happen after a click is captured, do it in the handler code and not after *attaching* the handler — Bergi, Nov 02 '12 at 12:52
KeyBrd, what Bergi says is the problem in your thinking which makes your code work in a different way you are expecting it to work, I tried to explain this in my answer. Take a look if that helps you. — Christoph, Nov 02 '12 at 13:02
Will the down voter please care to explain the down vote.....! — Sayan, Nov 02 '12 at 14:27

Brandon Montgomery · Answer 1 · 2012-11-02T12:28:57.583

I think this is what you want:

function launchPopUp(ID) {
        if ($('#popUp').is(':hidden')) {
            var serial = ID.id; // ID of the element or area clicked.
            var headData = 'SVG PopUp';
            var entData = 'Enter the data you want to store:';
            var ok = "No";
            var input = "";

            var header = addHeader(headData);
            var enterable = addEnterable(entData);
            var buttons = addButtons();

            $('#popUp').append(header);
            $('#popUp').append(enterable);
            $('#popUp').append(buttons);
            $('#popUp').show();

            var collData = { "ID": serial, "header": headData, "OK": ok, "input": input };
            collection.push(collData);

            $('#btnSubmit').click(function() {
                input = document.getElementById('txtData').value;
                if (input != "") {
                    collData.OK = "yes";
                    $(ID).css('fill', 'green'); // Change colour to green only if some valid data is entered.
                    closePopUp();
                }
            });
        }  
    }

Note that the collData is a variable containing a reference to an object. That object is added to the collection, and modified within the click handler when the btnSubmit button is clicked. This way, if the save button is never clicked, the object is still added to the collection. But if it is clicked, the object is changed, and closePopUp() is called, presumably allowing you to do what you need to do with the objects which exist in the collection variable.

do you mean to say if the *btnSubmit* button is clicked, the value of *ok* (in the object already pushed to the array) will change...?? — Sayan, Nov 02 '12 at 12:05
Ah, I just updated the code, FYI. The `ok` property should be `OK` since it's case-sensitive. — Brandon Montgomery, Nov 02 '12 at 12:29

Christoph · Accepted Answer · 2012-11-02T13:06:25.057

You are misunderstanding the event handlers.

Javascript has asynchronous nature, so (in normal cases) there is no "waiting" for an event.

You register an eventhandler like your click() and then the function is executed when (eventually) a click on that element is registered. In the meantime the execution of the rest of your code goes on.

If you want to make your code dependent on the click, you have to write this code into the function of the click handler or pass a callback to the function.

Registering Event-Handlers is a one-time process and has to be done outside your function - at the moment you are registering a new click-handler every time you call launchPopUp. E.g. if you are calling launchPopUp five times, your code

input = document.getElementById('txtData').value;
if (input != "") {
    ok = "yes";
    $(ID).css('fill', 'green'); 
    closePopUp();
}

also gets executed five times as soon as you click on #btnSubmit.

Basically you have to structure your code like the following:

register eventhandler for #btnSubmit - define what is happening when the button is clicked in this function (evaluation of your inputs)
write the launchPopUp function which gets eventually executed. No eventhandler in here and no evaluation code on btnSubmit this is all done in your eventhandler.

Thats what i was missing out on "*Asynchronous Nature*"! The first two lines of your answer made everything crystal clear. — Sayan, Nov 05 '12 at 07:44

svenbravo7 · Answer 3 · 2012-11-02T11:03:55.570

0

 $('#btnSubmit').click(function() {
                    input = document.getElementById('txtData').value;
                    if (input != "") {
                        ok = "yes";
                        $(ID).css('fill', 'green'); // Change colour to green only if some valid data is entered.
                        closePopUp();
                    }

                });

Put the above outside your loadPopup function and put it in a

$(document).ready(function()
{

});

That might just solve it.

EDIT:

$('#btnSubmit').click(function()
{
    input = document.getElementById('txtData').value;
    if (input != "")
    {
        ok = "yes";
        $(ID).css('fill', 'green'); // Change colour to green only if some valid data is entered.
        closePopUp();
    }
    var collData = { "ID": serial, "header": headData, "OK": ok, "input": input };
    collection.push(collData);
});

var collData should be IN your click function, then it will be executed when you click on the submit button.

edited Nov 02 '12 at 11:03

answered Nov 02 '12 at 10:44

svenbravo7

317
1
6

I have added the code above. Does it look like i hv missed a loop?? – Sayan Nov 02 '12 at 10:47
No, but you edited the post after I was writing my answer. Sorry! – svenbravo7 Nov 02 '12 at 10:48
No problem! BTW i know I can make it work by writing the click handler itself outside the function.....bt that isnt my question. M perplexed as to why the handler is not fired if der are lines to b executed after it. – Sayan Nov 02 '12 at 10:54
But that violates the logic m trying to implement. I would like to push certain data to the array if the button is clicked and also, push certain data ***if*** it isnt! – Sayan Nov 02 '12 at 11:06
Then put the two lines `var collData` and `collection.push` also outside the click function. – svenbravo7 Nov 02 '12 at 11:12

score 0 · Answer 4 · answered Nov 02 '12 at 11:06

0

The above code will not work good if I understand it correctly. It looks like every time you launch the popup you bind a new click event to it. So if you launch the same popup twice you will have two on click event handlers bound to the object.

Accessing variables outside the closure is practical. However, you can only access the variables that has been defined before you define your closure.

Imagine that you move the definition of "ok" after you define your click event handler. In that case OK would not be defined and there will be another ok in the event handler.

(I hope I understood your question correct, please comment otherwise)

answered Nov 02 '12 at 11:06

Nils

2,041
1
15
20

I think i hvnt made myself clear enough. The issue is, control jumps to the *var collData = .....* line (second last line) w/o waiting for the btnSubmit to be clicked. Why? – Sayan Nov 02 '12 at 11:11
Because it is outside the click() function. The click-function ends with "});" – Nils Nov 02 '12 at 12:53
It's because it is outside the click-function. The click function after the "});" move it into the click function. – Nils Nov 02 '12 at 13:04

score 0 · Answer 5 · answered Nov 02 '12 at 12:38

Try this:

var launchPopUp = function launchPopUp(ID) {
    'use strict';
    var popup = $('#popUp'), //cache #popup instead of doing multiple lookups
        headData = 'SVG PopUp',
        entData = 'Enter the data you want to store:',
        submit = null, //declare a var to cache #btnSubmit instead of doing multiple lookups
        submitHandler = function (e) { //handler can be defined anywhere in this routine
            //collData should be defined in the handler
            var collData = {
                "ID": ID.id, // ID of the element or area clicked.
                "header": headData,
                "OK": "No",
                "input": document.getElementById('txtData').value
            };
            //modify collData based on inputs at time #btnSubmit is clicked.
            if (collData.input !== "") {
                collData.OK = "yes";
                $(ID).css('fill', 'green'); // Change colour to green only if some valid data is entered.
                closePopUp();
            }
            collection.push(collData);
        };
    if (popup.is(':hidden')) {
        popup.append(addHeader(headData));
        popup.append(addEnterable(entData));
        //if addButtons() defines/creates/adds #btnSubmit then you will need
        //to attach the handler after #btnSubmit exists in the DOM
        popup.append(addButtons());
        //once #btnSubmit is in the DOM, you can add the handler at any time
        //although I recommend doing it prior to showing #popup
        submit = $('#btnSubmit'); //cache #btnSubmit
        if (!submit.data('handlerAttached')) {
            //only need to attach the handler one time.
            //also note that attaching the handler does not fire the handler
            //only clicking the button, or calling the handler (i.e., submit.click()
            //or submitHandler(), etc.) will fire the handler.
            submit.click(submitHandler);
            //set flag to indicate that the handler has been attached.
            submit.data('handlerAttached', true);
        }
        popup.show();
    }
};

Also, as long as these are all defined elsewhere:

addEnterable()
addButtons()
addHeader()
closePopUp()
collection[]

your routine shouldn't have any errors preventing execution of the handler.

Should event handlers be the last statement in a method?

5 Answers5