Return to Revisions

First of, for someone learning jQuery and JavaScript, you've done a great job of avoiding the pitfalls of most new developers to JavaScript;

1. You're not passing strings to setTimeout, which a lot of people do.
2. You've got a good grasp on closures (no matter how long they took you to learn :)).
3. You're using strict equals (===) rather than equals.

... so, if you want me to be really, really picky;

1. I don't see much point in adding your code to the jQuery namespace. It would work just as well to be added to your own namespace. A lot of people fall into the habit of defining everything on $, and are scared of declaring your own namespace; don;t be:

   var ME = {};
   
   ME.notify = function (title, msg, duration) {
       return new notification(title, msg, duration);
   };
   

2. It's a code convention to use a capital letter for constructors (e.g. functions you need to call new on); change function notification to function Notification.

3. You could make use of prototypical inheritance. As it stands, you're defining and adding a hide function on every instance of a notification you create. Obviously this has a negligible memory impact. Instead, use prototypical inheritance, and declare the function once;

   function Notification (blah, blah, blah) {
      // blah blah blah
   } 
   
   Notification.prototype.hide = function () {
       var self = this;
   
       this.element.fadeOut(1000, function () {
           self.element.remove();
       });
   }
   

... you'd then have to change your double click handler to use the anonymous function approach I recommended in my answer, as this will no longer be the correct this.

3. You've got a potential XSS exploit. For example, given the message:

   <script>alert(document.cookie);</script>
   

This will be injected, and evaluated in your page. You can fix this by setting the h3 and p using text() explicitly;

    this.element = $('<div class="notification"><h3></h3><div class="notification_body"><span class="text"></span><br/><i>(double click to hide)</i></div></div>');

    this.element.find('h3').text(title);
    this.element.find('span.text').text(message);

... but again, I want to re-iterate that I'm being very, very picky. What you've got is well written, functioning JavaScript.