5
\$\begingroup\$

I am writing my own tooltip plugin for my portfolio website that I am currently working on. It works rather well, however, I feel that there is a lot I could improve. This is one of my bigger jQuery plugins that I have written.

(function($) {
// Used as a template for addTooltip()
var tooltipDefaults = {
 'class': null,
 'showOn': 'mouseenter',
 'hideOn': 'mouseleave',
 'direction': 'top',
 'offset': null
}
// Store generated IDs to avoid conflicting IDs
var tooltipIds = new Array();
// Keep track of displayed popups
var displayedTooltips = new Array();
function generateUniqueId()
{
 var id;
 do {
 id = Math.floor(Math.random()*90000) + 10000;
 } while ($.inArray(id, tooltipIds) !== -1);
 tooltipIds.push(id);
 return id;
}
function getUniqueId(id)
{
 return parseInt(id.substr(0, 5));
}
function isUniqueId(id)
{
 return !NaN(getUniqueId(id));
}
function removeUniqueId(id)
{
 var id = getUniqueId(id);
 var idIndex = $.inArray(id, tooltipIds);
 if (idIndex !== -1) {
 tooltipIds.splice(idIndex);
 }
}
$.fn.displayTooltip = function()
{
 var element = $(this);
 var tooltip = $('#' + element.attr('data-tooltip-id'));
 var config = element.data('config');
 var offset = element.offset();
 var left; 
 var top;
 switch (config.direction) {
 case 'left':
 top = offset.top + "px";
 left = offset.left - tooltip.outerWidth() - config.offset + "px";
 break;
 case 'top':
 top = offset.top - element.outerHeight() - config.offset + "px";
 left = offset.left + ((element.outerWidth() / 2) - (tooltip.outerWidth() / 2)) + "px";
 break;
 case 'right':
 top = offset.top + "px";
 left = offset.left + element.outerWidth() + config.offset + "px";
 break;
 case 'bottom':
 top = offset.top + element.outerHeight() + config.offset + "px";
 left = offset.left + ((element.outerWidth() / 2) - (tooltip.outerWidth() / 2)) + "px";
 break;
 }
 tooltip.css({
 'position': 'absolute',
 'left': left,
 'top': top,
 'z-index': 5000
 });
 if (element.isTooltipDisplayed()) {
 return;
 }
 tooltip.show();
 displayedTooltips.push(element.attr('id'));
}
$.fn.hideTooltip = function()
{
 var element = $(this);
 var idIndex = $.inArray(element.attr('id'), displayedTooltips);
 if (idIndex !== -1) {
 displayedTooltips.splice(idIndex);
 }
 $('#' + element.attr('data-tooltip-id')).hide();
}
$.fn.addTooltip = function(content, params)
{
 var config = $.extend(tooltipDefaults, params);
 return this.each(function() {
 var element = $(this);
 // If the element already has a tooltip change the content inside of it
 if (element.hasTooltip()) {
 $('#' + element.attr('data-tooltip-id')).html(content);
 return;
 }
 var tooltipId = (element.is('[id]') ? element.attr('id') : generateUniqueId()) + '-tooltip';
 element.attr('data-tooltip-id', tooltipId);
 var tooltip = $('<div>', {
 id: tooltipId,
 role: 'tooltip',
 class: config.class
 }).html(content);
 $('body').append(tooltip);
 /**
 * If showOn and hideOn are the same events bind a toggle
 * listener else bind the individual listeners
 */
 if (config.showOn === config.hideOn) {
 element.on(config.showOn, function() {
 if (!element.isTooltipDisplayed()) {
 element.displayTooltip();
 } else {
 element.hideTooltip();
 }
 });
 } else {
 element.on(config.showOn, function() {
 element.displayTooltip();
 }).on(config.hideOn, function() {
 element.hideTooltip();
 });
 }
 // Store config for other functions use
 element.data('config', config);
 // Saftey check incase the element recieved focus from the code running above
 element.hideTooltip();
 });
}
$.fn.hasTooltip = function()
{
 return $(this).is('[data-tooltip-id]');
}
$.fn.isTooltipDisplayed = function()
{
 var element = $(this);
 if (!element.hasTooltip()) {
 return false;
 }
 return ($.inArray(element.attr('id'), displayedTooltips) === -1) ? false : true;
}
$.fn.removeTooltip= function()
{
 return this.each(function() {
 var element = $(this);
 var tooltipId = element.attr('data-tooltip-id');
 var config = element.data('config');
 $('#' + tooltipId).remove();
 if (isUniqueId(tooltpId)) {
 removeUniqueId(tooltipId);
 }
 element.removeAttr('data-tooltip-id');
 if (config.showOn === config.hideOn) {
 element.off(config.showOn);
 } else {
 element.off(config.showOn);
 element.off(config.hideOn);
 }
 element.removeData('config');
 });
}
// Reposition tooltip on window resize
$(window).on('resize', function() {
 if (displayedTooltips.length < 1) {
 return;
 }
 for (var i = 0; i < displayedTooltips.length; i++) {
 $('#' + displayedTooltips[i]).displayTooltip();
 console.log(displayedTooltips);
 }
});
}(jQuery));
Jamal
35.2k13 gold badges134 silver badges238 bronze badges
asked Feb 16, 2014 at 14:43
\$\endgroup\$
5
  • \$\begingroup\$ new Array() -> [] \$\endgroup\$ Commented Feb 16, 2014 at 14:45
  • \$\begingroup\$ if (element.isTooltipDisplayed()) { return; } could probably called before all left & top calculations... \$\endgroup\$ Commented Feb 16, 2014 at 14:52
  • 2
    \$\begingroup\$ 1.this is not good jquery plugin api style conversion. your plugin should only has one namespace. 2. not use utility in jQuery(like $.inArray), the performance is too slow \$\endgroup\$ Commented Feb 17, 2014 at 1:49
  • \$\begingroup\$ check this performance table jspref \$\endgroup\$ Commented Feb 17, 2014 at 2:01
  • \$\begingroup\$ @caoglish indexOf() \$\endgroup\$ Commented Feb 17, 2014 at 8:35

1 Answer 1

1
\$\begingroup\$

From a once over:

  • There seems to be a ton of overkill with regards to id, since the user will never look at the id of a tooltip, you can simply use:

    function generateUniqueId()
    {
     var id = generateUniqueId.id = ( generateUniqueId.id || 0) + 1;
     tooltipIds.push(id);
     return id;
    }
    

    on the whole I would review the code and simplify how id's work.

  • Namespaces; @caoglish is right, your approach is wrong. jQuery plugin style would be 1 function tooltip on which you can call then create, show, hide etc.
  • In removeUniqueId you simply use id = getUniqueId(id); ( drop the var ), since you already have the id as a parameter
  • As @Charlie mentioned; var tooltipIds = new Array(); -> var tooltipIds = [];
  • In removeTooltip you have a bug in (isUniqueId(tooltpId)) { , you meant to use tooltipId
  • You would have caught this if you added "use strict"; right after (function($) { (this activates strict mode)
  • I really like the way you use config in displayTooltip
  • console.log() does not belong in production code
answered Feb 19, 2014 at 13:58
\$\endgroup\$
5
  • \$\begingroup\$ Thank you a lot! I will be sure to make these changes. Can you possibly explain to me how that generateUniqueId method works? \$\endgroup\$ Commented Feb 19, 2014 at 16:47
  • \$\begingroup\$ I am using the function to store the value of id and I just keep adding 1 so that the value is unique. \$\endgroup\$ Commented Feb 19, 2014 at 16:57
  • \$\begingroup\$ Alright, also is it really ideal to only use one namespace. It just seems cluttered to toss everything into one namespace. Or is there a design pattern that could make something like that better? \$\endgroup\$ Commented Feb 19, 2014 at 16:59
  • \$\begingroup\$ I will agree that it might be more cluttered for the author, but the convenience of the user comes first ;) \$\endgroup\$ Commented Feb 19, 2014 at 17:02
  • \$\begingroup\$ I just rewrote the whole plugin to an OOP approach. I also made the changes you suggested and the whole ID system is now out. I dropped about 80 lines of code. How much better is this than the previous one and are there still areas to be improved? If you wouldn't mind taking a look that is. \$\endgroup\$ Commented Feb 19, 2014 at 18:05

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.