JQuery plugin not working when used in multiple places in a single page

Question

I am writing a JQuery plugin for a project I'm working on which turns from tabbed content on desktop devices to an accordion on mobile devices. I've used JQuery Boilerplate (https://github.com/jquery-boilerplate/jquery-boilerplate/blob/master/dist/jquery.boilerplate.js) as an initial pattern for my plugin.

The plugin is called on any element with the class ".tabs2accordion" as shown here:

 $(".tabs2accordion").tabs2Accordion({state:"desktop"});

The plugin works as expected if there is only one element with ".tabs2accordion" class on a page but starts to malfunction as soon as another element with the same class is added to the page. I've created a codepen of the basic code to demo the issue. To show the issue, on a window size of >768px try clicking any of the titles and observe how the content below changes as each title is clicked. Next uncomment the block of HTML and try clicking on the titles again.

http://codepen.io/decodedcreative/pen/MyjpRj

I have tried looping through each element with the class "tabs2accordion" like this:

$(".tabs2accordion").each(function(){
    $(this).tabs2Accordion({state:"desktop"});
});

But this didn't fix the issue either.

Any ideas?

*but starts to malfunction as soon as another element with the same class is added to the page* ....How so? you've only vaguely explained what your code *should* do, and havent explained at all what the actual issue is other that to say there is an issue. Please be more specific — Wesley Smith, Mar 13 '16 at 17:58
On screen sizes 768px and above, clicking on one of the three titles will display its corresponding tab content paragraph. Clicking another title will show another tab content paragraph. I may not have described it very well but I did provide a Codepen demonstrating the issue so there was no reason to be rude — James Howell, Mar 13 '16 at 22:36
My apologies for coming off rude, I just want you see that you havent given us the information we need to help you. We are all happy to help, but before we go trudging through the code in your example we need to know what the code *should do* and *how the code currently fails* .these are not usualy so obvious to an outsider as they are to the asker. For example, when I make the screen smaller in your demo, nothing happens when I click the titles, see http://i.imgur.com/v0JPO2g.png — Wesley Smith, Mar 14 '16 at 00:15
Ok I've edited the original post to further explain how to show the issue in Codepen. I didn't include the JS for the accordion functionality as it involves using a third party library and I didn't want to overcomplicate the example. The Codepen above has the same issue as in my project when viewed on a window >768px. I suspect that the two instances of the plugin are not fully self-contained and so when there are multiple instances on a page, clicking on the titles of one plugin instance is affecting the HTML of another. — James Howell, Mar 14 '16 at 08:51

Steve H. · Accepted Answer · 2016-03-16T14:48:59.687

I have not used jQuery Boilerplate, but I believe the problem here is with your variable called plugin.

Nowhere in your code do you declare a variable called plugin. When I stop the debugger in Plugin.prototype.showTabContent, I can evaluate window.plugin and it returns the global value for plugin.

In the constructor for Plugin, the first line reads plugin= this;. Since plugin is not defined, it is declaring the variable at global scope on the window object.

The fix is to pass a reference to the plugin object when setting up the $().on() hook. The data passed is available in the event handlers via the event parameter that is passed in the data property.

Here is the solution (at http://codepen.io/shhQuiet/pen/JXEjMV)

(function($, window, document, undefined) {
  var pluginName = "tabs2Accordion",
    defaults = {
      menuSelector: ".tabs2accordion-menu",
      tabContentSelector: ".tabs2accordion-content"
    };

  function Plugin(element, options) {
    this.element = element;
    this.$element = $(this.element);
    this.options = $.extend({}, defaults, options);
    this.$menu = $(this.element).find(this.options.menuSelector),
    this.$tabs = $(this.element).find(this.options.tabContentSelector),
    this.$accordionTriggers = $(this.element).find(this.$tabs).find("h3");
    this._defaults = defaults;
    this._name = pluginName;
    this.init();
  }

  Plugin.prototype = {

    init: function() {
      //Set all the tab states to inactive
      this.$tabs.attr("data-active", false);

      //Set the first tab to active
      this.$tabs.first().attr("data-active", true);

      //If you click on a tab, show the corresponding content
      this.$menu.on("click", "li", this, this.showTabContent);

      //Set the dimensions (height) of the plugin
      this.resizeTabs2Accordion({
        data: this
      });

      //If the browser resizes, adjust the dimensions (height) of the plugin
      $(window).on("resize", this, this.resizeTabs2Accordion);

      //Add a loaded class to the plugin which will fade in the plugin's content
      this.$element.addClass("loaded");

      console.log(this.$element);

    },

    resizeTabs2Accordion: function(event) {
      var contentHeight;
      var plugin = event.data;

      if (!plugin.$element.is("[data-nested-menu]")) {
        contentHeight = plugin.$tabs.filter("[data-active='true']").outerHeight() + plugin.$menu.outerHeight();
      } else {
        contentHeight = plugin.$tabs.filter("[data-active='true']").outerHeight();
      }

      plugin.$element.outerHeight(contentHeight);
    },

    showTabContent: function(event) {
      var $target;
      var plugin = event.data;
      plugin.$menu.children().find("a").filter("[data-active='true']").attr("data-active", false);
      plugin.$tabs.filter("[data-active='true']").attr("data-active", false);
      $target = $($(this).children("a").attr("href"));
      $(this).children("a").attr("data-active", true);
      $target.attr("data-active", true);
      plugin.resizeTabs2Accordion({data: plugin});

      return false;
    },

    showAccordionContent: function(event) {
      var plugin = event.data;
      $("[data-active-mobile]").not($(this).parent()).attr("data-active-mobile", false);

      if ($(this).parent().attr("data-active-mobile") === "false") {
        $(this).parent().attr("data-active-mobile", true);
      } else {
        $(this).parent().attr("data-active-mobile", false);
      }
    }

  };

  $.fn[pluginName] = function(options) {
    return this.each(function() {
      if (!$.data(this, "plugin_" + pluginName)) {
        $.data(this, "plugin_" + pluginName, new Plugin(this, options));
      }
    });
  };

})(jQuery, window, document);

$(window).on("load", function() {
  $(".tabs2accordion").tabs2Accordion({
    state: "desktop"
  });
});

I've used 'plugin=this' to pass the context of the 'this' keyword from the init function to other functions within the plugin such as showTabContent. Otherwise I wouldn't be able to use variables defined in the function Plugin because the context of the 'this' keyword would have changed. — James Howell, Mar 16 '16 at 09:45
I see that, however this is the root cause of the failure. You are using a global variable and overwriting the value when initializing the second instance of the plugin. You need to find another approach. — Steve H., Mar 16 '16 at 12:05
Thanks I appreciate the help. Do you know another way I could pass the context of the plugin instance around so that I can keep my variables self-contained to each plugin instance? — James Howell, Mar 16 '16 at 12:40
I will take a look later today and change my answer to reflect a solution as well. — Steve H., Mar 16 '16 at 12:42
Thanks. This will not only help me but a lot of other developers writing their first plugins with this pattern. — James Howell, Mar 16 '16 at 12:51
I've added a codepen for a working version and included the JS code here. Hope this answers your question. If you have any other questions, let me know. — Steve H., Mar 16 '16 at 14:29
Thanks so much for the help. I don't think I would have ever worked that out. I actually thought it would be much more straight-forward — James Howell, Mar 16 '16 at 22:26
I'm glad I was able to help. Since you accepted my answer, you can also award the bounty by clicking the bounty award icon next to my answer :) Let me know if you have any other questions... — Steve H., Mar 17 '16 at 05:10

score 1 · Answer 2 · answered Mar 17 '16 at 14:34

I rewrote your code following jQuery's Plugin creation standard.

http://codepen.io/justinledouxmusique/pen/GZrMgB

Basically, I did two things:

Moved away from using data attributes for styling (switched to using an .active class instead)
Moved away from using this everywhere, as it bring a whole wave of binding issues...

$.fn.tabs2Accordion loops through all the selectors, and applies $.tabs2Accordion. It also returns the selector for chaining (it's a standard in jQuery).

Then, all the internal methods are function expressions which are in the same scope as all your old this "variables". This simplifies the code greatly as you can refer to those variables without passing them in as a parameter or without having to .bind( this ) somehow.

Finally, the old init() function is gone. Instead, I put the code at the end of the $.tabs2Accordion function.

Hope this helps!

(function ( window, $ ) {
    $.tabs2Accordion = function ( node, options ) {
        var options = $.extend({}, {
                    menuSelector: '.tabs2accordion-menu',
                    tabContentSelector: '.tabs2accordion-content'
                }, options )

        var $element = $( node ),
                $menu = $element.find( options.menuSelector ),
                $tabs = $element.find( options.tabContentSelector ),
                $accordionTriggers = $tabs.find( 'h3' )

        var resizeTabs2Accordion = function () {
            $element.outerHeight( !$element.is( '[data-nested-menu]' )
                ? $element.find( 'div.active' ).outerHeight() + $menu.outerHeight()
                : $element.find( 'div.active' ).outerHeight() )
        }

        var showTabContent = function () {
            var $this = $( this ) // This will be the clicked element

            $menu
                .find( '.active' )
                    .removeClass( 'active' )

            $element
                .find( '.active' )
                    .removeClass( 'active' )

            $( $this.find( 'a' ).attr( 'href' ) )
                .addClass( 'active' )

            $this
                .find( 'a' )
                    .addClass( 'active' )

            resizeTabs2Accordion()

            return false
        }

        var showAccordionContent = function () {
            var $this                   = $( this ),
                    $parent                 = $this.parent(),
                    mobileIsActive  = $parent.data( 'active-mobile' )

            $( '[data-active-mobile]' )
                .not( $parent )
                    .data( 'active-mobile', false )

            $parent
                .data( 'active-mobile', mobileIsActive ? false : true )
        }

        // The equivalent of init()
        $tabs
            .removeClass( 'active' )
            .first()
                .addClass( 'active' )

        $element.addClass( 'loaded' )

        $menu.on( 'click', 'li', showTabContent )

        $( window ).on( 'resize', resizeTabs2Accordion )

        resizeTabs2Accordion()

        console.log( $element )
    }

    $.fn.tabs2Accordion = function ( options ) {
        this.each( function ( index, node ) {
            $.tabs2Accordion( node, options )
        })

        return this
    }
})( window, jQuery )

$( window ).on( 'load', function () {
    $( '.tabs2accordion' ).tabs2Accordion({
        state: 'desktop'
    })
})

JQuery plugin not working when used in multiple places in a single page

2 Answers2