views:

792

answers:

2

I'm new to building jQuery plugins. I have seen and used a lot of tooltip plugins, and today I've decided to build my own.

Can I get some feedback on the code? What work, what doesn't. Optimizations. Caching.
What can I do to make it faster and better?

This would be really helpful for my learning and hopefully for others too.

Heres my plugin:

;(function($) {
 $.fn.jTooltip = function(options) {

  opts = $.extend({}, $.fn.jTooltip.defaults, options);

  return this.each(function() {

   var $this = $(this);
   var content;
   var showTimeout;

   $tip = $('#jTooltip');
   if($tip.size() == 0){
    $('body').append('<div id="jTooltip" style="display:none;position:absolute;"><div></div></div>');
    $tipContent = $('#jTooltip div');
   }

   $this.mouseover(function(event) { 
    content = $this.attr('title');
    $this.attr('title', '');
    $tipContent.html(content); 
    $body.bind('mousemove', function(event){
     $tip.css({
      top: $(document).scrollTop() + (event.pageY + opts.yOffset),
      left: $(document).scrollLeft() + (event.pageX + opts.xOffset)
     });
    });
    showTimeout = setTimeout('$tip.fadeIn(' + opts.fadeTime + ')', opts.delay);
   });

   $this.mouseout(function(event) {
    clearTimeout(showTimeout); 
    $this.attr('title', content);
    $('body').unbind('mousemove');
    $tip.hide();
   });

  });
 };

 $.fn.jTooltip.defaults = {
  delay: 0,
  fadeTime: 300,
  yOffset: 10,
  xOffset: 10
 };

})(jQuery);


Updated code

  ;(function($) {
 $.fn.jTooltip = function(options) {

  opts = $.extend({}, $.fn.jTooltip.defaults, options);

  return this.each(function() {

   var $this = $(this);
   var showTimeout;

   $this.data('title',$this.attr('title'));
   $this.removeAttr('title');

   $document = $(document);
   $body = $('body');
   $tip = $('#jTooltip');
   $tipContent = $('#jTooltip div');
   if($tip.length == 0){
    $body.append('<div id="jTooltip" style="display:none;position:absolute;"><div></div></div>');  
   }

   $this.hover(function(event){
    $tipContent.html($this.data('title'));
    $body.bind('mousemove', function(event){
     $tip.css({
      top: $document.scrollTop() + (event.pageY + opts.yOffset),
      left: $document.scrollLeft() + (event.pageX + opts.xOffset)
     });
    });
    showTimeout = setTimeout(function(){
     $tip.fadeIn(opts.fadeTime);         
    }, opts.delay);     
   }, function(){
    clearTimeout(showTimeout); 
    $body.unbind('mousemove');
    $tip.hide();
   });

  });
 };

 $.fn.jTooltip.defaults = {
  delay: 0,
  fadeTime: 300,
  yOffset: 10,
  xOffset: 10
 };

})(jQuery);

Let me know if you have some more feedback ;)

+1  A: 

Can't say there's anything horribly wrong (others correct me if I'm wrong) but if I were to nitpick I would say use this:

$this.removeAttr('title'); //slightly more readable

instead of this:

$this.attr('title', '');

I just think it's prettier to call removeAttr instead of setting the attribute to the empty string - also, it allows other code to conditionally check for the existence of that attribute, rather than testing it's currently set value against the empty string.

karim79
+4  A: 

Use .length rather than .size(). Internally size() calls length so just saves the extra call.

Consider removing the title attribute when you create the tip and storing the tip on the element using .data('tip', title). This will avoid the need for you to constantly blank the title and then add it back again.

Create a function for the work you do inside the setTimeout. Implied eval is considered bad which is what happends when you pass a string to setTimeout/Interval.

window.setTimeout( yourFadeTipFunc , opts.fadeTime);

Cache the $(document) in a var rather than calling it twice.

You could chain the mouseout to the mouseover.

Other than that it is a really good, clean first effort.

redsquare