patternjavascriptMinor
jQuery plugin to ajaxify forms
Viewed 0 times
formspluginjqueryajaxify
Problem
This is my first foray into the wild world of jQuery plugin development. The plugin "ajaxifies" forms, which isn't particularly spectacular:
Everything seems to be working as expected (repo, with a simple demo). Since this is my first plugin, I'd be particularly interested in critiques of its structure. That said, as always in reviews, any aspect of the code is fair game for criticism.
The plugin code:
```
(function ($) {
$.fn.ajaxForm = function (options) {
var settings = $.extend({}, $.fn.ajaxForm.defaults, options);
return this.each(function () {
var $form = $(this);
dataType = $form.attr("data-dataType");
if (typeof dataType != "undefined") dataType = settings.dataType;
$form.on("submit", function (event) {
event.preventDefault();
$.ajax({
type: $form.attr("method"),
url: $form.attr("action"),
data: $form.serialize(),
cache: false,
dataType: dataType,
beforeSend: function () {
$form.addClass("ajaxForm-loading");
if (typeof settings.beforeSend == "function") settings.beforeSend.call($form);
},
complete: function () {
$form.removeClass("ajaxForm-loading");
if (typeof settings.complete == "function") settings.complete.call($form);
},
success: function (response) {
if (typeof settings.success == "functio
- It gets the basics (action, method) from the form itself, serializes the form's fields, and sends the request.
- Users can provide (via its options) callback functions for the local ajax events (beforeSend, complete, success and error).
- The dataType can also be set from a custom data attribute, which - if present - takes precedence.
Everything seems to be working as expected (repo, with a simple demo). Since this is my first plugin, I'd be particularly interested in critiques of its structure. That said, as always in reviews, any aspect of the code is fair game for criticism.
The plugin code:
```
(function ($) {
$.fn.ajaxForm = function (options) {
var settings = $.extend({}, $.fn.ajaxForm.defaults, options);
return this.each(function () {
var $form = $(this);
dataType = $form.attr("data-dataType");
if (typeof dataType != "undefined") dataType = settings.dataType;
$form.on("submit", function (event) {
event.preventDefault();
$.ajax({
type: $form.attr("method"),
url: $form.attr("action"),
data: $form.serialize(),
cache: false,
dataType: dataType,
beforeSend: function () {
$form.addClass("ajaxForm-loading");
if (typeof settings.beforeSend == "function") settings.beforeSend.call($form);
},
complete: function () {
$form.removeClass("ajaxForm-loading");
if (typeof settings.complete == "function") settings.complete.call($form);
},
success: function (response) {
if (typeof settings.success == "functio
Solution
Structure and style looks fine, although I have some general notes:
In terms of features/behavior:
I'd highly suggest simply modifying the user's settings, and then passing them to
Besides that:
Most importantly: Please forward any and all arguments to the callbacks. For instance, the regular
By the way, it'd be in the spirit of jQuery to invoke the callbacks in the element's context, not in the context of the jQuery wrapper around the element. But if you follow my suggestion of modifying the settings rather than replacing them, be sure to note in your documentation that this context-switching only applies to those four callbacks. E.g. if I declare a dynamic
You may also want to get some of the form's attributes using
Here's what I came up with (untested)
- Prefer the strong (in)equality operators (
!==and===) whenever possible
- Always use braces, even for one-line "blocks". It's nice to save a few keystrokes, but personally I prefer the consistency of always using braces
- You could skip the defaults for the
beforeSend,complete, etc. callbacks and leave them undefined. Defining them as empty functions is fine, but not necessary with the checks you're doing.
In terms of features/behavior:
I'd highly suggest simply modifying the user's settings, and then passing them to
$.ajax(), rather than declare a new object. That way, users can set whatever extra ajax options they want, and not see them ignored. In other words, let your plugin fill in the blanks in the settings the user passes in, not the other way around.Besides that:
- You may want to skip elements that aren't actually forms
- It'd probably be wise to use
$form.data("dataType")instead of$form.attr()in case the data type was set using.data()and isn't a real attribute.
- Speaking of data type, you should check if it's a string specifically, since that's the only thing jQuery wants anyway.
- Lastly about the data type: I don't think your plugin should define a
"json"default. Better to let the be defined explicitly by whomever is using the plugin, or leave it at jQuery's default.
Most importantly: Please forward any and all arguments to the callbacks. For instance, the regular
beforeSend receives the XHR object and its settings, but if I specify a beforeSend using this plugin, I won't get those. Since the name's the same, I'd expect it to receive the same arguments. I like to use beforeSend to grab the XHR so I can use it as a Deferred/Promise object, but I can't do that here.By the way, it'd be in the spirit of jQuery to invoke the callbacks in the element's context, not in the context of the jQuery wrapper around the element. But if you follow my suggestion of modifying the settings rather than replacing them, be sure to note in your documentation that this context-switching only applies to those four callbacks. E.g. if I declare a dynamic
201 callback to handle the 201 Created status code, it won't be called in the element's context (and I can think of a clean way to make it happen).You may also want to get some of the form's attributes using
.prop() instead of .attr(). For instance, a method attribute is not required (it'll default to "GET" in that case). Also, the accept attribute is worth checking and carrying over to the ajax options, if present.Here's what I came up with (untested)
(function ($) {
$.fn.ajaxForm = function (options) {
options = $.extend({}, $.fn.ajaxForm.defaults, options);
// A simple helper function
function callHandler(name, form, args) {
if(typeof options[name] === "function") {
options[name].apply(form, [].slice.call(args, 0));
}
}
return this.each(function () {
var settings,
form = this;
settings = $.extend({}, {
action: $form.prop("action"),
method: $form.prop("method"),
accepts: $form.attr("accept"),
dataType: $form.data("dataType")
}, options);
settings.beforeSend = function () {
$form.addClass("ajaxForm-loading");
callHandler("beforeSend", form, arguments);
};
settings.complete = function () {
$form.removeClass("ajaxForm-loading");
callHandler("complete", form, arguments);
};
settings.success = function () {
callHandler("success", form, arguments);
};
settings.error = function () {
callHandler("error", form, arguments);
};
$(this).on("submit", function (event) {
event.preventDefault();
$.ajax(settings);
});
});
};
$.fn.ajaxForm.defaults = {};
})(jQuery);Code Snippets
(function ($) {
$.fn.ajaxForm = function (options) {
options = $.extend({}, $.fn.ajaxForm.defaults, options);
// A simple helper function
function callHandler(name, form, args) {
if(typeof options[name] === "function") {
options[name].apply(form, [].slice.call(args, 0));
}
}
return this.each(function () {
var settings,
form = this;
settings = $.extend({}, {
action: $form.prop("action"),
method: $form.prop("method"),
accepts: $form.attr("accept"),
dataType: $form.data("dataType")
}, options);
settings.beforeSend = function () {
$form.addClass("ajaxForm-loading");
callHandler("beforeSend", form, arguments);
};
settings.complete = function () {
$form.removeClass("ajaxForm-loading");
callHandler("complete", form, arguments);
};
settings.success = function () {
callHandler("success", form, arguments);
};
settings.error = function () {
callHandler("error", form, arguments);
};
$(this).on("submit", function (event) {
event.preventDefault();
$.ajax(settings);
});
});
};
$.fn.ajaxForm.defaults = {};
})(jQuery);Context
StackExchange Code Review Q#35488, answer score: 4
Revisions (0)
No revisions yet.