patternjavascriptMinor
jQuery form data
Viewed 0 times
formjquerydata
Problem
Could someone please help me make this code as professional as it can be? It works fine, but I feel there's a better way of doing this. I would really appreciate some guidance so I learn to code better.
```
$(function () {
$('#subForm').submit(function (e) {
e.preventDefault();
$.getJSON(
this.action + "?callback=?",
$(this).serialize(),
function (data) {
if (data.Status === 400) {
$('#slidemarginleft p').append("" + data.Message);
$("#slidemarginleft p").append('OK, I'll try again');
$(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});
});
//add click functionality for the button
$('#slidemarginleft button.wee').click(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});
setTimeout(function() {
$('#slidemarginleft p').empty();}, 500);
});
} else { // 200
$(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() :
```
$(function () {
$('#subForm').submit(function (e) {
e.preventDefault();
$.getJSON(
this.action + "?callback=?",
$(this).serialize(),
function (data) {
if (data.Status === 400) {
$('#slidemarginleft p').append("" + data.Message);
$("#slidemarginleft p").append('OK, I'll try again');
$(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});
});
//add click functionality for the button
$('#slidemarginleft button.wee').click(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});
setTimeout(function() {
$('#slidemarginleft p').empty();}, 500);
});
} else { // 200
$(function() {
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() :
Solution
This is typical jQuery mess. If you write a lot of code like this every day, consider switching to frameworks like Backbone or Ember.js which will make your life easier. Concerning this code, here are my remarks:
-
Move this code into its own function:
- Care more about indentation! All those callbacks are hard enough to read as they are.
- Only put
e.preventDefault();at the end. If there's a bug in the JS code, the user will be able to use the fallback, that is submitting the form without JS.
- Don't build html elements with string concatenations, but write things like
$('').addClass('css3button')and so on.
-
Move this code into its own function:
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});- More generally try to separate logic functions from display functions.
Code Snippets
var $marginLefty = $('.inner');
$marginLefty.animate({
marginLeft: parseInt($marginLefty.css('marginLeft'),10) == 0 ?
$marginLefty.outerWidth() : 0
});Context
StackExchange Code Review Q#22879, answer score: 2
Revisions (0)
No revisions yet.