ansaurus

Question

possible reasons chained effects would break a toggle in jquery?

Answer 1

+2 A:

You should use this instead of a selector in the callback, like this:

$('#show_hide_comments').toggle(function() {
    //alert('1');
    $('#show_hide_comments').attr('src','images/up.png');

    $('.comments').fadeTo('slow', 0.01, function()
    {
        $(this).slideUp('slow');
    });

},function()
{
    $('#show_hide_comments').attr('src','images/down.png');

    $('.comments').slideDown('slow', function()
    {
        $(this).fadeTo('slow', 1);
    });
});

If you don't do it like this (use a selector), and you have say 10 .comments elements, bad things can happen with the animation queue. You have to remember the callback fires for each animated element, so if 10 .comments fade out, they'll each queue a .slideUp() on each .comments element again, suddently causing 100 animations, not 10.

By using $(this) you're calling .slideUp() for the element that just finished fading out, not any others.

Actually, on second thought, there are animations, they can just be queued, like this:

$('#show_hide_comments').toggle(function() {
  $(this).attr('src','images/up.png');
  $('.comments').fadeTo('slow', 0.01).slideUp('slow');
}, function() {
  $(this).attr('src','images/down.png');
  $('.comments').slideDown('slow').fadeTo('slow', 1);
});

Are you getting the same result if you're adding to the queue immediately like this?

One more note, there's some invalid HTML going on here:

<input id="email" "name="email" type="text" value="your email">

You should remove that extra " before name="email" to avoid other issues :)

Nick Craver 2010-07-21 01:28:01

While true that's almost certainly irrelevant here.

cletus 2010-07-21 01:28:43

He should also use it for the `#show_hide_comments` in `.toggle()`. I don't see how that would change the outcome, though.

patrick dw 2010-07-21 01:29:49

@cletus - It is...I added reasoning onto how this happens, and quickly causes problems in IE for example, with low thresholds for scripts.

Nick Craver 2010-07-21 01:30:39

@Nick yea I had it as `this` before and there is only one div with a class of .comments

ian 2010-07-21 01:32:25

@patrick - I tried to address this as clearly as possible, basically you're causing n^2 animation queues, if you have more than a few elements this can get expensive *real* fast (IE blows up pretty early).

Nick Craver 2010-07-21 01:32:27

@ian - In that case...what "breaks"? Does the `fadeTo` not happen, does it stop and not slide up?

Nick Craver 2010-07-21 01:33:14

Nick - Your update is indeed clear. I left the comment before I saw that.

patrick dw 2010-07-21 01:34:31

The entire second half of the `toggle()` fails. The `#show_hide_comments` wont change images, and the second chain that shows the `.comments <div>` again will not happen. Basically only the first half will happen, but if I comment out the `slideUp` then the `toggle` works normally.

ian 2010-07-21 01:36:15

@ian - If you have only one `.comments` I'm not seeing a problem with only the code you posted...can you post the markup that goes with it?

Nick Craver 2010-07-21 01:43:01

@Nick, ok I added it on but the thing is when I do this code. Separately from the rest of the page it runs fine, that's why I a trying to identify possible conflicts because the whole thing is too large to look at really.

ian 2010-07-21 01:48:28

@ian - Is it behaving the same way in every browser?

Nick Craver 2010-07-21 01:51:38

In the latest FF and Safari on OS X and Chrome on Windows yes... I did notice though if I keep clicking the toggle before one of the toggles completes it keeps going back and forth but once I let it fully toggle it is dead.

ian 2010-07-21 01:56:27

@ian - Just curious here, take a look at: `<div class="email"><input id="email" "name="email" type="text" value="your email"></div>`, rey removing that extra quote before `name=` on there. I'm still trying to narrow down a cause here for your main issue, but that can lead to other issues.

Nick Craver 2010-07-21 02:10:03

So I took out that extra quote and the page works! Then I put it back in and it still works correctly... Don't know if that was the fix or what but you can put it in as answer and ill give it to yah for sure!

ian 2010-07-21 02:21:57

@ian - Added...hopefully the other addition simplifies your code a bit as well :)

Nick Craver 2010-07-21 02:25:02

Answer 2

A:

For me this works perfectly in Firefox 3.6, Chrome 5 and IE 8 (with and without DOCTYPE, which I'd suggest you also add for many reasons). I took your code and made it static HTML for testing purposes.

If you click it repeatedly in quick succession it will break however but that's because you're not handling the animation queue correctly. It's a good practice to stop() animations. You don't even have to do it repeatedly. If you click it twice really fast you may break it as well.

Also, inside event handlers you generally want to use this to refer to the source of the event (if not the source from the event object) rather than re-selecting, which could have unforeseen (and typically undesired) consequences.

Lastly, animations can be queued directly without using a callback.

Try:

$('#show_hide_comments').toggle(function() {
  $(this).attr('src','images/up.png');
  $('div.comments').stop().fadeTo('slow', 0.01).slideUp('slow');
}, function() {
  $(this).attr('src','images/down.png');
  $('div.comments').stop().slideDown('slow').fadeTo('slow', 1);
});

and tell me what happens.

cletus 2010-07-21 02:22:25

Is this queuing of effects better than chaining?

ian 2010-07-21 02:29:43

ansaurus

tags:

views:

answers:

possible reasons chained effects would break a toggle in jquery?

related questions