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));
\$\endgroup\$
  • \$\begingroup\$ new Array() -> [] \$\endgroup\$ – Charlie Feb 16 '14 at 14:45
  • \$\begingroup\$ if (element.isTooltipDisplayed()) { return; } could probably called before all left & top calculations... \$\endgroup\$ – jerone Feb 16 '14 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\$ – caoglish Feb 17 '14 at 1:49
  • \$\begingroup\$ check this performance table jspref \$\endgroup\$ – caoglish Feb 17 '14 at 2:01
  • \$\begingroup\$ @caoglish indexOf() \$\endgroup\$ – Jivings Feb 17 '14 at 8:35
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
\$\endgroup\$
  • \$\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\$ – Ryan Printup Feb 19 '14 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 Feb 19 '14 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\$ – Ryan Printup Feb 19 '14 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 Feb 19 '14 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\$ – Ryan Printup Feb 19 '14 at 18:05

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

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