Javascript callback function issue.

Question

Background

I'm writing an asynchronous comment system for my website, after reading plenty of tutorials on how to accomplish this I started building one from scratch. The comments are pulled using a JSON request and displayed using Javascript (jQuery). When the user adds a new comment it goes through the hoops and finally is sent via AJAX to the backend where it's added to the database. In the success section of the AJAX request I had the script empty the comments, then repull the new list (including the new post) and redisplay them.

Problem

While that was all nice, since it's making the page much shorter, then much longer it messes up where the user is viewing the page. I wanted to have it readjust the page back down to the end of the comment list (where the add comment form is). It also re-enables the add button, which was disabled when the clicked it to prevent impatient people from spamming.

$('#commentList').empty();
getComments('blog', $('input#blogId').val());
window.location = "#addComment";
$('#comAdd').removeAttr('disabled');

While this worked all well and good in theory, it seemed that the browser was getting ahead of itself and processing the window.location before the getComments function was done. So I read a little more and googled it and it seemed people were saying (for similar problems) to use callback functions, so I came up with this:

$('#commentList').empty();
getComments('blog', $('input#blogId').val(), function() {
    window.location = "#addComment";
    $('#comAdd').removeAttr('disabled');
});

This generates no javascript errors according to FireFox, but nothing within the callback function is working, it's not re-enabling the button nor changing the window.location.

Any ideas? Better ways to go about it? Do I have a glaring typo that I can't seem to see?

Thanks!

Update

I was under the impression the callback functions were a standard thing you could use.

function getComments(type, id)
{

    $.getJSON("/ajax/"+type+"/comments?jsoncallback=&id="+id, function(data) {
     for (var x = 0; x < data.length; x++)
     {
      var div = $("<div>").addClass("comment").appendTo("#commentList");
      var fieldset = $("<fieldset>");
      var legend = $("<legend>").addClass("commentHeader");
      if ( data[x].url == "" )
      {
       legend.text((x+1) + ' - ' + data[x].name);
      }
      else
      {
       $("<a>").attr({href: data[x].url}).text((x+1) + ' - ' + data[x].name).appendTo(legend);
      }
      legend.appendTo(fieldset);
      $("<div>").addClass("date").text(data[x].timestamp).appendTo(fieldset);
      $("<p>").addClass("comment").text(data[x].content).appendTo(fieldset);
      fieldset.appendTo(div);
     }
    });

}

This is called on document ready. Pulling all the comments and displaying them inside the #commentList div. When the user submits his/her comment it performs an AJAX request to a PHP script that adds the new comment to the database, upon success of this I have this:

$('#commentList').empty();
getComments('blog', $('input#blogId').val());
window.location = "#addComment";
$('#comAdd').removeAttr('disabled');

Deletes all the comments from the page. Uses JSON to request the comments again (including the users new one). Moves the page to the #addComment anchor, which is where their new comment would be displayed. Re-enables the add comment button.

The problem is that the browser does the window.location line before the getComments function is done rendering all the comments, so as the page grows the user isn't looking anywhere near their new comment.

Answer 1

I expect here the problem is your getComments() function (for which more detail is required). You're supplying a third argument being a callback but does the function actually use a callback? What is it doing?

Certain jQuery functions provide callbacks but this isn't an automatic feature. If you're waiting for a user to type a comment you need to trigger the relevant event when they click "Done" or whatever they do.

Ok, try this:

function get_comments(type, id, callback) {
  $.getJSON("/ajax/"+type+"/comments?jsoncallback=&id="+id, function(data) {
    for (var x = 0; x < data.length; x++) {
      var div = $("<div>").addClass("comment").appendTo("#commentList");
      var fieldset = $("<fieldset>");
      var legend = $("<legend>").addClass("commentHeader");
      if ( data[x].url == "" ) {
        legend.text((x+1) + ' - ' + data[x].name);
      } else {
        $("<a>").attr({href: data[x].url}).text((x+1) + ' - ' + data[x].name).appendTo(legend);
      }
      legend.appendTo(fieldset);
      $("<div>").addClass("date").text(data[x].timestamp).appendTo(fieldset);
      $("<p>").addClass("comment").text(data[x].content).appendTo(fieldset);
      fieldset.appendTo(div);
      if (typeof callback != 'undefined') {
        callback();
      }
    }
  });
}

Note: the difference here is that a third argument is supplied to get_comments() which is a callback that'll be called at the end of your $.getJSON() callback. That'll give you the proper ordering you want.

I might also suggest not constructing the HTML like that but including it in your page and hiding/unhiding it as necessary. It tends to be much more performant that dynamic HTML and have less issues (eg new HTML, unless you use $().live() will not have relevant event handlers).

Edit: Made the callback optional as per the comments. With the above code you can call the function without or without the callback.

Answer 2

Simple. Re-enable the button and go to the anchor after you receive the request and process the information. Like so:

function getComments(type, id)
{
    // ADDED
    $('#commentList').empty();

    $.getJSON("/ajax/"+type+"/comments?jsoncallback=&id="+id, function(data) {
        for (var x = 0; x < data.length; x++)
        {
                var div = $("<div>").addClass("comment").appendTo("#commentList");
                var fieldset = $("<fieldset>");
                var legend = $("<legend>").addClass("commentHeader");
                if ( data[x].url == "" )
                {
                        legend.text((x+1) + ' - ' + data[x].name);
                }
                else
                {
                        $("<a>").attr({href: data[x].url}).text((x+1) + ' - ' + data[x].name).appendTo(legend);
                }
                legend.appendTo(fieldset);
                    $("<div>").addClass("date").text(data[x].timestamp).appendTo(fieldset);
                    $("<p>").addClass("comment").text(data[x].content).appendTo(fieldset);
                fieldset.appendTo(div);
        }

        // ADDED
        window.location = "#addComment";
        $('#comAdd').removeAttr('disabled');
    });
}

Personal opinion: rather than fetching all comments, why not fetch comments from a certain date? When you load the page, include a server time in the response. The Javascript uses this to query behind the scenes (to automatically check for new comments). The JSON response includes a new server time, which is used in the next response.

How would you handle deleted comments? Easy: have a deleted_on column in your database table, query it, and spit that out in the JSON response along with new posts.

Suggestion: instead of #addcomment, ID comments by timestamp.