Menu script efficiency

Question

I've got a jQuery/JavaScript script for a menu that uses icons as well as label text. When the icon or text is clicked, it turns blue, until such time as another icon is clicked. Also, the page scrolls down to the relevant div on the page, but the menu bar still stays fixed along the top of the screen.

I've created several versions of this script and this is the most efficient (least code) I have managed so far.

Can anyone tell me any methods to use instead, or any other way I could make it more efficient?

window.onload = function() {

        [].forEach.call(document.querySelectorAll('.navico'), function(el) { 
        el.addEventListener('click', imageButtonClickHandler); 
    });

        function imageButtonClickHandler() 
        {  
                switch(this.id) 
                {
                        case "aboutnav":
                                grey();
                                $('#abouttitle').css('color', 'blue');
                                $('#a').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#about").offset().top - 90 }, 2000); 
                        break;
                        case "routenav":
                                grey();
                                $('#routetitle').css('color', 'blue');
                                $('#b').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#route").offset().top - 90 }, 2000); 
                        break;
                        case "enternav":
                                grey();
                                $('#entertitle').css('color', 'blue');
                                $('#c').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#enter").offset().top - 90 }, 2000); 
                        break;
                        case "racedaynav":
                                grey();
                                $('#racedaytitle').css('color', 'blue');
                                $('#d').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#raceday").offset().top - 90 }, 2000); 
                        break;
                        case "gallerynav":
                                grey();
                                $('#gallerytitle').css('color', 'blue');
                                $('#e').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#gallery").offset().top - 90 }, 2000); 
                        break;
                        case "newsnav":
                                grey();
                                $('#newstitle').css('color', 'blue');
                                $('#f').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#news").offset().top - 90 }, 2000); 
                        break;
                        case "resultsnav":
                                grey();
                                $('#resultstitle').css('color', 'blue');
                                $('#g').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#resu").offset().top - 90 }, 2000); 
                        break;
                        case "contactnav":
                                grey();
                                $('#contacttitle').css('color', 'blue');
                                $('#h').css('color', 'blue');
                                $('html, body').animate( { scrollTop: $("#contact").offset().top - 90 }, 2000); 
                        break;

                }
        }
};


function grey()
        {
                $('.sectitle').css('color', 'grey');
                $('.roger').css('color', 'grey');
                return;
}

Answer 1

As mentioned in konijn's answer the simplest and cleanest way would be to change '#resu' to '#result' and then use this.id to extrapolate the other two id's.

However without modifing your html I condensed most of you duplicate code into a function which take parameters for the things that change.

window.onload = function() {
     [].forEach.call(document.querySelectorAll('.navico'), function(el) {
         el.addEventListener('click', imageButtonClickHandler);
     });

     function imageButtonClickHandler() {
         if(this.id == "aboutnav") handleIDAction('abouttitle', 'a', 'about');
         elseif(this.id == "routenav") handleIDAction('routetitle', 'b', 'route');
         elseif(this.id == "enternav") handleIDAction('entertitle', 'c', 'enter');
         elseif(this.id == "racedaynav") handleIDAction('racedaytitle', 'd', 'raceday');
         elseif(this.id == "gallerynav") handleIDAction('gallerytitle', 'e', 'gallery');
         elseif(this.id == "newsnav") handleIDAction('newstitle', 'f', 'news');
         elseif(this.id == "resultsnav") handleIDAction('resultstitle', 'g', 'resu');
         elseif(this.id == "contactnav") handleIDAction('contacttitle', 'h', 'contact');
     }
};

function handleIDAction(titleId, letterId, scrollId){
     $('.sectitle, .roger').css('color', 'grey');
     $( '#'+titleId+', #'+letterId ).css('color', 'blue');
     $('html, body').animate( { scrollTop: $('#'+scrollId).offset().top - 90 }, 2000);
}

Answer 2

I am happy you posted your best script, you can still see a ton of repetition here.. If you were to always name the navbutton after the scroll target and title ( "aboutnav" -> about & abouttitle ) then this can be much shorter. For now I can only see resultsnav break the mold.

I would suggest something like this :

window.onload = function () {

    [].forEach.call(document.querySelectorAll('.navico'), function (el) {
        el.addEventListener('click', imageButtonClickHandler);
    });

    function imageButtonClickHandler() {
      var buttonId = this.id,
          target   = buttonId.substring( 0 , buttonId.length - 3 ), 
          titleId = '#' + target + 'title',
          scrollTarget = '#' + target;

      var navMap = {
          'aboutnav'   : '#a',
          'routenav'   : '#b',
          'enternav'   : '#c',
          'racedaynav' : '#d'
          //etc. etc.
      }    

      grey();
      $( titleId ).css('color', 'blue');
      $( navMap[ target ] ).css('color', 'blue');
      $('html, body').animate({
        scrollTop: $(scrollTarget).offset().top - 90
      }, 2000);      
   }
};


function grey() {
    $('.sectitle').css('color', 'grey');
    $('.roger').css('color', 'grey');
    return;
}

Basically, I derive the link id, the scroll target id from the name of the nav link. Then, I also mapped the nav names to 'a','b','c' etc. From there this becomes a really short piece of code. I also changed the tabs to 4 spaces, if you use more than 4 spaces, then it becomes harder to grok the code and realy understand what you are doing. I also opted for single quote strings, easier to read and less carpal tunnel by hitting shift all the time ;)

Note that you could write grey like this:

function grey() {
    $('.sectitle, .roger').css('color', 'grey');
}

You can comma chain selectors, also the function will return regardless at the end, you dont need to put return.