0

I am using jQuery each function and on click on current i.e $(this) I am executing JS code, It will be more clear if you look on following code

$('.switch-cal').each(function(){
    $(this).on( 'click', function( e ){
         ....  CODE HERE ....

Please tell me correct way of use "this" with ".on" inside ".each" function.

Thank you.

Prabhu Murthy
  • 9,031
  • 5
  • 29
  • 36
Mian Majid
  • 814
  • 2
  • 8
  • 22
  • Attaching handlers in jquery loops implicitly through your selected elements. You don't need to use each. – kinakuta Sep 30 '14 at 16:25
  • 2
    What you have is not common but also not incorrect. Why do you think you are not using `this` correctly? – Felix Kling Sep 30 '14 at 16:28
  • There's no issue in your code. If you're experiencing an actual problem, you need to ask specifically about the issue. –  Sep 30 '14 at 16:32

2 Answers2

4

1. Short answer: There is no need for the each:

Do not use click inside of each... you do not need to with jQuery. It handlers collections automatically for most operations (including event handlers):

e.g.

$('.switch-cal').click(function(){
    //$(this) is the calendar clicked
         ....  CODE HERE ....

2. Try a delegated handler:

It is often more convenient to use a single delegated event handler (attached to the nearest non-changing ancestor element, or document if none is handy). The best feature of delegated events is that they can work on elements that do not even exist yet! :)

The run-time overhead is quite low as they only need to apply the selector to the elements in the bubble-chain. Unless you are processing 50,000 mouse operations per second the speed difference is negligible, so for mouse events, don't be put of by ridiculous claims of inefficiency (like the down-voter's ranting below). The benefits usually out-way any extra overhead. Nobody clicks 50,000 times per second!:

e.g.

$(document).on('click', '.switch-cal', function(){
    //$(this) is the calendar clicked
         ....  CODE HERE ....

Delegated events work by listening for events (in this case click) bubbling up the DOM to a non-changing ancestor element. It then applies the selector. It then calls the function on each matching element that caused the event.

The closer the non-changing element is to the elements in question the better, to reduce the number of tests required, but document is your fall-back if nothing else is convenient. Do not use body as it has problems relating to styling that means events may not fire.

iCollect.it Ltd
  • 92,391
  • 25
  • 181
  • 202
  • Delegating events to the `document` is not more efficient. They got rid of `.live()` because people were delegating *all* their events that way. If delegation is used, it should be kept as local as possible. –  Sep 30 '14 at 16:35
  • Please research? Have you studied the code of jQuery's event system? Please don't try to condescend. What does *"...they only need to apply to the elements in the bubble-chain"* mean? That doesn't make sense. What it has to do is after the event bubbles up to the `document`, it has to go back to the `event.target`, manually bubble up to the `document` and run an `.is()` operation on every element in between to see if it matches the selector. And it has to do it for *every* selector provided. It is not at all efficient. –  Sep 30 '14 at 16:39
  • Nope, no agreement. The claim that it is more efficient is spurious. Just because you can minimize the concern of computational overhead and not do so with the memory overhead of binding the same handler to multiple elements doesn't meant that you've demonstrated anything convincing. –  Sep 30 '14 at 16:46
  • ...I love it how people look at one example of inefficient code and say it's too small to make a difference, but then they fail to grasp the cumulative effect of using the same or similar inefficient practices everywhere in their code. Yep, if that's the only code on the page, then sure, no problem. –  Sep 30 '14 at 16:50
  • Ah yes, I must have just been offended since you couldn't possibly be rightfully corrected. I'm satisfied with my vote placement. Given a page that has only 10 click handlers bound, and a user clicks somewhere on an element nested 10 levels deep, the `.is()` is going to run 100 times, even if the click took place where there was no targeted element. Nope, can't see suggesting that to people. –  Sep 30 '14 at 17:08
  • Sorry, but I'm perfectly confident in the placement of my vote. It took far too much work on my part to get to those changes that were actually made. I think you're just having trouble accepting that you didn't really understand jQuery's event system, and were a bit embarrassed by it. You don't need to be embarrassed. Just show a little humility next time rather than making snarky, condescending comments in order to hide the fact. Now you're trying to play a game where there are observers noting my supposed failure of a personality test. That bothers me not. I'm very secure in myself. –  Sep 30 '14 at 17:51
  • Ah, the revenge down vote. Didn't notice it earlier. Now *that's* a personality indicator. :-D Have an excellent day! :-) ...oh, and now my corrections are merely a "downvoter's ranting". This just gets better and better! :-D I didn't think I'd have so much to amuse me when I clicked on this question. LOL! This is fun! –  Sep 30 '14 at 18:33
-1

@TrueBlueAussie had the correct answer. You should use a single delegated event.

$(document).on('click', '.switch-cal', function(){ //$(this) is the calendar clicked .... CODE HERE ....

Still if you want to keep your "this" scope, you may

$('.switch-cal').each(function(){ $(this).on( 'click', function( e ){ .... CODE HERE .... }.bind(this)) });

aemonge
  • 2,289
  • 1
  • 24
  • 26