3
\$\begingroup\$

How can I shorten and optimize this code which displays then hides a DIV?

setTimeout(function () {
    $('.updateNotification').show().addClass('heightener');
    callback();
},2000)

function callback() {
  setTimeout(function() {
    $( '.updateNotification' ).fadeOut();
  }, 10000 );
};

My first thought was to combine the functions. However, I would like them to remain separate because at a later date I will additionally incorporate a button which will trigger callback() in addition to the timer.

\$\endgroup\$

3 Answers 3

6
\$\begingroup\$

I'm not sure how you would shorten this code. However I think that callback is a terrible name for a function that fades a CSS class. Remember that a function signature should represent what a function does and with that said I would rename it to something like fadeUpdateNotification().

And in the fashion of fading you could replace .show() with .fadeIn() to have the div fade in and out - I dunno how your styling/website looks like but that might look nice...

\$\endgroup\$
3
\$\begingroup\$

There's not much here to optimize mostly because there's very little here. A few things you could do:

Store your selection to a variable outside both functions (bad idea if you may have multiple notificationDivs).

var notificationDiv = $( '.updateNotification' )

You could save 1 character by adding the callback to the show animation (not really an improvement):

notificationDiv.show(0,callback)

I'm more confused about why you're doing the setTimeouts the way you are. Why do you need a 2 second delay to show the div? Also if you plan to call callback later, I think it'd confuse most people if the div takes 10 seconds to start to disappear.

Here's a fiddle I used while tinkering.

\$\endgroup\$
3
\$\begingroup\$

if you want to combine them you can just do it like this

$('.updateNotification').delay(2000).show(0).addClass('heightener').delay(10000).fadeOut();    

http://jsfiddle.net/4XYg8/

or to keep the callback function something like this

var notifications= $('.updateNotification'); //store the selection like Daniel Cook said
setTimeout(function () {
    //if you use the no-arguments form of .show()
    //you can just add display:block on the class .heightener
    notifications.addClass('heightener');
    callback();
},2000);

function callback() {
    notifications.delay(10000).fadeOut();//if you only want to fadeOut you can use delay
};    

http://jsfiddle.net/4XYg8/1/

\$\endgroup\$
1
  • \$\begingroup\$ I knew someone would know the "correct" way to combine it. +1 \$\endgroup\$
    – Daniel
    Dec 5, 2013 at 0:24

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

Not the answer you're looking for? Browse other questions tagged or ask your own question.