Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

Sign up
Here's how it works:
  1. Anybody can ask a question
  2. Anybody can answer
  3. The best answers are voted up and rise to the top

I have created a function using jQuery Ajax which create a new CSS element whenever I update the database using a form.

Is it ok? If not, hat can be the best method to do it?

$(document).ready(function(){
    $('#statusupdater').submit(function(e) {
        updatestatus();
        e.preventDefault();
    });
});

function updatestatus()
{
$.ajax({
    type: "POST",
    url: "statusupdate.php",
    data: $('#statusupdater').serialize(),
    dataType: "dataString"          
});
$('#statuscontainer').prepend( $('<div id="stat">').load('statusupdate.php'));
$("#statusupdater")[0].reset();
    $('.statuspop').fadeOut(300);
            $('#opacify').hide();
}

statusupdate.php

<?php include "dbcon.php"; 

 $status=$_POST['status'];
 $photo=$_POST['photoupload'];

 if ($status || $photo)
 {
    $date = date('m/d/Y h:i:s');
    $statusinsert = mysqli_query($con,"INSERT INTO status (st_id,statu,date_created,userid) VALUES ('','$status','$date','$id')" );
 }

 $selectstatus = mysqli_query($con,"select * from status where userid = '".$id."' order by st_id desc limit 1");
 $getstatus=mysqli_fetch_array($selectstatus);
 ?>

    <div class="statbar"><?php echo $getstatus["userid"];?></div>
    <div class="stattxt"><?php echo $getstatus["statu"];?></div>
share|improve this question
up vote 3 down vote accepted

Set the return datatype to html and update the DOM in the success callback:

function updatestatus()
{
    $.ajax({
        type: "POST",
        url: "statusupdate.php",
        data: $('#statusupdater').serialize(),
        dataType: "html",
        success: function(data, status, xhrObj){
            $('#statuscontainer').prepend($('<div id="stat">').html(data));
            $("#statusupdater")[0].reset();
            $('.statuspop').fadeOut(300);
            $('#opacify').hide();
        }
    });
}

You could also update the DOM using the error and statusCode callbacks if it fails.

share|improve this answer

There really isn't much here and Ben got the big one. In addition to using the success callback, you could pass the form into your updatestatus() function. This will allow you to reuse this code, if necessary, and forces you to use a referenced pointer to the jQuery object rather than forcing jQuery to rebuild it every time you need to do something with it. Additionally, if you use the form's action and method attributes you wont have to worry about changing the url and type indices on multiple pages.

updatestatus( $( this ) );

function updatestatus( $form ) {
    $.ajax({
        type: $form.attr( 'method' ),
        url: $form.attr( 'action' ),
        data: $form.serialize(),
        dataType: "html",
        success: function( data ){
            $('#statuscontainer').prepend($('<div id="stat">').html(data));
            $form[0].reset();
            $('.statuspop').fadeOut(300);
            $('#opacify').hide();
        }
    });
}

Finally, you should really sanitize user input instead of using it directly.

$status = filter_input( INPUT_POST, 'status', FILTER_SANITIZE_STRING );

The only other thing I've really noticed is some minor inconsistencies in styling.

share|improve this answer

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.