Skip to content

Commit

Permalink
ajax_demo: Added jquery dialog boxes for displaying messages
Browse files Browse the repository at this point in the history
  • Loading branch information
sdaityari committed Jun 11, 2013
1 parent a067a4b commit ba08935
Showing 1 changed file with 60 additions and 16 deletions.
76 changes: 60 additions & 16 deletions jscripts/ajax/FileStorage.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,7 @@ ATutor.fileStorage = ATutor.fileStorage || {};

//Function to be called on clicking Delete for a comment
fileStorage.deleteComment = function (ot, oid, file_id, id) {
//Asks for confirmation from user, returns if user cancels
if (!confirm (deleteMessage)) {
return;
}


//Sets POST variables to be sent
var parameters = {
"ot" : ot,
Expand All @@ -31,26 +27,74 @@ ATutor.fileStorage = ATutor.fileStorage || {};
"submit_yes": true
};

//AJAX Request sent to required URL
$.ajax({
type: "POST",
url: deleteUrl,
data: parameters,
success: function(message) {
commentOnDelete(message, parameters);
}

var buttonOptions = {
"Yes": function (){
$.ajax({
type: "POST",
url: deleteUrl,
data: parameters,
success: function(message) {
commentOnDelete(message, parameters);
}
});
$(this).dialog("close");

This comment has been minimized.

Copy link
@anvk

anvk Jun 11, 2013

Ok here is a major issue with the code. Whenever I will see "this" in your code most likely I will be VERY grumpy.
Please Google online about common pitfalls with keyword "this". It is a huge pitfall in the world of webdev.
Here is a good link to look at for starters http://stackoverflow.com/questions/3127429/javascript-this-keyword
Your code here is the case #3. You will execute close() on the window object !!

},
"No" : function () {
$(this).dialog("close");
}
};

// Create dialog for the page if it doesn't exist
if ($("#comment-delete-dialog").length === 0){
$("body").append("<div title='" + deleteTitle + "' id='comment-delete-dialog'>" +
deleteMessage +"</div>");
}

$("#comment-delete-dialog").dialog({
autoOpen: true,
width: 400,
modal: true,
closeOnEscape: false,
buttons: buttonOptions
});

};

//Callback function for AJAX Request
var commentOnDelete = function (responseMessage, parameters) {
var ajaxResponse = "Ajax Response",
accessDeniedMessage = "Access Denied",
unknownErrorMessage = "Unknown Error Occurred";

// Create dialog for the page if it doesn't exist
if ($("#ajax-response-dialog").length === 0){

This comment has been minimized.

Copy link
@anvk

anvk Jun 11, 2013

do not hardcode #ajax-response-dialog in your code.

$("body").append("<div title='" + ajaxResponse + "' id='ajax-response-dialog'></div>");
}

if (responseMessage === "ACTION_COMPLETED_SUCCESSFULLY") {
$("#comment" + parameters.id).fadeOut();
return;
} else if (responseMessage === "ACCESS_DENIED") {
alert ("Access denied");
$("#ajax-response-dialog").html(accessDeniedMessage);

This comment has been minimized.

Copy link
@anvk

anvk Jun 11, 2013

Every time you call $("#ajax-response-dialog"), your code will trigger jQuery function which would find this element in a whole DOM. Get $("#ajax-response-dialog") only once and assign it to a variable and then use this variable to call html(). It is one of the major issue with current web-dev. Lots of developers call $('selector') constantly in their code which in the end triggers constant lookups in a DOM which is a very expensive task.

} else {
alert ("Action not completed. Unknown Error Occurred.");
$("#ajax-response-dialog").html(unknownErrorMessage);
}

//Set an Ok button for the dialog box to be shown in case the comment was not deleted
var buttonOptions = {
"Ok" : function () {
$(this).dialog("close");
}
};

$("#ajax-response-dialog").dialog({
autoOpen: true,
width: 400,
modal: true,
closeOnEscape: false,
buttons: buttonOptions
});
};

})(ATutor.fileStorage);

0 comments on commit ba08935

Please sign in to comment.