Dynamically-loading interactive table

I've got a page on a website that shows a table, and upon clicking on a row in the table, it can dynamically load in more results. I am new to jQuery though.

index.php page:

```


Knife kills


$(document).ready(function() {
$("#list tr").bind("click", function(event) {
//Get clicked row
var clickedRow = $("#list tr#" + this.id);

//Increase clicks
if (!$(clickedRow).data("clicks")) {
$(clickedRow).data("clicks", 0);
}
$(clickedRow).data("clicks", $(clickedRow).data("clicks") + 1);

//Check if detailed information has been retrieved
if ($(clickedRow).data("detailed")) {
if ($(clickedRow).data("clicks") % 2 == 0) {
//Hide
$("#list tr#" + this.id + "row").remove();
}
else {
//Show
$($(clickedRow).data("detailed")).insertAfter($(clickedRow));
}
}
else {
var loading = $("Loading...");
loading.insertAfter($(clickedRow));
$.ajax({
url: "ajax_detailed.php",
data: {
id: this.id
},
success: function(html) {
if (html) {
//Success
$(loading).remove();
$(html).insertAfter($(clickedRow));
$(clickedRow).data("detailed", html);
}
else {

Some basic things (looking only the at the JS)

• Don't keep repeating $(clickedRow) - clickedRow is already a jQuery object. There's no need to use $() again.

• Avoid manually tracking a number to figure out whether to hide or show the details. Easier to check if the details are present or not and proceed accordingly.

Functionality-wise, there are a lot of ways to do what you need. I'd suggest breaking things out into functions, and a lot of other stuff.

But I don't want to go too far from your current code, so here's a simple, "light" refactoring (and here's a demo)

$(function() { // same as $(document).ready()
  $("#list tr").on("click", function (event) { // use .on() instead of .bind() (since jQuery 1.7)
    // Get relevant rows, the simpler way
    var xhr,
        id = this.id,
        clickedRow = $(this),
        loadingRow = clickedRow.next(".loading"),
        detailsRow = clickedRow.next("#" + id + "row");

    if(loadingRow.length > 0) {
      return; // stop here if we're currently loading
    }

    // Hide details and stop, if the details are shown
    if(detailsRow.length > 0) {
      detailsRow.remove();
      return;
    }

    // insert cached details and stop, if possible
    if(clickedRow.data("details")) {
      clickedRow.data("details").insertAfter(clickedRow);
      return;
    }

    // if we get this far, we'll need to load the details, so:

    // show the loading message
    loadingRow = $('Loading...').insertAfter(clickedRow);

    // fetch details (using the deferred/promise API)
    xhr = $.ajax({
      url: "ajax_detailed.php",
      data: { id: id },
      cache: true
    });

    // add details when they've loaded
    xhr.done(function (html) {
      // I'm skipping the if(html) check, since the server should
      // return an error code if there's an error. And if so, this
      // done-handler will never be called anyway
      detailsRow = $(html).insertAfter(clickedRow);
      clickedRow.data("details", detailsRow);
    });

    // remove the loading row regardless of how the ajax request went
    xhr.always(function (html) {
      loadingRow.remove();
    });
  });
});

The PHP also looks iffy to me, but it's been a long time since I had to work with PHP, so I won't comment.

I will say though, that you're extremely vulnerable to SQL injection attacks. For instance, if I send an id value of 0'; DROP DATABASE bf4; DROP USER bf4; to your ajax_detailed.php script, it'll permanently delete your entire database and remove the database user. You may want to look into that...

Ignore the above - I had overlooked the call to mysql_real_escape_string

StackExchange Code Review Q#35536, answer score: 2