HiveBrain v1.2.0
Get Started
← Back to all entries
patternjavascriptMinor

jQuery Load More Plugin

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
pluginjquerymoreload

Problem

This is a simple load more plugin, built for teaching purposes. For this reason, it's not pretty in terms of output.

I'd like to know if this can be improved, or if anyone can spot any major problems with this. The one thing I'm concerned about is the 'templating', whereby it gets the template from the initial markup and duplicates it.

loadmore.js

```
/jslint unparam: true, white: true /

(function($) {
"use strict";

$.fn.loadmore = function(options) {

var self = this,

settings = $.extend({
source: '',
step: 2
}, options),

stepped = 1,
item = self.find('.item'),

finished = function() {
self.find('.items-load').remove();
},

append = function(value) {
var name,
part;

item.remove();

for(name in value) {
if(value.hasOwnProperty(name)) {
part = item.find('*[data-field="' + name + '"]');

if(part.length) {
part.text(value[name]);
}
}
}

item.clone().appendTo(self.find('.items'));
},

load = function(start, count) {
$.ajax({
url: settings.source,
type: 'get',
dataType: 'json',
data: {start: start, count: count},
success: function(data) {

var items = data.items;

if(items.length) {
$(items).each(function(index, value) {
append(value);
});

stepped = stepped + count;
}

if(data.last === true) {
finished();
}
}
});
};

if(settings.source.length) {

// Event handler for load more button

Solution

From some staring at your code

  • There are no comments, at all



  • You $.ajax({ call should handle failure by implementing error



-
The items treatment in success seems little clumsy, first I would not put the function inside the ajax call, secondly I would probably go with something like this:

success: function(data) {

    $(data.items).each(function(index, value) {
        append(value);
    });
    if(data.last === true) {
        finished();
    }
}


  • I do not understand the value of stepped, what does it bring to your plugin ? It would have been far better to keep the stepping logic out of the plugin, and let the caller pass a data object that will be passed to the ajax call.



  • '.items-load' is a magic, repeated constant, you should use a properly named variable



  • item.find('*[data-field="' + name + '"]');



  • "use strict"; on the top level, good stuff



  • Passes perfectly on JsHint.com



  • if(value.hasOwnProperty(name)) {



-
jQuery handles empty/undefined gracefully, so that you can write nicer code, this:

part = item.find('*[data-field="' + name + '"]');

if(part.length) {
    part.text(value[name]);
}


could be this:

item.find('#'+name).text(value[name]);


-
The CSS names you use are very generic, you might want to consider adding a small prefix to avoid name clashes

  • loadmore -> loadMore because lowerCamelCase



  • console.log in production is a no-no

Code Snippets

success: function(data) {

    $(data.items).each(function(index, value) {
        append(value);
    });
    if(data.last === true) {
        finished();
    }
}
part = item.find('*[data-field="' + name + '"]');

if(part.length) {
    part.text(value[name]);
}
item.find('#'+name).text(value[name]);

Context

StackExchange Code Review Q#44948, answer score: 3

Revisions (0)

No revisions yet.