B6c85faacb1a59a39f31f8820dcbd478

Using the jquery.form.plugin to simplify things but I'm really frustrated by how I have done this. It sucks. As you can see the functions are practically identical and I didn't want to make just one and clutter it with lots of conditionals because I might have to add more forms which function similarly. Could I perhaps extend the plugin?

// New function for enviar.
$(document).ready(function() { 
	var options = { 
			dataType: 'jsonp',		
			jsonp: 'jsoncallback',
			success: getResponse
		};
	
	$("#frmEnviar").ajaxForm(options);
	
	function getResponse(responseText, statusText){
		
		if(responseText.status === false){
    		var box = $('#enviarError');
		}
    	else{
    		var box = $('#enviarSuccess');
    	};   	
		
		for (message in responseText.messages){ 
			var messages = '';
			messages += '<p>'+responseText.messages[message]+'</p>';
		};
    	
		// Ensure boxes are hidden.
		$('#enviar .msg').hide();
		
		// Send messages to box.
    	if(messages){
    		box.html(messages);
    	};
    	
    	box.show();
    	
    	if(!messages){
    		$(".util-popup").fadeOut(3000);
    	};
	};
});

// New function for leer.
$(document).ready(function() { 
	var options = { 
		dataType: 'jsonp',
		jsonp: 'jsoncallback',
		success: getResponse
	};		
		
    $("#frmLeer").ajaxForm(options);
    
    function getResponse(responseText, statusText){
		var messages = '';
    	var error = false;
    	
    	if(responseText.status === false){
    		var box = $('#leerError');
    		error = true;
		}
    	else{
    		var box = $('#leerSuccess');
    	};   	
		
		for (message in responseText.messages){ 
			messages += responseText.messages[message];
		};
    	
		// Ensure boxes are hidden.
		$('#leer-mas-tarde .msg').hide();
		
    	// Send messages to box.
    	box.html(messages);
    	box.show();
    	
    	// Fade out.
    	if(error === false){
    		$("frmLeer").clearForm();
    		$(".util-popup").fadeOut(3000);
    	};
    };    
});

Refactorings

No refactoring yet !

D41d8cd98f00b204e9800998ecf8427e

V

February 18, 2009, February 18, 2009 16:26, permalink

No rating. Login to rate!

Not a refactor, but a possible error, on line 20, didn't you mean:

var messages = '';
	for (message in responseText.messages){ 
		messages += '<p>'+responseText.messages[message]+'</p>';
	};
B6c85faacb1a59a39f31f8820dcbd478

crungmungus.myopenid.com

February 18, 2009, February 18, 2009 16:32, permalink

No rating. Login to rate!

Well spotted! thank you very much!

Aacfa176a8d73ca75b90b6375151765a

Paul Wilkins

February 23, 2009, February 23, 2009 07:39, permalink

No rating. Login to rate!

As the error variable is just carrying on through the value of responseText.status, that error variable has been removed and in the one place that it was used, responseText.status has been used instead. This helps to tidy up the box variable section, so that it can be used by both of the forms.

Now all that remains is to define behaviour based on the expected name of the form, and carry that form name on through to the getResponse function, which can be achieved by using a wrapper so that the name is retained within the closure of the wrapper itself.

Can things be taken further from here?

$(document).ready(function () {
	['enviar', 'leer'].each(function () {
		initForm(this);
	});
});
function initForm(name) { 
	var options = { 
		dataType: 'jsonp',		
		jsonp: 'jsoncallback',
		success: getResponseWrapper(name)
	};
	$('#frm' + titleCase(name)).ajaxForm(options);
	function getResponseWrapper(formName) {
		return function (responseText, statusText) {
			getResponse(responseText, statusText);
		};
	}
	function getResponse(responseText, statusText) {
		var messages = '';
		if (responseText.status === false) {
			var box = $('#' + formName + 'Error');
		} else {
			var box = $('#' + formName + 'Success');
		};   	
		for (var message in responseText.messages) { 
			messages += '<p>' + responseText.messages[message] + '</p>';
		};
		// Ensure boxes are hidden.
		var boxContainer = formName;
		if (formName === 'leer') {
			boxContainer += '-mas-tarde';
		}
		$('#' + boxContainer + ' .msg').hide();
		if (formName === 'envier') {
			if (messages) {
				box.html(messages);
			} else {
				$(".util-popup").fadeOut(3000);
			}
		}
		if (formName === 'leer') {
			// Send messages to box.
			box.html(messages);
			if (responseText.status === true) { // no error
				$('frm' + titleCase(formName)).clearForm();
				$(".util-popup").fadeOut(3000);
			}
		}
		box.show();
	};
}
function titleCase(word) {
	word.substring(0, 1).toUpperCase() + word.substring(1);
}
Aacfa176a8d73ca75b90b6375151765a

paul.wilkins.myopenid.com

February 23, 2009, February 23, 2009 07:41, permalink

No rating. Login to rate!

Sorry, my apologies. let me try again now that I'm logged in.

As the error variable is just carrying on through the value of responseText.status, that error variable has been removed and in the one place that it was used, responseText.status has been used instead. This helps to tidy up the box variable section, so that it can be used by both of the forms.

Now all that remains is to define behaviour based on the expected name of the form, and carry that form name on through to the getResponse function, which can be achieved by using a wrapper so that the name is retained within the closure of the wrapper itself.

Can things be taken further from here?

$(document).ready(function () {
    ['enviar', 'leer'].each(function () {
        initForm(this);
    });
});
function initForm(name) { 
    var options = { 
        dataType: 'jsonp',        
        jsonp: 'jsoncallback',
        success: getResponseWrapper(name)
    };
    $('#frm' + titleCase(name)).ajaxForm(options);
    function getResponseWrapper(formName) {
        return function (responseText, statusText) {
            getResponse(responseText, statusText);
        };
    }
    function getResponse(responseText, statusText) {
        var messages = '';
        if (responseText.status === false) {
            var box = $('#' + formName + 'Error');
        } else {
            var box = $('#' + formName + 'Success');
        };       
        for (var message in responseText.messages) { 
            messages += '<p>' + responseText.messages[message] + '</p>';
        };
        // Ensure boxes are hidden.
        var boxContainer = formName;
        if (formName === 'leer') {
            boxContainer += '-mas-tarde';
        }
        $('#' + boxContainer + ' .msg').hide();
        if (formName === 'envier') {
            if (messages) {
                box.html(messages);
            } else {
                $(".util-popup").fadeOut(3000);
            }
        }
        if (formName === 'leer') {
            // Send messages to box.
            box.html(messages);
            if (responseText.status === true) { // no error
                $('frm' + titleCase(formName)).clearForm();
                $(".util-popup").fadeOut(3000);
            }
        }
        box.show();
    };
}
function titleCase(word) {
    return word.substring(0, 1).toUpperCase() + word.substring(1);
}
F1e3ab214a976a39cfd713bc93deb10f

Tj Holowaychuk

February 27, 2009, February 27, 2009 00:46, permalink

No rating. Login to rate!

This will most likely not work because I have not tried it, and I do not know what it is supposed to do really ( didnt really read it much ), but you can change the styling of your code vastly.. and probably should. I would also not really do conditional work on specific forms a generalized function like that, maybe abstract out the functionality. I would probably throw a switch in there as well for the form names if you are doing that

// ; makes sure the last script wont cause issues
// (function($){})(jQuery) anonymous function calling itself, mapping jQuery to $ (good practice)
;(function($){
  
  function titleCase(word) {
    return word.substring(0, 1).toUpperCase() + word.substring(1)
  }
  
  function getResponse(name, response, status) {
    var messages = '', box, id = name
    
    if (!response.status) box = $('#' + name + 'Error')
    else box = $('#' + name + 'Success')
      
    for (var message in response.messages)
      messages += '<p>' + response.messages[message] + '</p>'
    
    if (name == 'leer') id += '-mas-tarde'

    $('#' + id + ' .msg').hide()
    
    if (name === 'envier') {
      if (messages) box.html(messages)
      else $(".util-popup").fadeOut(3000)
    }
    
    if (formName === 'leer') {
      box.html(messages)
      if (responseText.status) { 
        $('frm' + titleCase(formName)).clearForm()
        $(".util-popup").fadeOut(3000)
      }
    }
    
    box.show()
  }
  
  function initForm(name) { 
    $('#frm' + titleCase(name)).ajaxForm({ 
      dataType: 'jsonp',        
      jsonp: 'jsoncallback',
      success: function (response, status) { getResponse(name, response, status) }
    })
  }
  
  // ^^ all those functions are scoped to this anonymous function, so name them as concise as you want 
  
  // Shorthand for $(document).ready(...)
  $(function() {
    initForm('enviar')
    initForm('leer')
  });
  
})(jQuery)

// if you call initForm (or the others) out here they will be undefined, another
// reason why you should wrap with anonymous functions, if you want to expose it use
// this.initForm = initForm
F6768a4f29bf0262b847cc99d4db184a

Alan Cuevas

March 3, 2010, March 03, 2010 14:34, permalink

No rating. Login to rate!

I NEED HELP PLEASE... IM USING AJAXform and i need send a javascript variable but i don´t know how i can do that! this is the code:

obs: I need send the form and the array "series"

//en series[] se almacena cada fila de la tabla 
	   	for(var i=0;i<numFilas;i++){
			var fila=filas[i];
			var columnas=fila.cells;
			var id_aux=0;
			series[i]=columnas[0].innerHTML+"-"+ columnas[1].innerHTML; 					
		}
		
	    var opciones = { 
	    	    		target: '#resp',
	    	    		//clearForm: true,
	    	    		resetForm:true,
	    	    		data:"series="+series
	    	    		
	    	    		} 
	    
	    	$('#form').ajaxForm(opciones) ; 
	    	//var queryString = $('#form').formSerialize();
    		//queryString=queryString +"&series="+series;
	        //$(this).ajaxSubmit(options); 
	        //$.post('respuesta.php', queryString);

Your refactoring





Format Copy from initial code

or Cancel