Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

This is the function which I wrote and am currently using in my project. I want to know if there is a better way to write it:

function pageLoader(pageIndex){

            $(".ServicesSectionWrapper,.ServicesSectionWrapper .Selector,.ServicesSection,.JournalSectionWrapper,.JournalSectionWrapper .Selector,.JournalSection,.AboutSectionWrapper,.AboutSectionWrapper .Selector,.AboutSection").hide();

        switch(pageIndex){

        case 1:
        $(".AboutSectionWrapper").fadeIn(400,function(){$("#AboutWrapper").fadeIn(400,function(){$("#ManagerWrapper").fadeIn(400,function(){$("#DeveloperWrapper").fadeIn(400,function(){$("#DesignerWrapper").fadeIn(400,function(){$(".AboutSection").fadeIn(400,function(){$(".AboutSection").addClass("PreLoadRotate")})})})})})});
        break;

        case 2:
        $(".JournalSectionWrapper").fadeIn(400,function(){$("#DateOne").fadeIn(400,function(){$("#DateTwo").fadeIn(400,function(){$("#DateThree").fadeIn(400,function(){$("#DateFour").fadeIn(400,function(){$("#DateFive").fadeIn(400,function(){$("#DateSix").fadeIn(400,function(){$("#DateSeven").fadeIn(400,function(){$("#DateEight").fadeIn(400,function(){$(".JournalSection").fadeIn(400,function(){$(".JournalSection").addClass("PreLoadRotate")})})})})})})})})})});
        break;

        case 3:
        $(".ServicesSectionWrapper").fadeIn(400,function(){$("#AppsWrapper").fadeIn(400,function(){$("#ResponsiveWrapper").fadeIn(400,function(){$("#DigitalWrapper").fadeIn(400,function(){$("#PTRWrapper").fadeIn(400,function(){$(".ServicesSection").fadeIn(400,function(){$(".ServicesSection").addClass("PreLoadRotate")})})})})})});
        break;

        }
    }

And here is the HTML:

<div class="AboutSectionWrapper">

 <div class="Selector" id="AboutWrapper"></div>
 <div class="Selector" id="DesignerWrapper"></div>
 <div class="Selector" id="ManagerWrapper"></div>
 <div class="Selector" id="DeveloperWrapper"></div>

<div class="AboutSection">
 <div class="Indicator"></div>
</div>

This is a menu (AboutSectionWrapper) which has selectors as you can see. I want to load the menu first and then sequentially load the selectors and finally fade in the menu indicator and add a CSS class which has a transition in it.

It is the same for other cases but for example in CASE 2, there are more selectors!

share|improve this question

2 Answers 2

up vote 4 down vote accepted

display:none instead of .hide()

Instead of hiding them like this:

 $(".ServicesSectionWrapper,.ServicesSectionWrapper .Selector,.ServicesS...hide()

Add a class common to each (on the HTML) and with that class, use display:none.

.commonClass{display:none}

Sequential Animation

Well, I haven't found anything elegant for sequential jQuery animation (or haven't looked that hard). However, I have made this which executes functions sequentially. It isn't a generic solution for animation, only for this situation.

function animateQueue(q){

    // Get the next item
    var next = q.shift();

    // Remove from the array the selector (or jquery object) and function name
    var obj = $(next.shift());
    var functionName = next.shift();

    // Add in the callback that runs after the animation
    next.push(function(){
       if(!q.length) return; // When done, don't continue
       animateQueue(q);      // otherwise, pass on the queue
    });

    // Run the animation
    jQuery.fn[functionName].apply(obj,next);    
}

All you need to do is write the queue like so:

[selector OR jQuery object, animation function, params except the callback]
animateQueue([
    ['#red','fadeIn',400], 
    ['#blue','fadeIn',400],
    ['#green','fadeIn',400],
    [$('#red'),'fadeOut',400], 
    [$('#blue'),'fadeOut',400],
    [$('#green'),'fadeOut',400]  
]);

Added for a jQuery object since...

Cache objects

to avoid fetching them from the DOM everytime

var red = $('#red');
var blue = $('#blue');
var green = $('#green');

animateQueue([
    [red,'fadeIn',400], 
    [blue,'fadeIn',400],
    [green,'fadeIn',400]
]);

Taking it further

Since the only vital function for the pageLoader with respect to this functionality is selecting what to animate, we can move out everything but the selection. Further, we can store the animation queues into an array, and have pageIndex select them instead. pageIndex should be 0-indexed, so we subtract 1:

var queues = [
  first : [...],
  second : [...],
  third : [...]
];

function animateQueue(q){...}

function pageLoader(pageIndex) {
  animateQueue(queues[pageIndex-1]);
}
share|improve this answer
    
+1 great answer, you did the queue like i said in the comment but more elegant. –  Marco Acierno Apr 18 at 23:24
    
Just before Cache Objects, why do you mix selector strings and calls to $(selector_string)? –  David Harkness Apr 19 at 2:59
    
@DavidHarkness That's a jQuery object. I realized later that they could be cached outside in advance, rather than repeatedly calling them in the queue. –  Joseph the Dreamer Apr 19 at 3:25
    
What I meant is why pass in a selector string on line 1 and then pass in the same $(selector string) a few lines later? –  David Harkness Apr 19 at 3:41
    
@DavidHarkness Because the script can use either a selector or a jQuery object. Check the demo. –  Joseph the Dreamer Apr 19 at 4:13
  1. Dont put so much }}}} it just looks bad and you will not know if everything is ok.
  2. I'm wrong or just a

    $(".AboutSectionWrapper, #AboutWrapper, #ManagerWrapper, #DeveloperWrapper, #DesignerWrapper, .AboutSection").fadeIn(400, function()
                    {
                        $(".AboutSection").addClass("PreLoadRotate");
                    });
    

Could do the same thing of what you write above? Only if you want an effect "first one then another" you could separate the things.. but here are the same time and no differences at all.

The same is for the others...

    function pageLoader(pageIndex) {

        $(".ServicesSectionWrapper,.ServicesSectionWrapper .Selector,.ServicesSection,.JournalSectionWrapper,.JournalSectionWrapper .Selector,.JournalSection,.AboutSectionWrapper,.AboutSectionWrapper .Selector,.AboutSection").hide();

        switch (pageIndex) {

            case 1:
                $(".AboutSectionWrapper, #AboutWrapper, #ManagerWrapper, #DeveloperWrapper, #DesignerWrapper, .AboutSection").fadeIn(400, function()
                {
                    $(".AboutSection").addClass("PreLoadRotate");
                });
                break;

            case 2:
                $(".JournalSectionWrapper, #DateOne, #DateTwo, #DateThree, #DateFour, #DateFive, #DateSix, #DateSeven, #DateEight, .JournalSection").fadeIn(400, function()
                {
                    $(".JournalSection").addClass("PreLoadRotate");
                });
                break;

            case 3:
                $(".ServicesSectionWrapper, #AppsWrapper, #ResponsiveWrapper, #DigitalWrapper, #PTRWrapper, .ServicesSection").fadeIn(400, function()
                {
                    $(".ServicesSection").addClass("PreLoadRotate")
                });
                break;

        }
    }

DateOne, #DateTwo, #DateThree, #DateFour, #DateFive, #DateSix, #DateSeven, #DateEight

Sounds like something which belong to the same thing, why not make a general <div> (or another container) and let it appear? his child will appear with him

<div id="dates">
 <div id="DateOne"></div>
 <!-- etc !-->
</div>
share|improve this answer
    
No I want to sequentially fade in these elements, that is why I did this! –  Farhoud Zolhayat Apr 18 at 23:00
    
Well I tested it and I don't see any "sequentialfade" but anyway in this case you should indent your code better –  Marco Acierno Apr 18 at 23:02
    
Ok I got this idea: Make an integer (pointer), an array with all elements to fadeIn and a single function where you increment the pointer and set fadeIn to the current pointer if it reach the max apply the class. It could be more readable (since you use the same fade time) –  Marco Acierno Apr 18 at 23:11

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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