4
\$\begingroup\$

I've written a plugin that takes a generic approach to creating sticky headers and would like to have the code reviewed. Is this acceptable JavaScript?

Demo Page

It's my first plugin however and I'd like feedback about how to make it more user friendly and more readable before releasing it and putting it up on GitHub.

The header becomes stuck to the top of the page until it encounters the bottom of it's container, upon which it gets stuck to the bottom of the container and floats off the screen.

I had to create a placeholder to take the header's original place once it leaves the document flow and also apply fixed CSS settings to the header to keep it's appearance the same when it leaves the flow.

(function ($) {
$.fn.stickySectionHeaders = function (options) {

    var settings = $.extend({
        stickyClass: 'header',
        padding: 0
    }, options);

    return $(this).each(function () {

        var container = $(this);
        var header = $('.' + settings.stickyClass, container);

        var originalCss = {
            position: header.css('position'),
            top: header.css('top'),
            width: header.css('width')
        };

        var placeholder = undefined;
        var originalWidth = header.outerWidth();

        $(window).scroll(function () {
            var containerTop = container.offset().top;
            var headerOrigin = header.offset().top;
            var headerHeight = header.outerHeight();
            var containerHeight = container.outerHeight();
            var containerTop = container.offset().top;
            var containerSize = container.outerHeight();
            var pageOffset = $(window).scrollTop() + settings.padding;
            var containerBottom = containerHeight + containerTop;

            if (pageOffset < containerTop && placeholder != undefined) {
                if (placeholder != undefined) {
                    placeholder.remove();
                    placeholder = undefined;
                    header.css(originalCss);
                }
            }
            else if (pageOffset > containerTop && pageOffset < (containerBottom - headerHeight)) {
                if (placeholder == undefined) {
                    placeholder = $('<div/>')
                    .css('height', header.outerHeight() + 'px')
                    .css('width', header.width() + 'px');
                    header.before(placeholder);
                    header.css('position', 'fixed');
                    header.css('width', originalWidth + 'px');
                }
                header.css('top', settings.padding + 'px');
            }
            else if (pageOffset > (containerBottom - headerHeight)) {
                header.css('top', (containerBottom - headerHeight) - pageOffset + settings.padding + 'px');
            }
        });
    });
}
})(jQuery);
\$\endgroup\$
  • \$\begingroup\$ A working example would be great, because currently I can't imagine what "sticky headers" are supposed to be... \$\endgroup\$ – RoToRa Mar 22 '11 at 16:10
  • \$\begingroup\$ Added a link to the demo! \$\endgroup\$ – Robert Fall Mar 22 '11 at 18:57
2
\$\begingroup\$

At the risk of nitpicking there's quite a fiew small/micro-optimizations that could be done:

  • group multiple var declarations using commas

    var containerTop = container.offset().top, headerOrigin = header.offset().top;

  • chain method calls to the same object

    header.before(placeholder).css('...')

  • group multiple calls to css() passing an object as the parameter:

    header.css({ position: 'fixed', width: originalWidth + 'px'});

\$\endgroup\$
  • \$\begingroup\$ Thanks, this is exactly the sort of thing I was looking for! \$\endgroup\$ – Robert Fall Mar 24 '11 at 9:12
0
\$\begingroup\$

I used this with an element which was on the left of the container, which meant that the div put in unnecessary space on the right. I needed to adjust line 40 to the folloing:

    placeholder = $('<div style="display:none;"/>')

Not sure how you would do this automatically.

Also, what is the purpose of

    header.css('width', originalWidth + 'px');

Seems to work better without this for me.

\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.