4
\$\begingroup\$

This is a jQuery plug-in I've created that relies on jQuery UI's tabs plug-in. It adds some framework specific logic when setting up tabs:

  1. Fixes a bug in jQuery that appears when an id/class contains a "."
  2. Manages the tab state using a framework specific cookie API (does not use the jQuery cookie plugin)
  3. Exposes a couple of options for transition behavior

I'd love to get some suggestions on how to improve this, as it'll be used widely in the framework.

(function ($) {
    $.fn.dnnTabs = function (options) {
        var opts = $.extend({}, $.fn.dnnTabs.defaultOptions, options),
            $wrap = this;

        $(document).ready(function () {
            // Patch for Period in IDs (jQuery bug)
            $.ui.tabs.prototype._sanitizeSelector = function (hash) {
                // we need this because an id may contain a ":" or a '.'
                return hash.replace(/:/g, "\\:").replace(/\./g, "\\\.");
            };

            var id = 'dnnTabs-' + $wrap.selector.replace('#', '').replace('.', '');
            if (opts.selected === -1) {
                var tabCookie = dnn.dom.getCookie(id);
                if (tabCookie) {
                    opts.selected = tabCookie;
                }
                if (opts.selected === -1) {
                    opts.selected = 0;
                }
            }

            $wrap.tabs({
                show: function (event, ui) {
                    dnn.dom.setCookie(id, ui.index, 1, '/', '', false);
                },
                selected: opts.selected,
                fx: {
                    opacity: opts.opacity,
                    duration: opts.duration
                }
            });
        });

        return $wrap;
    };

    $.fn.dnnTabs.defaultOptions = {
        opacity: 'toggle',
        duration: 'fast',
        selected: -1
    };

})(jQuery);
\$\endgroup\$

1 Answer 1

2
\$\begingroup\$

These are probably more comments, but I'll post them as an "answer", because it's easier.

  • Could you point out a source or an example for the bug with ids/classes containing . and :, because in a quick test they seem to work fine: http://jsfiddle.net/NSXLc/

  • I don't think you should use the document ready event. It should be the plugins user's choice when he wants to apply the tabs.

  • The way you are getting the id for the cookie looks dangerously wrong. You assume the selector is always a simple id or class selector and extending on that you assume, that the selector only matches one element and thus only one one set of tabs are initialized at a time.

It's probably better to do something like this:

$wrap.each(function() { // In case the selector matches multiple elements
  var showEvent = null;
  if (this.id) { // only use cookie if ID is set. TODO: use className as alternative
    var id = 'dnnTabs-' + this.id;
    if (opts.selected === -1) {
      // ...
    }

    showEvent = (function(cookieId) { // closure for cookie id
      return function (event, ui) {
        dnn.dom.setCookie(cookieId, ui.index, 1, '/', '', false);
      }
    })(id);
  }
  $(this).tabs({
    show: showEvent,
    // ...
  });
});
\$\endgroup\$
3
  • \$\begingroup\$ Thanks very much for your feedback, it's very helpful and exactly what I was hoping for. Here is the link to the bug demo - I think you have to try it specifically with the tabs plugin: jsfiddle.net/9Mst9/2 \$\endgroup\$ Apr 27, 2011 at 16:06
  • \$\begingroup\$ The only thing that's a little weird now is the requirement on each element the plugin is operating on to have an id. I think this is probably fine in general practice, but there will inevitably be an edge case where it will not. Is your suggestion to add fallback logic to use the className when the id is not present? Thanks again. \$\endgroup\$ Apr 27, 2011 at 16:29
  • \$\begingroup\$ Ok, that's a real bug the Tabs plugin in. I thought you meant the bug was in jQuery itself. Since you want to store the state of set of tabs in a cookie, id would make sense to give the tabs an id, so that it can use as the cookie name. \$\endgroup\$
    – RoToRa
    Apr 28, 2011 at 12:34

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge that you have read and understand our privacy policy and code of conduct.

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