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

jQuery plugin for ajax select fill

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

Problem

I've created my first jQuery plugin, so please try to understand :)

My plugin code looks like so:

(function($) {
$.fn.ajaxSelect = function(options) {
    var $this = this;
    //options
    var settings = $.extend({}, defaults, options);
    //disable select
    if ($.ui.selectmenu && settings.selectmenu && settings.disableOnLoad) {
        $this.selectmenu('disable');
    }
    //ajax call
    $.ajax({
        type: settings.type,
        contentType: settings.contentType,
        url: settings.url,
        dataType: settings.dataType,
        data: settings.data
    }).done(function(data) {
        var n = data.d || data;
        var list = "";
        $.each(n, function(i) {
            list += '' + n[i].Nazwa + '';
        });
        $this.filter("select").each(function() {
            $(this).empty();
            $(this).append(list);
            if ($.ui.selectmenu && settings.selectmenu) {
                $this.selectmenu();
            }
        });
        settings.success.call(this);
    }).fail(function() {
        settings.error.call(this);
    });

    return this;
};

var defaults = {
    type: "POST",
    contentType: "application/json; charset=utf-8",
    url: '/echo/json/',
    dataType: 'json',
    data: null,
    async: true,
    selectmenu: true,
    disableOnLoad: true,
    success: function() {},
    error: function() {}
};
})(jQuery);


Demo of usage is available at http://jsfiddle.net/Misiu/ncWEw/

What I was trying to accomplish:

  • Select multiple elements at one time



  • Filter only selects from selected items (in case selector would select other elemnts)



  • Makes only ONE request to server



  • First build option string and then append it instead of adding items in loop



  • Specify 2 callbacks: one for error and second for success



I think that it would be better to use more of deferred objects, but I don't have idea how.
Any improvements and hints are welcome :)

Solution

Your demo throws errors in console, something about uniqueId in ui.selectmenu.

Anyway:

  • You can expose $.ajax deferred so someone may use all features of jQuery.Deferred which is already there.



-
Call success callback with data as argument.

settings.success.call(this, n);


-
If you do not want to do anything special in case of failure, and just want to provide this to error callback, you do not have to create closure there.

.fail( settings.error );


..., unless you want to hide other rejection arguments.

-
Do not use each if for do the same (for performance reason):

for( var index = 0; index ' + n[index].Nazwa + '';
}


  • Personally I don't like single char variable names, as they are really painful for future development



-
Instead of $(this).empty();$(this).append(list); you can simply use $(this).html(list);

-
You can expose jQuery.Deffered, for example as this.promise = $.ajax(..., example in here http://jsfiddle.net/dhRRN/5/

-
I would suggest also applying received options to ajax call, so your plugin's user could also alter request itself.

Code Snippets

settings.success.call(this, n);
.fail( settings.error );
for( var index = 0; index < n.length; index ++){
  list += '<option value=' + n[index].Id + '>' + n[index].Nazwa + '</option>';
}

Context

StackExchange Code Review Q#18897, answer score: 3

Revisions (0)

No revisions yet.