2
\$\begingroup\$

I just created a script on jQuery which loads my articles dynamically with an AJAX call if the user changes the page.

I added some cool effects like the articles move on the left, then the new ones appear in fade while a loading black screen cover the articles.

The thing is, I'm afraid my code is too heavy and not really well optimized. Could you give me your feedback about it? By the way, do you have any idea if Google is going to be able to read all my articles with that system?

$(".pages li a").click(function(e){ // IF PAGE NUMBER IS CLICKED
    e.preventDefault();
    $(".pages li a").removeClass("active");
    $(this).addClass("active");


    //VARIABLES
    var currentPage=currentPage;
    var nextPage=$(this).attr("data-page");
    var currentCategory="1";

    //SHOW DYNAMIC LOADING WINDOW
    $(".layerarticles").before("<div class='layerloading'><img src='images/ajax-loader.gif'></div>");
    $(".layerloading").height($(".layerarticles").height());
    $(".layerloading img").css("margin-top",$(".layerarticles").height()/2-16);
    $(".layerloading").fadeIn("slow",function(){

    //DATA LOAD
    $.post("ajax/articles-request.php", { currentPage: currentPage, nextPage: nextPage },function(data){ //AJAX CALL

            //WE KICK ALL OLD ARTICLES
            $(".layerarticles").animate({"margin-left":"-=630px"},'slow','easeOutQuint',function(){
                $(".layerarticles").css("height",$(".layerarticles").height());
                $(".layerarticles article").remove();
                $(".layerarticles").css("margin-left","0px");

                //WE MAKE APPEAR THE OTHERS IN THE HTML
                $.each(data, function(){
                    $(".layerarticles").append("<article style='display:none' id='"+this.iden+"'><header><h2>"+this.title+"</h2></header><img src='images/"+this.image+"'><p>"+this.chapeau+"</p></article>");
                });

                //WE SHOW THEM WITH FADE EFFECT
                $('.layerarticles article:hidden:first').delay(100).fadeIn("fast",function(){
                     if($('.layerarticles article:hidden').length!=0){
                        $(this).next().delay(100).fadeIn("fast",arguments.callee);
                     }else{

                        //ONCE ALL ARTICLES APPEARED WE REMOVE THE LOADING
                        $(".layerloading").fadeOut("slow",function(){
                            $(".layerloading").remove();    
                        });

                     }
                });

            });
        }, "json");

    });
});
\$\endgroup\$
1
  • 1
    \$\begingroup\$ I think you could improve by perhaps utilising chaining, and also wrapping it up as your own plugin so you can just go $('div').myplugin() but if it works and looks good then that's always a positive start, happy coding. \$\endgroup\$ – Tom Ingram Jan 6 '12 at 2:43
2
\$\begingroup\$

One big step to opimize your code is to cache your jQuery objects.

var $layer = $('#layer');

You can also chain your method calls

$layer.css({ height: '30px' }).fadeIn(300).append('<span>hi</span>');

Also, make use of context

var $layerLoading = $('.layerLoading');
$layerLoading.fadeOut("slow",function(){
  //$(".layerloading").remove();
    // instead do - this refers to the context which is '.layerloading' in this case
  $(this).remove();   
});
\$\endgroup\$
1
  • \$\begingroup\$ Is using $(this) in this case a performance improvement over reusing the cached $layerLoading or is it more of a style preference? I tend to always repeat the cached jQuery element but shouldn't if it's a performance hit. \$\endgroup\$ – DA. Jan 6 '12 at 2:58

Your Answer

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