patternjavascriptMinor
JavaScript file for a CMS application
Viewed 0 times
filecmsapplicationjavascriptfor
Problem
Be as harsh as you can! I'm trying to improve my coding style and would like to know if there is anything I could improve.
EDIT: To make this fair, The person who provides the most helpful tips (both in quantity and quality) will get marked as the answer
The code below is the JavaScript file for a CMS application I'm creating:
When the application is complete it will be Open Source for anyone to use.. I'll keep you guys updated if you're interested in trying it out
```
$(function () {
loadPage(htmlLandingPage, currentSiteName) //added on by previous page
$("#dashBody").css('top', $("#topDash").outerHeight());
$("#dashBody").hide()
$('#dashBody').load(function () {
iFrameContents.find("head").append($("",
{
rel: "stylesheet",
href: "/cmsCSS/loadedPageCSS.css"
}
));
$(window).on("scroll", function () {
iFrameContents.find("HTML,body").scrollTop($(window).scrollTop());
});
iFrameContents.on("click", ".cmsEdit", function (e) {
e.stopPropagation();
selectedElem = $(this)
if (e.ctrlKey) {
e.preventDefault();
if (!$(e.target).is('.inEdit *') && !$(e.target).hasClass('inEdit')) {
clearEditSection()
}
if (!$(this).hasClass("inEdit")) {
$.when(checkOutStatus(selectedElem.prop("id"))).done(function (result) {
if (!result.d) {
checkOutSection(selectedElem.prop("id"))
selectedElem.addClass("inEdit")
selectedElem.prepend($(""));
for (i = 0; i ", {
html: editBoxButtons[i],
addClass: "editButtons",
click: function () { editButtonAction(this, selectedElem) }
EDIT: To make this fair, The person who provides the most helpful tips (both in quantity and quality) will get marked as the answer
The code below is the JavaScript file for a CMS application I'm creating:
When the application is complete it will be Open Source for anyone to use.. I'll keep you guys updated if you're interested in trying it out
```
$(function () {
loadPage(htmlLandingPage, currentSiteName) //added on by previous page
$("#dashBody").css('top', $("#topDash").outerHeight());
$("#dashBody").hide()
$('#dashBody').load(function () {
iFrameContents.find("head").append($("",
{
rel: "stylesheet",
href: "/cmsCSS/loadedPageCSS.css"
}
));
$(window).on("scroll", function () {
iFrameContents.find("HTML,body").scrollTop($(window).scrollTop());
});
iFrameContents.on("click", ".cmsEdit", function (e) {
e.stopPropagation();
selectedElem = $(this)
if (e.ctrlKey) {
e.preventDefault();
if (!$(e.target).is('.inEdit *') && !$(e.target).hasClass('inEdit')) {
clearEditSection()
}
if (!$(this).hasClass("inEdit")) {
$.when(checkOutStatus(selectedElem.prop("id"))).done(function (result) {
if (!result.d) {
checkOutSection(selectedElem.prop("id"))
selectedElem.addClass("inEdit")
selectedElem.prepend($(""));
for (i = 0; i ", {
html: editBoxButtons[i],
addClass: "editButtons",
click: function () { editButtonAction(this, selectedElem) }
Solution
I'd cache the following jQuery objects...
In function
Also, a lot of the functions that perform ajax calls can be merged into 1 function, and you also don't need to do as much work with the data object...
There's other places where you duplicate DOM lookups so just be aware that repeated jQuery selections can drastically slow the page, depending on the size of the DOM and what browser you are using.
One other thing that I'd strongly advise is that you get into the habit of ending every single "line" of code with a semi-colon. Most of the time you'll get away with it, but there are instances where it can make your code mean something different. Terminate your statements correctly and you won't go wrong :)
var $dashBody = $("#dashBody");
var $topDash = $("#topDash");
var $loadProgress = $(".loadProgress");
var $loadingText = $(".loadingText");In function
clearEditSection() you do a second search for elements that can be cut down to 1...iFrameContents.find('.inEdit')
.prop('contenteditable', false)
.removeClass("inEdit");Also, a lot of the functions that perform ajax calls can be merged into 1 function, and you also don't need to do as much work with the data object...
function ajaxPost(sectionID, url, beforeText, errorText) {
$.ajax({
type: "POST",
url: url
data: {
sectionID: sectionID,
personID: personID,
sitePageID: sitePageID
},
contentType: "application/json; charset=utf-8",
beforeSend: function () {
$(".loadProgress").fadeIn();
$(".loadingText").html(beforeText)
},
dataType: "json",
success: function (data) {
$(".loadProgress").fadeOut()
},
error: function (request, status, errorThrown) {
$(".loadingText").html(errorText)
return false
}
});
}function checkOutStatus(sectionID) could then be called like this...ajaxPost(sectionID, "default.aspx/checkOutStatus",
"Verifying Check Out Status",
"Verifying Check OutError");There's other places where you duplicate DOM lookups so just be aware that repeated jQuery selections can drastically slow the page, depending on the size of the DOM and what browser you are using.
One other thing that I'd strongly advise is that you get into the habit of ending every single "line" of code with a semi-colon. Most of the time you'll get away with it, but there are instances where it can make your code mean something different. Terminate your statements correctly and you won't go wrong :)
Code Snippets
var $dashBody = $("#dashBody");
var $topDash = $("#topDash");
var $loadProgress = $(".loadProgress");
var $loadingText = $(".loadingText");iFrameContents.find('.inEdit')
.prop('contenteditable', false)
.removeClass("inEdit");function ajaxPost(sectionID, url, beforeText, errorText) {
$.ajax({
type: "POST",
url: url
data: {
sectionID: sectionID,
personID: personID,
sitePageID: sitePageID
},
contentType: "application/json; charset=utf-8",
beforeSend: function () {
$(".loadProgress").fadeIn();
$(".loadingText").html(beforeText)
},
dataType: "json",
success: function (data) {
$(".loadProgress").fadeOut()
},
error: function (request, status, errorThrown) {
$(".loadingText").html(errorText)
return false
}
});
}ajaxPost(sectionID, "default.aspx/checkOutStatus",
"Verifying <br/> Check Out<br/> Status",
"Verifying <br/> Check Out<br/>Error");Context
StackExchange Code Review Q#32184, answer score: 4
Revisions (0)
No revisions yet.