4
\$\begingroup\$

I would like feedback on things I can improve. If you want to see the full example in action, here is the link.

$(document).ready(function() {

var all = $("#slider-view");

var winWidth            = window.innerWidth;
var winHeight           = window.innerHeight;   

$(all).css('height', window.innerHeight);
$(all).css('width', window.innerWidth); 

// Assigns the container that has all the sections that will be scrolled horizontally   
var sliderH             = $('.nav-h'); 
var sliderVMiddle       = $('.nav-v-middle');
var sliderVLast         = $('.nav-v-last');

// Gets the number of slides of the horizontal slider
var sliderCountH        = $('.nav-h').children().size(); 
var sliderCountVMiddle  = $('.nav-v-middle').children().size(); 
var sliderCountVLast    = $('.nav-v-last').children().size(); 


// Sets the properties of the arrows that have the events
setCssRect(sliderH, 'w', winWidth);
setCssRect(sliderH, 'h', winHeight);
setCssRect(sliderVMiddle, 'w', winWidth);
setCssRect(sliderVMiddle, 'h', winHeight);
setCssRect(sliderVLast, 'w', winWidth);
setCssRect(sliderVLast, 'h', winHeight);

// sets css size properties on the jQuery element.
function setCssRect(elt, property, value) {
    if(property=='w') {
        $('> div',elt).css('width', value);
    }
    else {$('> div',elt).css('height', value);} 
}       

// Gets the number of Horizontal or Vertical slide
var viewWidth           = sliderCountH * winWidth; 
var heightVMiddle       = sliderCountVMiddle * winHeight;
var heightVLast         = sliderCountVLast * winHeight;

// Sets the width and height for the Horizontal and Vertical slides    
$('.nav-h').css('width', viewWidth); 
$('.nav-v-middle').css('height', heightVMiddle);
$('.nav-v-last').css('height', heightVLast);


var isMobile = {
    Android: function() { return navigator.userAgent.match(/Android/i); },
    BlackBerry: function() { return navigator.userAgent.match(/BlackBerry/i); },
    iOS: function() { return navigator.userAgent.match(/iPhone|iPad|iPod/i); },
    Opera: function() { return navigator.userAgent.match(/Opera Mini/i); },
    Windows: function() { return navigator.userAgent.match(/IEMobile/i); },
    any: function() { return (isMobile.Android() || isMobile.BlackBerry() || isMobile.iOS() || isMobile.Opera() || isMobile.Windows()); }
};                                  

if ( isMobile.any() ) {

    $('a#prevh').toggle();
    $('a#nexth').toggle();  
    $('a#prevv').toggle();
    $('a#nextv').toggle();  

    var horizontalIndex = 0;
    var verticalIndexMiddle = 0;
    var verticalIndexLast = 0;

    // actions for the swiperight
    $(sliderH).hammer({prevent_default:true}).on('swiperight', function() {
        actionHorizontalAnimation(0, '+', '+');
    });    


    // Actions for the swipeLeft
    $(sliderH).hammer({prevent_default:true}).on('swipeleft', function() {  
        actionHorizontalAnimation((sliderCountH-1), '-', '-'); 
    }); 


    // Actions for the swipeUp
   $(sliderH).hammer({prevent_default:true}).on('swipedown', function() {   
        actionVerticalAnimation(0, sliderCountVLast, '+');
   });

    // Actions for the swipeDown
    $(sliderH).hammer({prevent_default:true}).on('swipeup', function() {    
        actionVerticalAnimation((sliderCountVMiddle-1), (sliderCountVLast-1), '-'); 
   });     


}

else { 

    var horizontalIndex = 0;
    var verticalIndexMiddle = 0;
    var verticalIndexLast = 0;

    findSlidePosition(horizontalIndex, verticalIndexMiddle, verticalIndexLast);

    $('a#prevh').on('click', function(event) {
        actionHorizontalAnimation(0, '+', '+');         
    });

    $('a#nexth').on('click', function(event) { 
        actionHorizontalAnimation((sliderCountH-1), '-', '-');  
    }); 

    $('a#prevv').on('click', function(event) {
        actionVerticalAnimation(0, sliderCountVLast, '+');      
    });

    $('a#nextv').on('click', function(event) {  
        actionVerticalAnimation((sliderCountVMiddle-1), (sliderCountVLast-1), '-'); 
    });                          
}

// Onclick horizontal movement
function actionHorizontalAnimation(sliderPosition, orientation, shift) {
    if(verticalIndexMiddle!==0) {
        verticalAnimation(sliderVMiddle, '+', (winHeight*verticalIndexMiddle))
        verticalIndexMiddle = 0;                    
    } 
    if(verticalIndexLast!==0) {
        verticalAnimation(sliderVLast, '+', (winHeight*verticalIndexLast))
        verticalIndexLast = 0;                      
    }                
    if(horizontalIndex!==sliderPosition) {      
        horizontalAnimation(sliderH, shift, winWidth);
        if(orientation==='-'){horizontalIndex+=1;}else{horizontalIndex-=1;}
    }
    findSlidePosition(horizontalIndex, verticalIndexMiddle, verticalIndexLast); 
} 

// OnClick vertical movement 
function actionVerticalAnimation(middleSlidePosition, lastSlidePosition, orientation) {
    if(horizontalIndex==1) { 
        if(verticalIndexMiddle!==middleSlidePosition) {     
            verticalAnimation(sliderVMiddle, orientation, winHeight);
            if(orientation==='-'){verticalIndexMiddle+=1;} else {verticalIndexMiddle-=1;}                               
        }
    } else if (horizontalIndex==2) {
        if(verticalIndexLast!==lastSlidePosition) {     
            verticalAnimation(sliderVLast, orientation, winHeight)
        if(orientation==='-'){verticalIndexLast+=1;} else {verticalIndexLast-=1;}               
        }               
    }
    findSlidePosition(horizontalIndex, verticalIndexMiddle, verticalIndexLast);     
}   

// animation for vertical slide
function verticalAnimation(slider, action, size) {      
    $('> div', slider).animate({
        top: action+'=' + size
    }, 600);    
} 

// animation for horizontal slide
function horizontalAnimation(slider, action, size) {        
    $('> div', slider).animate({
        left: action+'=' + size
    }, 600);    
}          

// Set hides or shows the arrows for the navigation
function findSlidePosition(h1, v1, v2) { 
    $('.nav-btns div').css('border-color', '#706E6D');
    resetObjectStyle();     
    if(h1==0 && v1==0 && v2==0) {
        $('.nav-home').css('border-color','#FFF');
        activateControlFlow(['#nexth']);    
    }
    else if(h1==1 && v1==0 && v2==0) {
        $('.nav-middle-slide1').css('border-color','#FFF'); 
            activateControlFlow(['#nexth', '#prevh', '#nextv']); 
    }
    else if(h1==1 && v1>=1 && v1<=8 && v2==0) {
        var test = '.nav-middle-slide'+ (v1+1);
        $(test).css('border-color','#FFF'); 
        activateControlFlow(['#nexth', '#prevh', '#nextv', '#prevv']);      
    }       
    else if(h1==1 && v1==9 && v2==0) {
        $('.nav-middle-slide10').css('border-color','#FFF');        
        activateControlFlow(['#nexth', '#prevh', '#prevv']);    
    }       
    else if(h1==2 && v1==0 && v2==0) {
        $('.nav-last-slide1').css('border-color','#FFF');       
        activateControlFlow(['#prevh', '#nextv']); 
    }       
    else if(h1==2 && v1==0 && (v2==1 || v2==2)) {
        $('.nav-last-slide2').css('border-color','#FFF');       
        activateControlFlow(['#prevh', '#nextv', '#prevv']);    
    }       
    else if(h1==2 && v1==0 && v2==3) {
        $('.nav-last-slide4').css('border-color','#FFF'); 
        activateControlFlow(['#prevh', '#prevv']);      
    }                           
}

// Hides all the arrows
function resetObjectStyle() {       
    $('a.control-flow').css({color:'transparent', cursor: 'auto'});         
}

// Shows specific arrows fiven as parameters in an array   
function activateControlFlow(arrayControl) {
    $.each(arrayControl, function(key, selector){
        $(selector).css({color:'#FFF', cursor: 'pointer'});  
    }); 
}
var scene4 = document.getElementById('scene4');
var parallax = new Parallax(scene4);    
var scene5 = document.getElementById('scene5');
var parallax = new Parallax(scene5);    
var scene6 = document.getElementById('scene6');
var parallax = new Parallax(scene6);    
var scene8 = document.getElementById('scene8');
var parallax = new Parallax(scene8);  
var scene9 = document.getElementById('scene9');
var parallax = new Parallax(scene9);  
var scene10 = document.getElementById('scene10');
var parallax = new Parallax(scene10);                      
});
\$\endgroup\$
6
  • \$\begingroup\$ Continuously numbered variables (css classes, IDs, etc) are always a code smell. \$\endgroup\$
    – Tomalak
    Nov 22, 2013 at 16:16
  • \$\begingroup\$ is this a bad practice? I was having a hard time giving names \$\endgroup\$
    – Monica
    Nov 22, 2013 at 16:17
  • \$\begingroup\$ If you have multiple instances of the same thing, use arrays. \$\endgroup\$
    – Tomalak
    Nov 22, 2013 at 16:18
  • \$\begingroup\$ You also have a lot of other duplication in your code. I'm trying to make something more compact over on jsFiddle. Might take a moment. \$\endgroup\$
    – Tomalak
    Nov 22, 2013 at 16:36
  • 1
    \$\begingroup\$ let us continue this discussion in chat \$\endgroup\$
    – Tomalak
    Nov 22, 2013 at 17:26

1 Answer 1

4
\$\begingroup\$

I took a look at your chat with Tomalak and it seems you got some good advice, so big tip-o'-the-hat to Tomalak.

These are just some thoughts on the overall approach to some of this stuff. I'm not so much looking at your current code, but the things I touch on should be applicable regardless.

As Tomalak mentioned, it's better to avoid sequential numbering. But you've also got the middle and last classes that also implies a sort of numbering system. A numbering system that can't go higher than 3 items in fact, so it's pretty limiting. Remember that the structure of the HTML itself can provide a lot of this stuff; ordering is maintained when you use jQuery to select elements.

To keep the code abstract, I'd probably use group as the name (and class) for the "columns" of slides. While I just used "column" in the description - and you could call them that instead - it locks everything into a concrete idea of how it looks. And that isn't always necessary from a coding perspective.

So, you've got 3 groups, and each has x number of slides. Within each group, you've got a first and a last slide, as determined by their order in the HTML.

Now, if you've got a slide that's marked as, say, "current", navigating to another slide is fairly easy. And you don't need to keep track of the current slide, as you can always find it again by its class name.

Within the group (column), the next and previous slides can be found from the current one using jQuery's aptly named .next() and .prev(). So to go to the next slide (i.e. "down" on your site), you could do something like:

var currentSlide = $(".slide.current"), 
    targetSlide  = currentSlide.next(".slide");
if(targetSlide.length === 0) {
  // no more slides in this group!
} else {
  currentSlide.removeClass("current");
  targetSlide.addClass("current");
}

For moving from group to group, it's a little different. However, provided you want the current functionality of always moving to the first slide in a group, you can do something like this:

var currentSlide = $(".slide.current"), 
    targetSlide  = currentSlide.closest(".group").next(".group").find(".slide:first");
if(targetSlide.length === 0) {
  // no slides to move to!
} else {
  currentSlide.removeClass("current");
  targetSlide.addClass("current");
}

(With your structure you can simply use .parent() instead of .closest(".group"), and .next() without a selector, but I'm being explicit/verbose to make things clearer. Included the pointless if-block for the same reason; you can obviously just do a !== comparison instead)

You'll note that only one line differs between the two, so it's a prime candidate for being factored into a function - or functions, plural.

To keep things fairly abstract, I'd refer to the slide changes as simply "next", "previous", "next group" and "previous group". Only when actually adding the event handlers would I link that to an up/down/left/right direction. Again, it's better, I find, to not tie yourself to a notion of how it looks; the groups might be stacked vertically, with the slides running horizontally, but it wouldn't matter to the code above, since it's more abstract.

In your case, the visual layout does matter in terms of animation (up/down versus left/right), but like with the event handlers, try to make such distinctions "late" in the code if you can.

As for finding a slide's position, you could do something like

var currentSlide = $(".current.slide"),
    slideIndex   = currentSlide.prevAll(".slide").length,
    groupIndex   = currentSlide.closest(".group").prevAll(".group").length;

Anyway, these are just some ideas based on your chat with Tomalak and the site's HTML. There are obviously things that can be optimized (e.g. caching slide/group indices using .data()). Just consider it food for thought.

Here's a simple jsfiddle just to demonstrate the DOM traversal stuff

\$\endgroup\$
2
  • \$\begingroup\$ Thank you, I just read this, I will make the proper changes to make the code better. Wow! the jsfiddle is awesome!!! I really want to change the code to this. I will try and post the code again. Million thanks!!! \$\endgroup\$
    – Monica
    Nov 28, 2013 at 18:15
  • \$\begingroup\$ @Monica No problem, glad you found it useful. Don't forget to checkmark the answer (if everything works out, of course) \$\endgroup\$
    – Flambino
    Nov 28, 2013 at 23:16

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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