current community

your communities

more stack exchange communities

company blog
Stack Exchange Inbox Reputation and Badges
tour help
Sign up ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
up vote 2 down vote favorite

I currently have a JavaScript file that I'm using to fix the navigation on a website I've built. The code is by no means 'DRY', though, and I'd like to fix that issue.

//fixing the nav dropdowns for mobile


//concrete and joint repair dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggle").click(function(){
            var $target = $('.target'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggle").hover(function(){
            var $target = $('.target'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".target").hover(function(){
            var $target = $('.target'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end concrete and joint repair dropdown section





//primers and sealers dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleTwo").click(function(){
            var $target = $('.targetTwo'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleTwo").hover(function(){
            var $target = $('.targetTwo'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetTwo").hover(function(){
            var $target = $('.targetTwo'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end primers and sealers dropdown section


//high build coatings dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleThree").click(function(){
            var $target = $('.targetThree'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleThree").hover(function(){
            var $target = $('.targetThree'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetThree").hover(function(){
            var $target = $('.targetThree'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end high build coatings dropdown section




//self levelers and overlays dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleFour").click(function(){
            var $target = $('.targetFour'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleFour").hover(function(){
            var $target = $('.targetFour'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetFour").hover(function(){
            var $target = $('.targetFour'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end self levelers and overlays dropdown section




//vertical coatings dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleFive").click(function(){
            var $target = $('.targetFive'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleFive").hover(function(){
            var $target = $('.targetFive'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetFive").hover(function(){
            var $target = $('.targetFive'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end vertical coatings dropdown section



//performance topcoats dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleSix").click(function(){
            var $target = $('.targetSix'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleSix").hover(function(){
            var $target = $('.targetSix'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetSix").hover(function(){
            var $target = $('.targetSix'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end performance topcoats dropdown section


//elastomerics dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleSeven").click(function(){
            var $target = $('.targetSeven'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleSeven").hover(function(){
            var $target = $('.targetSeven'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetSeven").hover(function(){
            var $target = $('.targetSeven'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end elastomerics dropdown section


//cleaning products dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleEight").click(function(){
            var $target = $('.targetEight'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleEight").hover(function(){
            var $target = $('.targetEight'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetEight").hover(function(){
            var $target = $('.targetEight'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end cleaning products dropdown section



//installations dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleNine").click(function(){
            var $target = $('.targetNine'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleNine").click.hover(function(){
            var $target = $('.targetNine'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetNine").click.hover(function(){
            var $target = $('.targetNine'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end installations dropdown section


//additional products dropdown section

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);


    function checkWidth() {
        var windowsize = $window.width();

        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggleTen").click(function(){
            var $target = $('.targetTen'),
                $toggle = $(this);

            $target.toggle();
            });
        } 


        else if (windowsize > 990) {

            //if the window is less than 990px wide then flyout on hover..

            $("#toggleTen").hover(function(){
            var $target = $('.targetTen'),
                $toggle = $(this);

                $target.toggle();
            }); 

            $(".targetTen").hover(function(){
            var $target = $('.targetTen'),
                $toggle = $(this);

                $target.toggle();
            }); 

        }
    }
    // Execute on load
    checkWidth();
    // Bind event listener
    $(window).resize(checkWidth);
});

//end additional products dropdown section


//end fixing the nav dropdowns for mobile
share|improve this question

migrated from stackoverflow.com Sep 3 at 20:41

This question came from our site for professional and enthusiast programmers.
1  
What if window size is equal to 990?! :-) – holroy Sep 3 at 22:11
    
I've currently changed it to be if (windowsize <= 990) & else if (windowsize >= 991). I believe this solves it. – Colin B Sep 4 at 15:49
    
Is your checkWidth function also responsible for styling things based on the width or just checking the width? – Sean Parsons Sep 4 at 16:20
    
It is only used for checking the width. Depending on what your referring to by 'style'. When my checkWidth runs it looks to see what the window width is. If it is smaller then the nav opens on click. If the window width is larger then nav items open on hover. – Colin B Sep 4 at 16:25
    
Are you sure that you cannot achieve this with only CSS and mediaqueries? – romuleald Sep 7 at 16:30

3 Answers 3

up vote 1 down vote

Rather than just fix it, the lesson to learn here is about parametrizing functions.

When you find yourself with lots of repeated code where only a small number of values change, you should wrap it up as a function and instead supplying the changing values as arguments.

For example, a large part of the code above is the duplication of this kind of code.

$('#toggleOne').hover(function() {
  var $target = $('.targetOne');
  $target.toggle();
});

Instead, you can write

function toggle(hoverElement, toggleElement) {
  $(hoverElement).hover(function() {
    var $target = $(toggleElement);
    $target.toggle();
  });
}

Then rather than writing it all out again, simply call the function and adjust those values.

toggle('#toggleOne', '.targetOne');
// ...
toggle('#toggleTwo', '.targetTwo');
share|improve this answer
    
Dan, I utilized your method and everything seems to work great. However, I ran the function within the 'if' statement and then within the 'else if' statement separately for each different window size and the 'else if' is now over riding the 'if' statement. Please see the JsFiddle of the code here: jsfiddle.net/Cboardway/h0eb8aLq any thoughts? – Colin B Sep 4 at 15:44
up vote 0 down vote

You would have to change your ids and classes to use numerica values such as .target-1 for instance... then just create a for loop to iterate with it. Note: You only need to call on document ready once:

$(document).ready(function() {
    // Optimalisation: Store the references outside the event handler:
    var $window = $(window);
    var limit = 10;
    function checkWidth() {
        var windowsize = $window.width();
        if (windowsize < 990) {
            //if the window is less than 990px wide then turn on flyout on click..
            $("#toggle-"+i).click(function(){
            var $target = $('.target-'+i),
                $toggle = $(this);
                $target.toggle();
            });
        } 


        else if (windowsize > 990) {
            $("#toggleSeven").hover(function(){
            var $target = $('.target-'+i),
                $toggle = $(this);
                $target.toggle();
            }); 

            $(".targetSeven").hover(function(){
            var $target = $('.target-'+i),
                $toggle = $(this);
                $target.toggle();
            }); 

        }
    }
    for(var i =0; i <= limit; i++) {
        checkWidth(i);
    }
});
share|improve this answer
up vote 0 down vote

If you call another function with the value of the width resolved by the checkWidth function then you will be able to style it based on the width. So something like this.

$(window).resize(checkWidth);

Function checkWidth () {

var width = $(window).width();

//example function call

stylePage(width);

}

function stylePage (width) {

//page styling code goes here

If(width < X) {

//style for smaller devices or whatever
}
share|improve this answer
    
Aaaah I see where you going with this. I took this into account and realized that I needed to call my $(window).resize(checkWidth); outside of my $document.ready(); function and it seems to have fixed the issue. See the jsfiddle here: jsfiddle.net/Cboardway/h0eb8aLq/1 the jsfiddle has my new code that I revised after looking at Dan Prince's repsonse above. Though with this code I am still having the issue of the 'if else' statement over riding the 'if' statement. Which makes it fly out on hover no matter what the screen size. – Colin B Sep 4 at 17:12

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.