Infinite scroll pagination with a Wookmark layout

Question

/* 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);

• user185: What is a Wookmark layout?
• Paritosh: 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

Answer 1

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();
   });
  };
  

• James Khoury: 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.

Answer 2

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.