4
\$\begingroup\$

I was wondering if someone could have a look at this BBC iPlayer style content accordion/slider I've just made, and help me to get it working properly? I'm no expert in jQuery and I'm sure I haven't done this correctly, or if there are more efficient ways of doing it.

The problem seems to be the clicking. It sometimes requires two clicks, and the sliding of the content, the left panel seems to re-size when animating.

Here is all of the code.

Snippet from jsfiddle:

$('#accordion > li.bg1 .heading').toggle(
    function () {
        $('#accordion > li.bg2').stop().animate({'width':'718px'},1000);
        $('#accordion > li.bg1').stop().animate({'width':'50px'},1000);
        $('#accordion > li.bg1 .heading').stop(true,true).fadeOut(1000);
        $('#accordion > li.bg1 .heading-side').stop(true,true).fadeIn(2000);
        $('a.see-more-p').css('display','none');
     },
    function () {
        $('#accordion > li.bg1').stop().animate({'width':'374px'},1000);
        $('#accordion > li.bg2').stop().animate({'width':'394px'},1000);
        $('#accordion > li.bg1 .heading-side').stop(true,true).fadeOut(1000);
        $('#accordion > li.bg1 .heading').stop(true,true).fadeIn(2000);    
        $('a.see-more-p').css('display','block');
    }
 ); 
$('#accordion > li.bg2 .heading, a.see-more-p').toggle(
    // same as above
);
$('#accordion > li.bg1 .heading-side').toggle(
    // same as above
); 
\$\endgroup\$
3
\$\begingroup\$

I'd normally suggest you post this on StackOverflow as it doesn 't work properly.

Firstly:

  1. You have the same two functions repeated over and over .... don't. Put them in one big selector.

    $('#accordion > li.bg1 .heading, #accordion > li.bg2 .heading, a.see-more-p, #accordion > li.bg1 .heading-side').toggle
    
  2. You have two headings for each. Semantically this is wrong. change your .heading-side to .heading.side. then all you need is an addClass('side'); and removeClass('side');
    also you will need:

    ul.accordion li .heading.side .right{ display: none; }
    

    to hide the >.
    To get the fadeIn/Out:

    .fadeOut(500).queue(function(next) {
        $(this).addClass("side");
        next();
    }).fadeIn(500);
    

    That looks confusing with the .queue(... but that's just to make sure it happens between the fading so the user doesn't see any oddness.

  3. The toggle only toggles for that specific context (i.e. the element being clicked) so it will toggle for each element, hence the double clickyness.
    My solution is to roll your own toggling:

    $(function() {
        var first = true;
        $('#accordion > li.bg1 .heading, #accordion > li.bg2 .heading, a.see-more-p, #accordion > li.bg1 .heading-side')
        .click(function() {
            if (first) {
                //do first stuff
            }else{
                // do second stuff
            }
            first = !first; // < --- Toggling done here!!!!
        });
    });
    

Viola! see jsFiddle for my awesomeness!

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.