/* 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);
-
\$\begingroup\$ What is a Wookmark layout? \$\endgroup\$– Eric BréchemierOct 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\$– ParitoshNov 8, 2012 at 12:04
2 Answers
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 wherethis
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(); }); };
-
\$\begingroup\$
this
referencesjQuery
iirc. Its standard for this type of code. Definitely agree there is no need for boththis
and_this
in most places and is inconsistent. \$\endgroup\$ Nov 21, 2012 at 0:18
Use jslint to fix all warnings . It is not necessary, but sometimes helpful.
There are some bad things, like using var
s 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.