Well, for one continuously querying for the same DOM objects is not only repetitive, but costly. You should always capture your query results with a variable. Doing this alone, will clean up your code:
var $tlb = jQuery('#top-link-block');
NOTE: I suggest prefixing your JQuery result variable with a $ to remind you that it is a JQuery wrapper object and not a traditional DOM object.
Now, you can just use this variable over and over:
if ($tlb.offset...)...
In addition, you are using this value:
jQuery('#footer-wrapper').offset().top
more than once, so I’d save that value in a variable as well:
var $footWrapOffsetTop = jQuery('#footer-wrapper').offset().top;
And then you can use that where needed without having to query the object and extract the offset.top value repeatedly.
Also, this:
jQuery(window)
is wasteful because since you are not capturing the result of the query and only using the query so that you can attach an event handler to window. Using JQuery to get a reference to the global window object does nothing more than perform a lookup on an object that is always available. Just use:
window.addEventListener("scroll", function(){...});
The only benefit for querying for window would be to take advantage of JQuery’s wrapped-set functions, but you aren’t doing that here.
Here’s the cleaned up version (using best-practices):
scrollToTop: function () {
var offset = 160;
var duration = 500;
// Result is JQuery wrapped-set object:
var $tlb = jQuery('#top-link-block');
// Result is top value:
var footWrapOffsetTop = $('#footer-wrapper').offset().top;
window.addEventListener('scroll', (function() {
// Don't take advantage of optional curly braces with blocks
// of only one statement. It can lead to bugs.
if (window.scrollTop() > offset){
$tlb.fadeIn(duration) : $tlb.fadeOut(duration);
}
if ($tlb.offset().top + $tlb.height() >= footWrapOffsetTop - 10) {
$tlb.css('position', 'absolute');
}
if ($(document).scrollTop() + window.innerHeight < footWrapOffsetTop){
$tlb.css('position', 'fixed');
}
if ($('#fixed-toolbar-menu')[0]){
$tlb.css({ bottom: 150 });
}
});
1
solved A cleaner/better way to write jQuery function [closed]