Using a popup to display messages to the user

Question

I am having a popup which is styled to match the page and am using that single popup to display messages to the user. My popup contains one h1 element and an ASP.NET Label.

The h1 element and Label contents and color change according to the operation: if the information was successfully inserted the h1 text becomes Success and Label's text becomes "Your information was saved".

On incorrect input it changes accordingly, and on some unexpected error it changes the same way.

Currently I am calling three different JavaScript functions on appropriate events from my code behind using RegisterClientScriptBlock.

HTML:

<div id="NewCustomerSubmitStatusPopUp">
 <a href="#" class="NewCustomerSubmitPopUpClose"><img class="NewCustomerSubmitPopUpImg" src="Images/cancel1.png"/></a>
 <h1></h1>
 <asp:Label ID="NewCustomerlblSubmitStatusMsg" runat="server" Text=""></asp:Label><br /><br />
 <input class="NewCustomercmdSubmitPopUp" type="button" value="Close" /> 
 </div>

CSS:

#NewCustomerSubmitStatusPopUp
 {
 display:none;
 position: fixed;
 width:500px;
 height: 150px;
 top: 50%;
 left: 50%;
 margin-left:-255px;
 margin-top:-110px;
 /*background-color:#F7DFDE; */
 padding:10px;
 z-index:102;
 font-family:Verdana;
 font-size:10pt;
 border-radius:10px;
 -webkit-border-radius:20px;
 -moz-border-radius:20px;
 font-weight:bold;
 }
 #NewCustomerSubmitStatusPopUp h1
 {
 /*color:#BD494A;*/
 padding:20px 0; 
 text-align:center; 
 }
 .NewCustomercmdSubmitPopUp
 {
 height:30px;
 width:50px;
 margin-left:220px;
 }
 .NewCustomerSubmitPopUpImg
 {
 float:right;
 margin-top:-30px;
 margin-right:-25px;
 }

javascript:

function ShowSubmitSuccessPopUp()
 {
 $('#NewCustomerMask').show("slow");
 $('#NewCustomerSubmitStatusPopUp').show("slow");
 $('#NewCustomerSubmitStatusPopUp').show("slow");
 $('#NewCustomerSubmitStatusPopUp').css({ background: 'white' });
 $('#NewCustomerSubmitStatusPopUp').css({ border: '10px solid #9cc3f7'});
 $('#NewCustomerSubmitStatusPopUp h1').html("Success");
 $('#NewCustomerSubmitStatusPopUp h1').css({ color: '#9cc3f7' });
 $('#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg').css({ color: '#9cc3f7' });
}
function ShowSubmitErrorPopUp()
{
 $('#NewCustomerMask').show("slow");
 $('#NewCustomerSubmitStatusPopUp').show("slow");
 $('#NewCustomerSubmitStatusPopUp').css({ background: '#F7DFDE' });
 $('#NewCustomerSubmitStatusPopUp').css({ border:'10px solid #BD494A'});
 $('#NewCustomerSubmitStatusPopUp h1').html("Error");
 $('#NewCustomerSubmitStatusPopUp h1').css({ color: '#BD494A' });
 $('#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg').html("There are some errors in the page.Please rectify the errors by visiting the Errors tab.");
 $('#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg').css({ color: '#BD494A' });
}
function ShowSubmitUnexpectedErrorPopUp()
{
 $('#NewCustomerMask').show("slow");
 $('#NewCustomerSubmitStatusPopUp').show("slow");
 $('#NewCustomerSubmitStatusPopUp').css({ background: '#F7DFDE' });
 $('#NewCustomerSubmitStatusPopUp').css({ border: '10px solid #BD494A' });
 $('#NewCustomerSubmitStatusPopUp h1').html("Something Went Wrong");
 $('#NewCustomerSubmitStatusPopUp h1').css({ color: '#BD494A' });
 $('#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg').html("There was some error while submitting your information.Please re-enter the values and try again.");
 $('#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg').css({ color: '#BD494A' });
}

As you can see, for every event, I change text text of the h1 and the label element and change the style too.

Can anybody tell me if I am doing it in a good manner or if there is something wrong?

• Florian Margaine: Next time, care about your grammar/spelling. It's ok not to speak english perfectly, but at least give some air to your text.
• Priyank Patel: @FlorianMargaine can you just point me what was not ok.
• Florian Margaine: Just look at the edit: codereview.stackexchange.com/posts/12378/revisions
• Priyank Patel: @FlorianMargaineya I checked that , I agreee it needs to be proper next time , I will take care of it.

Answer 1

It's pretty much commented in the code:

JavaScript:

$(function() {
 //cache repeatedly used elements to avoid re-querying them 
 var customermask = $('#NewCustomerMask'),
 popup = $('#NewCustomerSubmitStatusPopUp'),
 //use context to get descendants of a target context. it's similar to a .find()
 popupHeader = $('h1', popup),
 placeholder = $('#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg'),
 //move out your text as variables. don't mingle them with the code
 errorMsg = 'There are some errors in the page.Please rectify the errors by visiting the Errors tab.',
 unexpectedErrorMsg = 'There was some error while submitting your information.Please re-enter the values and try again.';
 //condense the function to one
 function ShowSubmitPopUp(status) {
 //and do common tasks
 customermask.show('slow');
 //use classes to determine the state and style of the pop-up 
 popup.show('slow').addClass(status);
 placeholder.addClass(status);
 //use the status to determine specific tasks
 switch (status) {
 case 'success':
 //use .text() when setting only text
 popupHeader.text('Success');
 break;
 case 'error':
 placeholder.text(errorMsg);
 popupHeader.text('Error');
 break;
 case 'uerror':
 placeholder.text(unexpectedErrorMsg);
 popupHeader.text('Something went awfully wrong!');
 break;
 }
 }
});​

CSS:

/*since styles are common, you can stack them to avoid repeating*/
/*success styles*/
#NewCustomerSubmitStatusPopUp.success{
 background: #FFF;
 border: 10px solid #9cc3f7;
}
#NewCustomerSubmitStatusPopUp.success h1,
#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg.success{
 color: #9cc3f7;
}
/*error and uerror styles*/
#NewCustomerSubmitStatusPopUp.error,
#NewCustomerSubmitStatusPopUp.uerror{
 background: #F7DFDE;
 border: 10px solid #BD494A;
}
#NewCustomerSubmitStatusPopUp.error h1,
#NewCustomerSubmitStatusPopUp.uerror h1,
#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg.error,
#ctl00_ContentPlaceHolder1_NewCustomerlblSubmitStatusMsg.uerror{
 color: #BD494A;
}​​

• Priyank Patel: I will give it a try and then come back to you , if I have any doubts.Thanks for the help.Really appreciate your effort.
• Priyank Patel: Another question ,I need to call ShowSubmitPopUp() function and pass appropriate class name as arguments right?
• Priyank Patel: Thanks a lot , it worked and the code is clean and less as well.Thanks.

Answer 2

A few ideas:

• Your three javascript functions could easily be combined into one with some simple arguments, like headerText, statusMessage, statusType.
• In your description you list some success text, but this isn't in the javascript, so I guess this is set in the code-behind. I consolidate how the text is set. If you choose to use javascript, you could simplify things by changing the asp:Label to a span.
• It might be nicer to put the style information in the CSS file and then in the javascript just toggle which classes apply.

Here's an example of how it might look:

CSS

#NewCustomerSubmitStatusPopup.successPopup
{
 background-color: white;
 border-color: #9cc3f7; /* width, pattern added to main CSS block */
}
#NewCustomerSubmitStatusPopup.successPopup h1, #NewCustomerSubmitStatusPopup.successPopup span
{
 color: #9cc3f7;
}
#NewCustomerSubmitStatusPopup.errorPopup
{
 background-color: #F7DFDE;
 border-color: #BD494A;
}
#NewCustomerSubmitStatusPopup.errorPopup h1, #NewCustomerSubmitStatusPopup.errorPopup span
{
 color: #BD494A;
}

javascript

function ShowSubmitPopUp(headingText, statusMessage, statusType)
{
 $('#NewCustomerMask').show("slow");
 $('#NewCustomerSubmitStatusPopUp').show("slow");
 var popupClass = (statusType == 'error' ? 'errorPopup' : 'successPopup');
 $('#NewCustomerSubmitStatusPopUp').attr('class', popupClass);
 $('#NewCustomerSubmitStatusPopUp h1').html(headingText);
 $('#NewCustomerSubmitStatusPopUp span').html(statusMessage);
}

Answer 3

Additionally to @CodeMonkey1 answer you can cache your jQuery objects for ex. $('#NewCustomerSubmitStatusPopUp') to a variable like this: var myObj = $('#NewCustomerSubmitStatusPopUp');

Then you can rewrite this code:

$('#NewCustomerSubmitStatusPopUp').show("slow");
$('#NewCustomerSubmitStatusPopUp').css({ background: '#F7DFDE' });
$('#NewCustomerSubmitStatusPopUp').css({ border: '10px solid #BD494A' });
$('#NewCustomerSubmitStatusPopUp h1').html("Something Went Wrong");
$('#NewCustomerSubmitStatusPopUp h1').css({ color: '#BD494A' });

to this:

myObj.show("slow");
myObj.css({ background: '#F7DFDE' });
myObj.css({ border: '10px solid #BD494A' });
myObj.find("h1").html("Something Went Wrong");
myObj.find("h1").css({ color: '#BD494A' });

This has the benefit that you dont have to query the DOM five times for the same thing.

• Priyank Patel: thanks a lot for correcting my mistake , really helpful.Thanks.
• Robert Niestroj: You made no mistake - you're just learning and that is very good.
• Priyank Patel: You are right,getting feedbacks from better people is a blessing.Thanks mate.