Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.
/* 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);
share|improve this question
    
What is a Wookmark layout? –  Eric Bréchemier Oct 10 '12 at 14:48
    
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 –  paritosh Nov 8 '12 at 12:04

2 Answers 2

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();
      });
    };
    
share|improve this answer
    
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. –  James Khoury Nov 21 '12 at 0:18

use jslint, fix all warnings then. 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 js). 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. For example
$("#wookmark-pagination-loader").fadeOut();
push $("#wookmark-pagination-loader") to plugin like argument or cahche it in var and make method, that will show and hide loading.
Also each block of code after comment you can substitute with 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 js some times.
var loadingContainer = $(document.getElementById('wookmark-pagination-loader')); showLoading = function () { this.loadingContainer.fadeIn(); }
for old browser add 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, substitute them with var. fetchReady : { status : true },
Use explicit comparisons
if (self.fetchReady.status === true) {...

Well, really, I can find much more "clever" advices. But IMHO it is nit-picking. It works, it's readable, it's compact, it's OK.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

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