Hello and welcome to our community! Is this your first visit?
Register
Enjoy an ad free experience by logging in. Not a member yet? Register.
Results 1 to 7 of 7
  1. #1
    New Coder
    Join Date
    Nov 2011
    Posts
    24
    Thanks
    0
    Thanked 0 Times in 0 Posts

    How can I shorten this code?

    Hi guys,

    I'm currently listening for multiple click events for a navigation, that do the same action but have different hooks.

    How can I go about shortening this code? I thought I could store the values into an array and access it from a loop or something but no such luck.
    Code:
    var nav = new Array(); 
    nav[0] = $('.js-nav-home');  
    nav[1] = $('.js-nav-about');
    nav[2] = $('.js-nav-work');
    nav[3] = $('.js-nav-contact');
    
    var position = new Array(); 
    position[0] = $('.js-home').offset().top;     
    position[1] = $('.js-about').offset().top;
    position[2] = $('.js-work').offset().top;
    position[3] = $('.js-contact').offset().top;
    
    nav[0].click(function(){
      $('html, body').animate({scrollTop:position[0]}, 'slow');
      return false;
    });
    
    nav[1].click(function(){
      $('html, body').animate({scrollTop:position[1]}, 'slow');
      return false;
    });
    
    nav[2].click(function(){
      $('html, body').animate({scrollTop:position[2]}, 'slow');
      return false;
    });
    
    nav[3].click(function(){
      $('html, body').animate({scrollTop:position[3]}, 'slow');
      return false;
    });
    this was the attempted loop.

    Code:
    for (i=0; i < nav.length; i++) {
      nav[i].click(function(){
        $('html, body').animate({scrollTop:position[i]}, 'slow');
        return false;
      });
    }
    Many thanks

  • #2
    Senior Coder rnd me's Avatar
    Join Date
    Jun 2007
    Location
    Urbana
    Posts
    4,334
    Thanks
    11
    Thanked 587 Times in 568 Posts
    Code:
    for (i=0; i < nav.length; i++) {(function(i){
      nav[i].click(function(){
        $('html, body').animate({scrollTop:position[i]}, 'slow');
        return false;
      });
    }(i)}
    my site (updated 13/9/26)
    BROWSER STATS [% share] (2014/5/28) IE7:0.1, IE8:5.3, IE11:8.4, IE9:3.2, IE10:3.2, FF:18.2, CH:46, SF:7.9, NON-MOUSE:32%

  • #3
    New Coder
    Join Date
    Nov 2011
    Posts
    24
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Hi there,

    Thanks for the solution, this really helped!

    Why do I have to have an anonymous function inside? Is the loop / array the right approach then ?

    Thanks.

  • #4
    Senior Coder rnd me's Avatar
    Join Date
    Jun 2007
    Location
    Urbana
    Posts
    4,334
    Thanks
    11
    Thanked 587 Times in 568 Posts
    Quote Originally Posted by obenns View Post
    Hi there,

    Thanks for the solution, this really helped!

    Why do I have to have an anonymous function inside? Is the loop / array the right approach then ?

    Thanks.
    it's because the variable "i" is recycled in the loop, so by the time your functions ran, "i" was at it's last point. Inside a function however, "i" is private to the function, so the code doesn't see the outer "i" that's incremented, it still sees its own "i", the formal parameter name to the anon function, set to the outer "i" as a passed argument.
    my site (updated 13/9/26)
    BROWSER STATS [% share] (2014/5/28) IE7:0.1, IE8:5.3, IE11:8.4, IE9:3.2, IE10:3.2, FF:18.2, CH:46, SF:7.9, NON-MOUSE:32%

  • #5
    The fat guy next door VIPStephan's Avatar
    Join Date
    Jan 2006
    Location
    Halle (Saale), Germany
    Posts
    8,662
    Thanks
    6
    Thanked 1,006 Times in 979 Posts
    Quote Originally Posted by obenns View Post
    Is the loop / array the right approach then ?
    I would say no. First of all, new Array() is basically obsolete, it’s much shorter and easier to use the literal notation []. Then, I think for using jQuery you are overcomplicating things. There is the each() function with which you can iterate over a jQuery object and execute a function on each item. And while you do that you can also calculate the position and execute the animation. I don’t know what your nav markup looks like, I assume the following:

    Code:
    <ul class="nav">
      <li><a>…</a></li>
      <li><a>…</a></li>
      <li><a>…</a></li>
    </ul>
    So, effectively you would do this:
    PHP Code:
    $('.nav li').each(function() {
        var 
    el = $(this);
        var 
    pos el.offset().top;
        
    el.children('a').click(function(e) {
            $(
    'html, body').animate({scrollTop:pos}, 'slow');
            
    e.preventDefault();
        });
    }); 

  • #6
    New Coder
    Join Date
    Nov 2011
    Posts
    24
    Thanks
    0
    Thanked 0 Times in 0 Posts
    Thanks both of you

    Stephan that's a good approach and have used .each a couple times before. The reason i used separate hooks s that there is another instance of a link outside of the navigation. But I shall see what I can come up with using what you have mentioned.

    Thanks again guys.

  • #7
    The fat guy next door VIPStephan's Avatar
    Join Date
    Jan 2006
    Location
    Halle (Saale), Germany
    Posts
    8,662
    Thanks
    6
    Thanked 1,006 Times in 979 Posts
    Then show us your complete HTML code and we might be able to give you a recommendation.


  •  

    Posting Permissions

    • You may not post new threads
    • You may not post replies
    • You may not post attachments
    • You may not edit your posts
    •