caramba
caramba

Reputation: 22480

Use function everywhere without writing it more than once

Trying to get my code cleaner I don't want to write the same code many times but don't know how to accomplish it. You can see in the code below is the if/else written twice but doing the same. How can I return data and call that function again with writing it only once?

Help and advices are much appreciated.

Code is like this:

(function($){
    var scroll = $(this).scrollTop();
    var header = $('header');
    var headerSmall = header.children('.small');
    var headerBigWrap = header.children('.big');
    var headerBigWrapHeight = headerBigWrap.height();

    var showNav = function() {
        header.css('position','fixed');
        headerBigWrap.css('display','none');
        headerSmall.css('display','block');
    }

    var hideNav = function() {
        header.css('position','static');
        headerSmall.css('display','none');
        headerBigWrap.css('display','block');
    }

    // don't want to write this more than once 
    // but need to call here on document ready
    if( scroll >= headerBigWrapHeight ) {
        showNav();
    } else {
        hideNav();
    }


    $(window).scroll(function(event){

        scroll = $(this).scrollTop();

        // here I need it again
        // scroll is changing on scroll
        // but "function" does exactly the same like above

        if( scroll >= headerBigWrapHeight ) {
            showNav();
        } else {
            hideNav();
        }
    });

})(jQuery);

if you need html and css working example: fiddle http://jsfiddle.net/cejs3jhs/ and rest of code:

<header>
    <div class="big">
        <h1>Page Title</h1>
    </div>
    <div class="small">
        <span class="icon nav-icon">Nav Icon</span>
    </div>
</header>
<div class="page"></div>

<style type="text/css">
header {
    width:100%;
    background-color:red;
    opacity:0.5;
}
header.toggle-nav {
    height:100%;
}
header.toggle-nav ul {
    display:block;
}
header > div {
    position:relative;
    width:976px;
    margin:0 auto;
}
header > div.big {
    padding:30px 0;
}
.page {
    height:5000px;
    background-color:orange;
    opacity:0.5;
}
</style>

Upvotes: 0

Views: 113

Answers (5)

twhb
twhb

Reputation: 4594

The answer to your question is simply what Justinas said, make another function. But you can improve a bit with some more aggressive editing, which is what I've done.

(function ($) {
    var header = $('header');
    var headerSmall = header.children('.small');
    var headerBigWrap = header.children('.big');
    var headerBigWrapHeight = headerBigWrap.height();

    function setNavStyle() {
        var scroll = $(this).scrollTop();

        if( scroll >= headerBigWrapHeight ) {
            header.css('position', 'fixed');
            headerSmall.css('display', 'block');
            headerBigWrap.css('display', 'none');
        } else {
            header.css('position', 'static');
            headerSmall.css('display', 'none');
            headerBigWrap.css('display', 'block');
        }
    }
    $(window).scroll(setNavStyle);
    setNavStyle();
})(jQuery);

That's one way, but it would be better (more maintainable, simpler, faster) to do the bulk of the work with CSS, like so:

CSS:

header {
    position: static;
}
header>.small {
    display: none;
}
header>.big {
    display: block;
}

.stickyNav header {
    position: fixed;
}
.stickyNav header>.small {
    display: block;
}
.stickyNav header>.big {
    display: none;
}

JavaScript:

(function ($) {
    var bigNavHeight = $('header>.big').height();
    function setNavStyle() {
        var bigIsOffscreen = $(this).scrollTop() >= bigNavHeight;

        $(document.body).toggleClass('stickyNav', bigIsOffscreen);
    }

    $(window).scroll(setNavStyle);
    setNavStyle();
})(jQuery);

Upvotes: 1

Hazem Taha
Hazem Taha

Reputation: 1184

You can define a new function just as you did with the showNav & hideNav functions

(function($){
    var scroll = $(this).scrollTop();
    var header = $('header');
    var headerSmall = header.children('.small');
    var headerBigWrap = header.children('.big');
    var headerBigWrapHeight = headerBigWrap.height();

    var showNav = function() {
        header.css('position','fixed');
        headerBigWrap.css('display','none');
        headerSmall.css('display','block');
    }

    var hideNav = function() {
        header.css('position','static');
        headerSmall.css('display','none');
        headerBigWrap.css('display','block');
    }

     var checkScroll = function (scroll,headerBigWrapHeight){
    if( scroll >= headerBigWrapHeight ) {
        showNav();
    } else {
        hideNav();
    }

    checkScroll(scroll,headerBigWrapHeight);


    $(window).scroll(function(event){
        scroll = $(this).scrollTop();
         checkScroll(scroll,headerBigWrapHeight);
    });



})(jQuery);

Upvotes: 1

Justinas
Justinas

Reputation: 43441

So just make separate function for that:

function showHideNav(scroll, headerBigWrapHeight, element) {
    var scroll = element.scrollTop(),
    header = $('header'),
    headerSmall = header.children('.small'),
    headerBigWrap = header.children('.big')
    headerBigWrapHeight = headerBigWrap.height();

    if( scroll >= headerBigWrapHeight ) {
        header.css('position','fixed');
    } else {
        header.css('position','static');
    }

    headerBigWrap.toggle();
    headerSmall.toggle();
}

(function($){
    showHideNav(scroll);

    $(window).scroll(function(event){
        scroll = $(this).scrollTop();
        showHideNav(scroll);
    });

})(jQuery);

Upvotes: 2

Martin Ernst
Martin Ernst

Reputation: 3269

Do it exactly like you have done with functions showNav and hideNav: define a function with if/else inside somewhere and you can call it by name wherever needed. There are also some variables which can be omitted then, so I show all code here:

(function($){
    // var scroll is moved into function scrollFunc
    var header = $('header');
    var headerSmall = header.children('.small');
    var headerBigWrap = header.children('.big');
    // var headerBigWrap is moved into function scrollFunc

    var showNav = function() {
        /* your code */
    }

    var hideNav = function() {
        /* your code */
    }

    var scrollFunc = function () {
        if($(this).scroll() >= headerBigWrap.height() ) {
            showNav();
        } else {
            hideNav();
        }
    }

    $(window).scroll(function(event){
        scrollFunc();
    });

    scrollFunc(); // call it first time to initialize

});

Upvotes: 0

user4094161
user4094161

Reputation:

Please find below updated JS code:

(function($){
    var scroll = $(this).scrollTop();
    var header = $('header');
    var headerSmall = header.children('.small');
    var headerBigWrap = header.children('.big');
    var headerBigWrapHeight = headerBigWrap.height();

    var showNav = function() {
        header.css('position','fixed');
        headerBigWrap.css('display','none');
        headerSmall.css('display','block');
    }

    var hideNav = function() {
        header.css('position','static');
        headerSmall.css('display','none');
        headerBigWrap.css('display','block');
    }

    var callFun = function(scroll) {
        if( scroll >= headerBigWrapHeight ) {
            showNav();
        } else {
            hideNav();
        }
    }


    callFun(scroll);


    $(window).scroll(function(event){
        scroll = $(this).scrollTop();

        callFun(scroll);
    });

})(jQuery);

Upvotes: 0

Related Questions