3
\$\begingroup\$

Update: Great answers up to now, but I would still love to have a bit of review about possibility of variable caching and the filter I'm using for rows and page sets!


After seeing a lot of Paginator plugin that never fit my needs, I've decided to create my own. I have followed the jQuery Authoring advices.

Configuration prerequisites:

  1. The plugin CSS have to be fully configurable, thus classes name have to be dynamic.
  2. The plugin text/html on buttons else than page number have to be configurable.
  3. The number of buttons and the number of elements on a page have to be configurable.
  4. The configuration can be done by data- or by JavaScript.

Other prerequisites :

  1. The plugin have to build his own html.
  2. Basic plugin functions can be called after initialization :
    • update (to cast a showPage (example: the table order get modified by a sorter, and you want to refresh the row shown)
    • changeSettings (update dynamically plugin settings)
    • destroy (remove the generated plugin html)
  3. The plugin private variables are accessed by only getter outside of the constructor.
  4. More than on table can get "paginated" on the page. And each table can have its own settings.
  5. As side-effect-less as possible

What to I think could get some improvement :

  • I'm not very used with JavaScript scope and even less when in a plugin. Thus there might be a lot of more variable caching that could be done. The thing is that we have to keep in mind that the table could change dynamically (adding removing rows) and that should not affect anything.
  • The selector for the row shown and page set shown which looks ugly :

    ':eq(' + first + '),:eq(' + last + ')' + ',:lt(' + last + '):gt(' + first + ')'
  • The length of the file(?). I'm not very knowledgeable about good practices in JavaScript, and thus I would like to have your opinions. Should I cut it or let it like that?

I do not think optimization is really an issue here as I haven't had any issue yet with the plugin being slow.

I do think the code style is correct, but I am open to any suggestion has this is my first plugin. And if you have any else suggestion to make, feel free! I'm always happy to improve my coding practice and code.

Example of initialization the plugin :

$(document).ready(function () {
    $('table').paginate({'elemsPerPage': 2, 'maxButtons': 6})
});

Here is a live fiddle

Here is the code :

I know it's a bit long, but couldn't really reduce it.

(function ($, Math) {
    "use strict";

    var prefix = 'paginate';

    var defaults = {
        'elemsPerPage': 5,
        'maxButtons': 5,
        //Css classes
        'disabledClass': prefix + 'Disabled',
        'activeClass': prefix + 'Active',
        'containerClass': prefix + 'Container',
        'listClass': prefix + 'List',
        'previousClass': prefix + 'Previous',
        'nextClass': prefix + 'Next',
        'previousSetClass': prefix + 'PreviousSet',
        'nextSetClass': prefix + 'NextSet',
        'showAllClass': prefix + 'ShowAll',
        'pageClass': prefix + 'Page',
        'anchorClass': prefix + 'Anchor',
        //Text on buttons
        'previousText': '«',
        'nextText': '»',
        'previousSetText': '…',
        'nextSetText': '…',
        'showAllText': '⇓'
    };

    var methods = {
        init: function (options) {

            var table = $(this);
            if (table.data(prefix)) {
                return;
            }
            var paginator = new Paginator(this, options);

            table.data(prefix, paginator);
        },
        update: function (pageNumber) {
            $(this).data(prefix).showPage(pageNumber || 1);
        },
        changeSettings: function (options) {
            $(this).data(prefix).updateConfig(options || {}).showPage(1);
        },
        destroy: function () {
            var elem = $(this);
            elem.data(prefix).destroy();
            elem.removeData(prefix);
        }
    };

    $.fn.paginate = function (args) {
        return this.each(function () {
            if (methods[args]) {
                return methods[args].apply(this, Array.prototype.slice.call(arguments, 1));
            } else if (typeof args === 'object' || !args) {
                return methods.init.apply(this, [args]);
            } else {
                $.error('Incorrect usage of jQuery.Paginate');
            }
        });
    };

    var Paginator = function (element, options) {
        var table = $(element);
        var config = $.extend({}, defaults, table.data() || {}, options || {});
        var container = null;

        this.getConfig = function () {
            return config;
        };

        this.getContainer = function () {
            return container;
        };

        this.getTable = function () {
            return table;
        };

        this.getSelector = function (name) {
            if ($.isArray(name)) {
                var response = '';
                for (var index in name) {
                    response += this.getSelector(name[index]);
                }
                return response;
            }
            return '.' + config[name + 'Class'];
        };

        this.updateConfig = function (settings) {
            $.extend(config, settings);
            return this;
        };
        this.destroy = function () {
            container.remove();
            return table;
        };

        container = new Builder(this);
        table.before(container);
        this.showPage(1);
    };

    Paginator.prototype.previousPage = function () {
        var previousPageNumber = parseInt(
            this.getContainer().find(this.getSelector(['page', 'active'])).children('a').text()) - 1;
        if (isNaN(previousPageNumber)) {
            previousPageNumber = 1;
        }
        this.showPage(previousPageNumber);
        return this;
    };

    Paginator.prototype.nextPage = function () {
        var nextPageNumber = parseInt(this.getContainer().find(
            this.getSelector(['page', 'active'])).children('a').text()) + 1;
        if (isNaN(nextPageNumber)) {
            nextPageNumber = 1;
        }
        this.showPage(nextPageNumber);
        return this;
    };

    Paginator.prototype.previousSet = function () {
        var previousPage = parseInt(this.getContainer().find(
            this.getSelector('page')).filter(visibleFilter).first().children('a').text()) - 1;
        this.showSet(previousPage);
        return this;
    };

    Paginator.prototype.nextSet = function () {
        var nextPage = parseInt(this.getContainer().find(
            this.getSelector('page')).filter(visibleFilter).last().children('a').text()) + 1;
        this.showSet(nextPage);
        return this;
    };

    Paginator.prototype.showAll = function () {
        var config = this.getConfig();
        var container = this.getContainer();
        this.getTable().find('tbody tr').show();

        container.find(this.getSelector('active')).removeClass(config['activeClass']);
        container.find(this.getSelector('showAll')).addClass(config['activeClass']);
        container.find(this.getSelector('previous')).addClass(config['disabledClass']);
        container.find(this.getSelector('next')).addClass(config['disabledClass']);

        return this;
    };

    /**
     * 
     * @param {int} pageNumber 1indexed
     * @returns {_L1.Paginator.prototype}
     */
    Paginator.prototype.showPage = function (pageNumber) {

        var config = this.getConfig();
        var container = this.getContainer();
        var tableTr = this.getTable().find('tbody tr');
        var maxPageNumber = Math.ceil(tableTr.length / config['elemsPerPage']);

        if (pageNumber > maxPageNumber) {
            pageNumber = maxPageNumber;
        } else if (pageNumber < 1) {
            pageNumber = 1;
        }

        container.find(this.getSelector('disabled')).removeClass(config['disabledClass']);
        if (pageNumber === 1) {
            container.find(this.getSelector('previous')).addClass(config['disabledClass']);
        } else if (pageNumber === maxPageNumber) {
            container.find(this.getSelector('next')).addClass(config['disabledClass']);
        }

        var zeroIndexPN = pageNumber - 1;

        container.find(this.getSelector('active')).removeClass(config['activeClass']);
        container.find(this.getSelector('page')).eq(zeroIndexPN).addClass(config['activeClass']);

        tableTr.hide();

        var firstRow = (zeroIndexPN) * config['elemsPerPage'];
        var lastRow = firstRow + config['elemsPerPage'];

        tableTr.filter(':eq(' + firstRow + '),:lt(' + lastRow + '):gt(' + firstRow + ')').show();

        //Adjust the set
        this.showSet(pageNumber);

        return this;
    };

    /**
     * 
     * @param {integer} pageNumber 1 indexed.
     * @returns {_L1.Paginator.prototype}
     */
    Paginator.prototype.showSet = function (pageNumber) {
        var config = this.getConfig();
        var container = this.getContainer();

        // Zero Indexed
        --pageNumber;

        var numberOfPage = Math.ceil(this.getTable().find('tbody tr').length / config['elemsPerPage']) - 1;
        // 2 buttons (previous, next sets) + 1 the first button
        var maxButtons = config['maxButtons'] - 3;

        var firstPageToShow = Math.ceil(pageNumber - maxButtons / 2);
        var lastPageToShow = firstPageToShow + maxButtons;

        container.find(this.getSelector('previousSet')).show();
        container.find(this.getSelector('nextSet')).show();

        if (firstPageToShow <= 1) {
            ++maxButtons;
            firstPageToShow = 0;
            lastPageToShow = firstPageToShow + maxButtons;
            container.find(this.getSelector('previousSet')).hide();
        } else if (lastPageToShow >= (numberOfPage - 1)) {
            ++maxButtons;
            lastPageToShow = numberOfPage;
            firstPageToShow = (numberOfPage > maxButtons) ? numberOfPage - maxButtons : 0;
            container.find(this.getSelector('nextSet')).hide();
        }

        var pages = container.find(this.getSelector('page'));
        pages.hide();

        pages.filter(':eq(' + firstPageToShow + '),:eq(' + lastPageToShow + ')' +
            ',:lt(' + lastPageToShow + '):gt(' + firstPageToShow + ')').show();
        return this;
    };

    // NEW BUILDER -------------------------------
    var Builder = function (paginate) {
        var config = paginate.getConfig();
        var container = paginate.getContainer();

        var create = function (name, attr) {
            if (typeof attr === 'string') {
                attr = {'class': attr};
            }
            return $('<' + name + '>', attr || {});
        };

        var createLi = function (type, content) {
            return create('li', {
                'class': config[type + 'Class'],
                'html': create('a', {
                    'class': config.anchorClass,
                    'html': content || config[type + 'Text']
                })
            });
        };

        var tableLength = paginate.getTable().find('tbody tr').length;
        var numberOfPage = Math.ceil(tableLength / config.elemsPerPage);

        var inNeedOfSetButtons = false;
        if (numberOfPage > config.maxButtons) {
            inNeedOfSetButtons = true;
        }

        container = create('div', config.containerClass);
        var list = new Array(numberOfPage + inNeedOfSetButtons * 2 + 2),
            index = 0;

        list[index++] = createLi('previous').click(function (e) {
            e.preventDefault();
            paginate.previousPage.apply(paginate);
        });

        if (inNeedOfSetButtons) {
            list[index++]= createLi('previousSet').click(function (e) {
                e.preventDefault();
                paginate.previousSet.apply(paginate);
            });
        }

        for (var i = 1; i <= numberOfPage; i++) {
            list[index++] = createLi('page', i).click(function (e) {
                e.preventDefault();
                paginate.showPage.apply(paginate, [parseInt($(this).text())]);
            });
        }

        if (inNeedOfSetButtons) {
            list[index++] = createLi('nextSet').click(function (e) {
                e.preventDefault();
                paginate.nextSet.apply(paginate);
            });
        }

        list[index++] = createLi('next').click(function (e) {
            e.preventDefault();
            paginate.nextPage.apply(paginate);
        });


        var listObj = create('ul', config.listClass);
        listObj.append(list);

        container.append(
            [
                listObj,
                create('ul', config['showAllListClass']).append(
                    createLi('showAll').click(function (e) {
                    e.preventDefault();
                    paginate.showAll.apply(paginate);

                }))
            ]);
        return container;
    };

    function visibleFilter () {
        return $(this).css('display') !== 'none';
    }

})(jQuery, Math);

EDIT : After comments, I've updated the builder. Let me know what you think about it. The old version is still in the JsFiddle.

Note: I posted a question for the CSS as the issues of CSS and JavaScript are really not related, thus no need to mention CSS style it here.

\$\endgroup\$

closed as off-topic by Jamal Jan 3 '15 at 2:35

If this question can be reworded to fit the rules in the help center, please edit the question.

  • \$\begingroup\$ I'm closing this as the question is now harder to follow for future visitors and reviewers, and the updated answer prevents this question's update from being justifiably reversed. \$\endgroup\$ – Jamal Jan 3 '15 at 2:35
1
\$\begingroup\$

You do have a lot of code for review so here are some quite general suggestions:

Code length shouldn't be a problem when you minify/gzip. Even though it is 300+ lines. If you can reduce more, power to you! I see you've used prototypes well and that should have already reduced quite a bit. Maybe focus on the Builder constructor for future reduces, as I do see some very similar, but not repeated, code there. The main thing I'm concerned in Builder is all the appends. What you might want to do is separate your tests, and set variables with the results, then call a single (or fewer) appends if at all possible. You avoided a common beginner mistake of putting an append in a loop so kudos to you.

Some reading materials for appends which I recommend: http://www.learningjquery.com/2009/03/43439-reasons-to-use-append-correctly http://rune.gronkjaer.dk/en-US/2010/08/14/jquery-append-speed-test/

I'll add more to this answer concerning the rest of your plugin as I get some more time. One last thing that can help you with scopes is if you use Chrome Dev tools and set breakpoints, on the right you can see scope variables and a whole lot more.

UPDATE:

I've seen your update and refactored parts of the Builder for ya. Here's the Fiddle. The parts I've edited follow:

//Edits start here
container = create('div', config.containerClass);
var list = [numberOfPage + inNeedOfSetButtons * 2 + 2], //Use the bracket notation instead: var list = [...];
    index = 0,
    prevOrNext = ["previous", "next"]; //Two mainly used values

$.each(prevOrNext, function(n, v) { //Reduced your code. Read jQuery.each docs for clarification.
    list[index++] = createLi(v).on('click', function(e) {
        e.preventDefault();
        paginate[v + "Page"].apply(paginate);
    });
});

if (inNeedOfSetButtons) {
    $.each(prevOrNext, function(n, v) {
        list[index++] = createLi(v + 'Set').on('click', function(e) {
            e.preventDefault();
            paginate[v + "Set"].apply(paginate);
        });
    });
}

for (var i = 1; i <= numberOfPage; i++) {
    list[index++] = createLi('page', i).on('click', function (e) {
        e.preventDefault();
        paginate.showPage.apply(paginate, [parseInt($(this).text())]);
    });
}

list[list.length] = list[3]; //Move the nextSet to end.
list[list.length +1] = list[1]; //Move paginateNext item to the end.
var listObj = create('ul', config.listClass);
listObj.append(list);
\$\endgroup\$
0
\$\begingroup\$

Some minor things I did not see mentioned before:

  • .nextPage() refers to config, and then does not use it
  • .previousSet() refers to config, and then does not use it
  • .nextPage() is using the UI to determine the next page, meaning that you violate MVC here. currentPage should be a property of your class with a getter/setter
\$\endgroup\$
  • \$\begingroup\$ You are right about "refers to config" then does not use it! It's caused by some refactor, where I forgot to remove the unused var! Might want to edit the word MVC not sure thats the word you wanted since this plugin is not MVC! \$\endgroup\$ – Hugo Dozois Mar 22 '13 at 19:32
  • 1
    \$\begingroup\$ I mean that you store state information 'which page are we on' not in your class, you use the UI as a means to know where you are, and hence you are not separating the UI and state(model) concerns. \$\endgroup\$ – konijn Mar 22 '13 at 19:35

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