8
\$\begingroup\$
/* Infinite Scroll Pagination with a Wookmark layout
 *
 * Requires: jQuery, Wookmark
 */

var myApp = myApp || {};

;(function(myApp,$){
  var WookmarkPagination = function(elem, options){
    this.elem = elem;
    this.$elem = $(elem);
    this.options = options;
  };

  WookmarkPagination.prototype = {
    defaults: {
      url: '',
      page: 2,
      per_page: 5,
      template: '',
      scroll: true,
      item: ".item",
      items: ".items",
      loader_image: ''
    },

    fetchReady: {
      status: true
    },

    /**
     * Initiate app. Bind the scroll event and onScroll function to the document.
     *
     * @return this
     */
    init: function(){
      var _this = this;
      _this.config = $.extend({}, _this.defaults, _this.options);
      var loader = '<div id="wookmark-pagination-loader"><img src="'+_this.config.loader_image+'" alt="Loading..." /></div>';

      // Set loader
      $(loader).appendTo("body").hide();

      // Build wookmark layout
      wookmark_handler = this.$elem.find(_this.config.item);
      wookmark_handler.wookmark({
        container: _this.$elem,
        offset: 15,
        itemWidth: 160,
        autoResize: true
      });

      if(_this.config.scroll){
        $(document).bind('scroll', function(){
          _this.onScroll();
        });
      }
      return _this;
    },

    /**
     * Rebuild Wookmark layout
     *
     * @return void
     */
    refreshLayout: function(){
      var _this = this;
      var opt = _this.config;
      //console.log(wookmark_handler);

      // Rebuild wookmark positions
      if(wookmark_handler) wookmark_handler.wookmarkClear();
      wookmark_handler = _this.$elem.find(_this.config.item);

      // Initialize Wookmark to build layout container
      wookmark_handler.wookmark({
        container: _this.$elem,
        offset: 15,
        itemWidth: 160,
        autoResize: true
      });
    },

    /**
     * Fetches the JSON from the server. On success rebuilds Wookmark positioning.
     *
     * @return the JSON object
     */
    fetch: function(){
      var _this = this;
      var opt = _this.config;

      // If no other getJSON is being processed continue
      if(_this.fetchReady.status){
        // Block other requests
        _this.fetchReady.status = false;

        // Show loader
        $("#wookmark-pagination-loader").fadeIn();

        // Fetch Data
        return $.getJSON(opt.url,{
          page: opt.page,
          per_page: opt.per_page
        })
        .success(function(data){
          // Each data item is wrapped in a template and appended to the page.
          $.each(data, function(i, item){
            _this.$elem.find(_this.config.items).append(Mustache.to_html(opt.template, item.item));
          });

          // Set next page number for next request.
          opt.page += 1;

          _this.refreshLayout();

        })
        // TODO: setup error catching and show a user there was a problem with the request
        //.error(function(data, response){})
        .complete(function(data){
          // Allow other getJSON requests
          _this.fetchReady.status = true;

          // Hide loader
          $("#wookmark-pagination-loader").fadeOut();
        });
      }
    },

    /**
     * Checks if the scroll position has reached the bottom, then runs the fetch function
     *
     * @return void
     */
    onScroll: function() {
      // Check if we're within 100 pixels of the bottom edge of the broser window.
      var closeToBottom = ($(window).scrollTop() + $(window).height() > $(document).height() - 100);
      if(closeToBottom) {
        this.fetch();
      }
    }
  };

  WookmarkPagination.defaults = WookmarkPagination.prototype.defaults;

  $.fn.wookmark_pagination = function(options) {
    return this.each(function() {
      new WookmarkPagination(this, options).init();
    });
  };

  myApp.WookmarkPagination = WookmarkPagination;
})(myApp,jQuery);
\$\endgroup\$
2
  • \$\begingroup\$ What is a Wookmark layout? \$\endgroup\$ Oct 10, 2012 at 14:48
  • \$\begingroup\$ Looks okay as per my review, no problem at all.. it will be better if you can give us a sample how actually you are using \$\endgroup\$
    – Paritosh
    Nov 8, 2012 at 12:04

2 Answers 2

2
\$\begingroup\$

The code looks pretty good, but here are a couple suggestions:

  • The semicolon at the top is unnecessary: ;(function(myApp,$){
  • You use _this in many places where this would work:
    • Consider Function.bind (see MDN, only works on modern browsers)
    • This very much depends on personal style
  • This is a little confusing. I don't write jQuery much, but it isn't clear what the first this is referring to (and the second this has a different meaning than the one above, which is also confusing):

    $.fn.wookmark_pagination = function(options) {
      return this.each(function() {
        new WookmarkPagination(this, options).init();
      });
    };
    
\$\endgroup\$
1
  • \$\begingroup\$ this references jQuery iirc. Its standard for this type of code. Definitely agree there is no need for both this and _this in most places and is inconsistent. \$\endgroup\$ Nov 21, 2012 at 0:18
0
\$\begingroup\$

Use jslint to fix all warnings . It is not necessary, but sometimes helpful. There are some bad things, like using vars without declaration and so on, and also bad formatting (can use jsmin to format JavaScript). I'd also add $(document) and $(window) as argument to plugin and use them inside plugin.

There are also many common thing that you repeat:

$("#wookmark-pagination-loader").fadeOut();

Push $("#wookmark-pagination-loader") to plugin like argument or cache it in var and make method, that will show and hide loading.

Also, each block of code after comment you can substitute with a function call:

// Build wookmark layout
wookmark_handler = this.$elem.find(self.config.item);
    wookmark_handler.wookmark({
    container : self.$elem,
    offset : 15,
    itemWidth : 160,
    autoResize : true
});

function builtWookmarkLayout() {
   ... //do things here
}

Use some hints to make jQuery faster. Cache requests to DOM tree in variables, use pure JavaScript sometimes.

var loadingContainer = $(document.getElementById('wookmark-pagination-loader'));
showLoading = function () {
    this.loadingContainer.fadeIn();
}

For old browser, add an explicit function create like:

if (typeof Object.create !== 'function') {
    Object.create = function (obj) {
        function F() {};
        F.prototype = obj;
        return new F();
    }
}

You also use objects with exactly one field, so substitute them with var.

fetchReady : {
    status : true
}

Use explicit comparisons:

if (self.fetchReady.status === true) {...

I can find much more "clever" advice. But in my opinion, it is nit-picking. It works, it's readable, it's compact, it's OK.

\$\endgroup\$

Your Answer

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

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