3
\$\begingroup\$

I have been writing a lot of jQuery plugins lately, but I don't really know if what I am doing is any good. Here is one of my most recent plugins. It allows the user to add a data attribute to any class and load a full page view window (which is written to the body on render) to e.g. open a menu on mobile or read large pieces of text without taking up the entire page (scrolling on a product page for example):

(function ($) {
    var jelm = $('.js-full-page-view');

    var methods = {
        init: function () {
            jelm.each(function () {
                var id = $(this).attr('id'),
                    title = ($(this).data('caFullPageTitle')) ? $(this).data('caFullPageTitle') : '',
                    content = (!$(this).data('caSingle')) ? $(this).html() : '',
                    action_container = ($(this).data('caActionContainer')) ? $(this).data('caActionContainer') : '',
                    indented_content = ($(this).data('caNoIndentedContent')) ? '' : 'full-page-view--indented-content',
                    bottom_action_container = '';

                if ($(this).data('caBottomActionContainer')) {
                    bottom_action_container = '<div class="full-page-view--footer-bar">' + $(this).data('caBottomActionContainer') + '</div>';
                }

                $('body').append(
                    '<div class="full-page-view js-full-page-view" id="fp_' + id
                    + '"><div class="full-page-view--main-bar"><div class="full-page-view--action-container"><button class="full-page-view--action js-full-page-view-close"><svg class="icon-svg alt-flip"><use xlink:href="#icon-arrow"></use></svg></button></div><div class="full-page-view--title">' + title
                    + '</div><div class="full-page-view--action-container">' + action_container + '</div></div><div class="full-page-view--scroll-pane full-page-view--content ' + indented_content + '">' + content
                    + '</div>' + bottom_action_container + '</div>');
            });
            methods.bind();
        },
        bind: function () {
            $('.js-full-page-view-open').on('click', function () {
                var open_elm = ($(this).data('caTarget')) ? $("#" + $(this).data('caTarget')) : $(this).closest('.js-full-page-view');
                if (methods.responsive_check(open_elm)) {
                    methods.open('#fp_' + open_elm.attr('id'));
                }
            });

            $('.js-full-page-view-close').on('click', function () {
                var close_elm = ($(this).data('caTarget')) ? $(this).data('caTarget') : $(this).closest('.js-full-page-view');
                methods.close('#' + close_elm.attr('id'));
            });
        },
        close: function (selector) {
            $(selector).removeClass('active').ceScrollLock('body', 'rm');
            if (methods.single_check('#' + selector.substring(4))) {
                var focus = $(selector + ' .full-page-view--content');
                var content = $(focus).html();
                $('#' + selector.substring(4) + ' .full-page-view--content-placeholder').append(content);
                focus.empty();
            }
        },
        open: function (selector) {
            $(selector).addClass('active');
            setTimeout(function() {$(selector).ceScrollLock('body', 'add');}, 200);
            if (methods.single_check('#' + selector.substring(4))) {
                var focus = $('#' + selector.substring(4) + ' .full-page-view--content-placeholder');
                var content = $(focus).html();
                $(selector + ' .full-page-view--content').append(content);
                focus.empty();
            }
        },
        responsive_check: function (selector) {
            return $.fn.getRealWidth() <= $(selector).data('caResponsiveMax');
        },
        responsive_resize: function () {
            $('.js-full-page-view.active').each(function() {
                if (!methods.responsive_check('#' + $(this).attr('id').substring(3))) {
                    methods.close('#' + $(this).attr('id'));
                }
            });
        },
        single_check: function (elem) {
            return !!$(elem).data('caSingle');
        }
    };

    $.fn.ceFullPageView = function (method) {
        return $(this).each(function (i, elm) {
            var errors = {};

            if (methods[method]) {
                return methods[method].apply(this, Array.prototype.slice.call(arguments, 1));
            } else if (typeof method === 'object' || !method) {
                return methods.init.apply(this, arguments);
            } else {
                $.error('fullpageview: method ' + method + ' does not exist');
            }
        });
    };
    
    if ($(document).ready()) {
        $('.js-full-page-view').ceFullPageView();

        $('.js-full-page-view-open').on('click', function () {
            $(this).ceFullPageView('bind');
        });
        $('.js-full-page-view-close').on('click', function () {
            $(this).ceFullPageView('bind');
        });

        $(window).resize(function () {
            $(this).ceFullPageView('responsive_resize');
        });
    }
})(jQuery);

And the supportive functions

(function (_, $) {
    var methods = {
        rm: function () {
            $('body').removeClass('is-non-scrollable-fixed');
        },
        add: function() {
            $('body').addClass('is-non-scrollable-fixed');
        }
    };

    $.fn.ceScrollLock = function (selector, method) {
        return methods[method].apply(this, arguments);
    };

    $.fn.ceIsIE = function (userAgent) {
        userAgent = userAgent || navigator.userAgent;
        return userAgent.indexOf("MSIE ") > -1 || userAgent.indexOf("Trident/") > -1 || userAgent.indexOf("Edge/") > -1;
    };

    $.fn.getRealWidth = function () {
        var outer = $('<div>').css({visibility: 'hidden', width: 100, overflow: 'scroll'}).appendTo('body'),
            widthWithScroll = $('<div>').css({width: '100%'}).appendTo(outer).outerWidth();
        outer.remove();
        var scrollBarWidth = 100 - widthWithScroll;

        if ($.fn.ceIsIE) {
            return window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;
        }

        if ($('.is-non-scrollable-fixed').length) {
            return $('body').width();
        } else {
            return $('body').width() + scrollBarWidth;
        }
    };
})(jQuery);

And the HTML:

<div class="js-full-page-view" data-ca-single="true" data-ca-responsive-max="667" data-ca-full-page-title="{sl 'filter'}" id="full_page_product_filter">
    <div class="full-page-view--content-placeholder"> 
        Any HTML content
    <div>
</div>

Let me know what you think.

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for adding the HTML and supportive js functions. I tried updating the fiddle cited in my answer but it didn’t work. I noticed the IiFE for the supportive functions passes one argument (I.e. jQuery) while the function definition expects two two parameters (I.e. _, $). Is underscore or old ash required for it to work? I don’t actually see _ used within the function... \$\endgroup\$ Commented Sep 9, 2018 at 6:24

1 Answer 1

1
\$\begingroup\$

Feedback

Overall the code seems to be architected acceptably (though see the first review point below about the DOM-ready code). Some of the HTML generation seems complex - perhaps using a template system (e.g. using a template <script> tag) would be a good way to abstract out that markup. I tried making a jsfiddle to use the plugin but without knowing what all HTML elements were required I wasn't able to really get it running as expected. Feel free to fork my fiddle, add requisite HTML elements and edit your post to contain a link to such a fiddle.

Review points

DOM-ready code

Towards the end of the code I see this:

if ($(document).ready()) {
      $('.js-full-page-view').ceFullPageView();

I honestly haven't seen much code like that before. .ready() returns a jQuery object. So that would be similar to the following:

if({"0": {"location": {"replace": function() {}} /* ... */}}) {

And any object used as an expression in a conditional statement evaluates to a truthy value1. Typically, the code to be evaluated when the DOM is ready is passed in a callback function - like this:

$(document).ready(function() {
    $('.js-full-page-view').ceFullPageView();
    $('.js-full-page-view-open').on('click', function () {
        $(this).ceFullPageView('bind');
    });
    $('.js-full-page-view-close').on('click', function () {
        $(this).ceFullPageView('bind');
    });

    $(window).resize(function () {
        $(this).ceFullPageView('responsive_resize');
    });
});

But the second line of the code does a DOM lookup:

(function ($) {
    var jelm = $('.js-full-page-view');

So perhaps the whole code should be wrapped in a callback when the DOM is ready. You didn't mention which version of jQuery is used/supported, but presuming it is 3.0 or higher (correct me if that is incorrect) the form $(document).ready() is deprecated2 and the only supported form is $(handler) so the code could be updated like below. Additionally, there is a shortcut method for .on('click' - .click().

$(function() {
    var jelm = $('.js-full-page-view');

    var methds = {
        /** methods **/
    };

    $.fn.ceFullPageView = function (method) {
        return $(this).each(function (i, elm) {
            /** skipped for brevity **/
        });
    }

    $('.js-full-page-view').ceFullPageView();

    $('.js-full-page-view-open').click(function () {
        $(this).ceFullPageView('bind');
    });
    $('.js-full-page-view-close').click(function () {
        $(this).ceFullPageView('bind');
    });

    $(window).resize(function () {
        $(this).ceFullPageView('responsive_resize');
    });

});

DOM lookups

I see many DOM lookups throughout but the selectors differ a bit so while I normally recommend those be cached in a variable, I don't see many that are redundant and it might not be feasible given the dynamic nature of the code. But jelm could be used in the first line of the code currently called after $(document).ready():

$('.js-full-page-view').ceFullPageView();

Can be changed to:

jelm.ceFullPageView();

function ceScrollLock()?

I am not familiar with that function, nor could I find any webpages online (besides this one and an untitled paste on pastebin.com which I presume is yours) that contain that string. Where is it defined?

1https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Operators/Logical_Operators#Description 2http://api.jquery.com/ready/

\$\endgroup\$
1
  • \$\begingroup\$ Thanks for your reply, I have added the HTML and other JS functions. \$\endgroup\$
    – Enigmatic
    Commented Sep 7, 2018 at 5:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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