Question

I am developing/have developed a few systems now, I have come across a querk in my programming, I seem to delegation all my JQuery functions to the document like so:

$(document).on('click', '.modal-editor', function () {
    //code
});

$(document).on('click', '.another-class', function () {
    //code
});

$(document).on('click', '#another-id', function () {
    //code
});

Is there anything wrong with this?

UPDATE:

Ok so in essence there is nothing wrong with this until:

  1. The applications reaches a scale where delegation needs to be more localised to prevent slowing down UI. "Every event that takes place needs to run every selector against every element between the e.target and the document"

  2. Delegation like this will increase the chances of unexpected behaviour like propagation if nested functions are used.

Referring back to my question (best practice), best practice would be to delegate the event to the closest static element as possible to attempt to avoid the above.

Was it helpful?

Solution 2

Yes. In itself there isn't, nto really, but delegating all events from the document (root of the DOM) does meant that all click events, including those you're not interested in will be handled at least partially:

$(document).on('click', '$another-id', function(){});

is a particularly bad idea, in that respect: you're after a single element, but if I click anywhere in the body, jQ will do something like:

if (event.target.id == 'another-id')
{
    return callback.call(event.target, $(event));//where callback is the anonymous function you passed to on
}
return event;

So all click events result in a function call. This can slow your UI down.
By no means should you stop delegating events, but bind the listeners wisely. If all the DOM elements you want to use are contained within the #container div, for example, then bind your listeners there. If you want to handle navigation events, bind the listener to the node that contains all the navigation elements your after
Add to that that, if you cancel an event, but failed to stopPropagation, the event will still end up invoking all your other listeners that might be queued. Returnign false can also cause trouble, seeing as, in jQ, return false is translated to e.preventDefault(); e.stopPropagation(). So be careful when dealing with nested elements, if you want to handle a click on links in your page, but also on elements like <a class='navigation'>, both handlers might be called, depending on the selectors used:

$(document).on('click', 'a', function(){});//is called for all links
$(document).on('click', 'a.navigation', function(){});//is called for navigation

Which will be invoked first? Which would you want to use in a given situation? There's room for error here, that shouldn't be there:

$('#navContainer').on('click', 'a.navigation', function(){});//is called for all links

At least makes things a tad safer, clearer and lighter, too.

If you want to delegate an event, using an ID selector, and the element already exists in the DOM, don't delegate:

$('#another-id').on('click', function(){});

is shorter, and will likely even be faster anyway

OTHER TIPS

"Is there anything wrong with this?"

Yes. Every event that takes place needs to run every selector against every element between the e.target and the document. That's unnecessarily expensive.

Basically like this simplified pseudo code:

// start on the e.target
var node = e.target;

// For the target and all its ancestors until the bound element...
while (node && node !== this) {

    // ...test the current node against every click selector it was given
    for (var i = 0; i < click_selectors.length; i++) {

        // If a selector matches...
        if ($(node).is(selector)) {
            // ...run the handler for that selector
        }
    }
    node = node.parentNode;
}

Also, if your code or some other code you loaded binds a handler between the e.target and the document, and calls e.stopPropagation(), the event will never reach the document, and so yours will not fire.

It's much better to keep your delegation as close to the intended element(s) as possible. And use it primarily for dynamically loaded elements because of the extra overhead it adds, especially in the case of events that fire rapidly.

Delegation is great when you want a particular event to apply to multiple elements, without each element having its own event handler. In the instance above this is likely for the first two methods.

Where there will only ever be one element with that matches the criteria in theory it might have a performance implication, depending on the size of your document, due to every event having to be tested against the handler filter to see if it matches.

Also, if every method is added as a delegate, and you are frequently loading and uploading sections of the page those events are going to hang around longer than the elements on the page that they belong to.

You can delegate your handlers to something other than the document element, such as a surrounding div or something instead perhaps in this kind of scenario. Or use event namespacing to make sure events aren't being added multiple times.

There is nothing wrong with your code,

$(document) is not necessary to be place always in event delegation, that selector refers to the closest parent element to which your elements are added dynamically:

Syntax:

$(closest parent selector).on('event','target selector', function(){
});

You can also use document.body

$(document.body).on('event','target selecor',function(){
});

Side Note: For existing DOM elements you can use normal click event.

$('selector').click(function(){
});

OR

$('selector').on('click',function(){
});

For more information refer to .on() API documentation.

Licensed under: CC-BY-SA with attribution
Not affiliated with StackOverflow
scroll top