0

I have a DIV that when click on it, it will add another DIV inside itself. Then if I click the newly added DIV it will remove it.

HTML

<div id="container"></div>

jQuery

$('#container').on('click', function(e){

  // Add other DIV
  $( this ).append('<div class="other">XYZ</div>');

  e.stopPropagation();

  // Remove other DIV
  $('div.other').bind('click', function(event){

    $( this ).remove();

    event.stopPropagation();

  });

});

How efficient is this method if I plan to have to lot of child DIVs?

Improve this question
8
  • Seems pretty efficient... Commented Jan 13, 2015 at 21:05
  • What's "a lot", it generally doesn't matter other than that you should probably attach the click to the added element only, not all elements matching the selector, but if you're going to insert hundreds of thousands of elements, a delegated event handler is probably better. Commented Jan 13, 2015 at 21:06
  • 4
    Do not use bind, use on Commented Jan 13, 2015 at 21:07
  • 1
    Feel free to use bind -- there's nothing wrong with it. Commented Jan 13, 2015 at 21:08
  • 2
    please ignore what Engineer Dollery just commented. As the docs state: "As of jQuery 1.7, the .on() method is the preferred method for attaching event handlers to a document." Commented Jan 13, 2015 at 21:11

3 Answers 3

Reset to default
2

Attaching events when another event fires will likely cause some unintentional side effects, and subject your DOM to memory leaks.

As a general rule, attach handlers once, run often.

$(document)
    .on('click', '#container', function(e) {
        // Add other DIV
        $(this).append('<div class="other">XYZ</div>');
    })
    .on('click', 'div.other', function(e) {
        $(this).remove();
        e.stopPropagation();
    });
Improve this answer
Sign up to request clarification or add additional context in comments.

14 Comments

@eg_dac Yes, I use it all the time. Attach and forget, no messing about in document.ready.
Can I use instead of $(document) a wrapper for #container?
@redhatlab It's not a recursive search. The event "bubbles up" the tree, which is a O(N) operation wrt the number of DOM layers, until it hits the handler's delegate (document in this case).
I like to attach it to the closest non changing parent selector. Just my thoughts.
It's not about bubbling, it's how jQuery determines that the click originated from the matching element. The event handler is still bound to the parent, and fires on all clicks on the parent, but inside the event handler there's a filter that determines if e.target or any of it's parents match the selector given in the second argument, and if that selector is complicated, it gets even worse, as the filter will run on all clicks. That's why you never attach delegated event handlers to the document as it's really inefficient.
|
1

I would slightly change the JQuery so you can chain off the selector for the .append, and use .on instead of .bind:

$('#container').on('click', function(e){
    // Add other DIV
    $( this ).append(
        $('<div>').attr('class', 'other')
            .html('XYZ')
            // Remove other DIV
            .on('click', function(event){
                $( this ).remove();
                event.stopPropagation();
            });
    )
    e.stopPropagation();
});

It's untested code, but it should be functionally identical to yours. I would avoid binding on $(document) because it is inefficient due to firing events on the entire DOM. Parsing the DOM is the most time-consuming part of Javascript code execution, which is why reusing JQuery selectors as I just have is also more efficient.

Improve this answer

2 Comments

This code creates a new function every time the #container is clicked.
... which is then destroyed when said element is removed, so the event still only exists once-per. It is only created as needed, and at execution time it will very quickly traverse the DOM for the element, which it would do less efficiently if $(document) is used. The more specific a JQuery selector the more quickly it will execute.
0

It probably doesn't need any optimisation because it doesn't do a lot of thing. but if you want it to be optimal without using vanilla JS, I would go like this :

var $myDiv = $('<div class="other">XYZ</div>'); //Cache the div instead of creating it every click

$('#container').on('click', function(e){
   if($myDiv.is(e.target)) //Check if your div is the clicked target
       $myDiv.remove(); //Remove the div
   else
       $myDiv.appendTo(this) // Else append the div.
});

Just note that if the div contain children element, the if should be changed to :

if($(e.target).closest($myDiv).length)
Improve this answer

3 Comments

I didn't have any luck with the event target as the #container was always first.
@redhatlab could you explain what you mean by "always first"? Except if $myDiv has a negative z-index (which shouldn't be the case if event delegation work), this solution should work and be efficient.
I made the switch to the delegation form and I haven't had to use the target again, but in earlier versions of my script I was never able to get the target element that was clicked. Now that you mention it, yes, I was using the z-index to accommodate the other DIVs. Thank you, for the heads up. I am now stuck trying to make the DIV.other drag-able.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.