0

In my web application I have a shopping cart that has an AJAX remove/delete button, which refreshes the cart and shows the new cart.

The code was like this:

$(".delBtn").on("click", function(){
    // Code here
});

What would happen is when the new cart was displayed the delete button would no longer work.

I now use the following instead: 

$(document).on("click", ".delBtn", function(){    
    // Code here
});

It works as expected, but is it the best option available? Would a senior developer or an expert think that it's bad code?

3dgoo
  • 15,716
  • 6
  • 46
  • 58
somango
  • 31
  • 9
  • 1
    Do you understand the difference between the two code snippets? That is key to answering your question. – Frédéric Hamidi Apr 07 '15 at 22:45
  • 1
    Personally, when playing with AJAX, I listen for events from $(document). I've not encountered any problems so far. – Amar Syla Apr 07 '15 at 22:46
  • 2
    Its normally recommended that you bind your delegates as far down the tree as possible. I would probably use a selector targeting the part of the cart that does not get replaced/updated. – prodigitalson Apr 07 '15 at 22:47
  • 1
    The "body" is the most outer wrapping element to the visual/interactive DOM. No need to go any higher than that: `$(document.body).on(...)` – Ryan Wheale Apr 07 '15 at 22:47
  • 1
    Reading this might be a good start : http://stackoverflow.com/questions/14879168/document-onclick-id-function-vs-id-onclick-function – β.εηοιτ.βε Apr 07 '15 at 22:47
  • @FrédéricHamidi Not 100% - I think I know why the first snippet doesn't work is because part of the DOM reloads and the $(document).ready isn't called again – somango Apr 07 '15 at 22:52
  • Document isn't a *bad* choice, it's just a slower choice. If you're going to run an app with lots of click events, you're going to want to put your delegated event handlers closer to the clicked elements so that click handlers fire faster. – zzzzBov Apr 07 '15 at 23:06

1 Answers1

4

First, why the first code snippet doesn't work.

What the first code says is "select every element currently on the page with a class of "delBtn", and, when that is clicked, run a function." However, at the time you do that, your button isn't on the page (since it hasn't been added yet), so no elements will be found. In comparison, the second method says that whenever you click the page, if the clicked element has the class "delBtn", then run the function. Because this just looks for clicks on the webpage, it will work for elements that are added later

As for the original question:

Yes and no. It is okay, but you might want to change this to something different. The way this works is that jquery listens to every click event on the document, and checks to see if the clicked element matches the selector. Because of this, jquery must run some code every time you click something on the page.

What you probably should do instead is change $(document) to whatever parent element contains the button. This will have better performance since jquery only has to check click events on the actual element that contains the button.

firefoxuser_1
  • 1,793
  • 13
  • 22
  • thanks everyone who made a comment. I agree with @Harko this answer is very good. It makes sense. Although I'm not 100% clear why the original code snippet didn't work after ajax request – somango Apr 07 '15 at 23:00
  • 1
    A great answer, and I couldn't disagree. I just want to point out that "performance" is relative here. Listening for an event which is already bubbling up the DOM tree and then performing a query selection is not a *heavy* operation. If he were to attach the event to the element which contains the button and then delete that element, he could have a memory leak, which is far less performant. Just being pedantic here ;) – Ryan Wheale Apr 07 '15 at 23:03