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));
1 Answer 1
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 thencreate
,show
,hide
etc. - In
removeUniqueId
you simply useid = getUniqueId(id);
( drop thevar
), since you already have theid
as a parameter - As @Charlie mentioned;
var tooltipIds = new Array();
->var tooltipIds = [];
- In
removeTooltip
you have a bug in(isUniqueId(tooltpId)) {
, you meant to usetooltipId
- 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
indisplayTooltip
console.log()
does not belong in production code
-
\$\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\$user36885– user368852014年02月19日 16:47:35 +00:00Commented 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\$konijn– konijn2014年02月19日 16:57:50 +00:00Commented 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\$user36885– user368852014年02月19日 16:59:07 +00:00Commented 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\$konijn– konijn2014年02月19日 17:02:28 +00:00Commented 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\$user36885– user368852014年02月19日 18:05:44 +00:00Commented Feb 19, 2014 at 18:05
new Array()
->[]
\$\endgroup\$if (element.isTooltipDisplayed()) { return; }
could probably called before all left & top calculations... \$\endgroup\$indexOf()
\$\endgroup\$