0

I have an expand/collapse block for which I have written a function as such:

function expandCollapse() {
    var $btnShowHide = $('.btn-show-hide');
    var $contentShowHide = $('.content-show-hide');

    contentToggle();

    $btnShowHide.each(function() {
        $(this).on('click', function() {
            var i = $btnShowHide.index(this);
            $contentShowHide.eq(i).slideToggle('fast');
            $contentShowHide.eq(i).toggleClass('collapsed');

            if ($contentShowHide.eq(i).hasClass('collapsed')) {
                $('.icon-show-hide', this).text('+');
            } else {
                $('.icon-show-hide', this).text('-');
            }
        });
    });

    function contentToggle() {
        $contentShowHide.each(function() {
            var i = $contentShowHide.index(this);
            if ($(this).hasClass('collapsed')) {
                $(this).hide();
                $('.icon-show-hide', $btnShowHide.eq(i)).text('+');
            } else {
                $('.icon-show-hide', $btnShowHide.eq(i)).text('-');
            }
        });
    }
}

and I call this function on $(document).ready. This works fine but fails when there is an ajax call done in the page. So, I looked at this answer and called the function again on ajax success, but this makes the behaviour odd (like, clicking on the btn once will collapse and expand the content multiple times for a single click). Any ideas on how I can get around this?

Sample HTML (there could be multiple of these on one page):

<h3 class="btn-show-hide">
 <span class="icon-show-hide"></span>
 <span>Title</span>
</h3>
<div class="content-show-hide collapsed">
//Stuff
</div>
Community
  • 1
  • 1
zer0
  • 4,657
  • 7
  • 28
  • 49
  • 1
    When and how are you calling expandCollapse()? And can you post the HTML too ;) – An0nC0d3r Dec 16 '15 at 18:09
  • @AdamJeffers on document.ready and ajax success. – zer0 Dec 16 '15 at 18:27
  • Ok, can you also post the code where you are using ajax – An0nC0d3r Dec 16 '15 at 18:31
  • The ajax calls are being made through JSF. I just have a function ajaxSuccess that is being called when the JSF is done. – zer0 Dec 16 '15 at 18:33
  • 1
    See my answer, each time you call `expandCollapse` you are rebinding the same event handler to all your buttons over and over. If you really don't want to change your code structure you COULD make a call to `off()` beforehand.... – A.O. Dec 16 '15 at 18:35
  • If your button triggers are replaced as part of the AJAX response content, then you will need to bind the click events to a static parent and use a [delegate](http://stackoverflow.com/questions/203198/event-binding-on-dynamically-created-elements?lq=1). – Jasen Dec 16 '15 at 19:02
  • 1
    Is there a reason why you're binding your event handlers in a loop? That's considered bad form. Consider using @Jasen's suggestion of using delegation. – Heretic Monkey Dec 16 '15 at 19:05
  • @Jasen so, I could do $('body').on() ? – zer0 Dec 16 '15 at 19:29
  • 1
    Yes, you can bind to body but it would be better to find the closest static parent. – Jasen Dec 16 '15 at 19:43

2 Answers2

1

Apparently the solution is to set an off before listening to the event. So, it would just be

 $btnShowHide.each(function() {
        $(this).off('click').on('click', function() {
            var i = $btnShowHide.index(this);
            $contentShowHide.eq(i).slideToggle('fast');
            $contentShowHide.eq(i).toggleClass('collapsed');

            if ($contentShowHide.eq(i).hasClass('collapsed')) {
                $('.icon-show-hide', this).text('+');
            } else {
                $('.icon-show-hide', this).text('-');
            }
        });
    });

This fixed my problem but I have no idea why this works.

zer0
  • 4,657
  • 7
  • 28
  • 49
0

Unless you are removing/replacing all instances of $('.btn-show-hide'); and $('.content-show-hide'); before each call to expandCollapse() you are re-adding the same event handler over and over, which I suspect is the cause of the multiple collapse/expands.

$btnShowHide.each(function() {
        $(this).on('click', function() {
            var i = $btnShowHide.index(this);
            $contentShowHide.eq(i).slideToggle('fast');
            $contentShowHide.eq(i).toggleClass('collapsed');

            if ($contentShowHide.eq(i).hasClass('collapsed')) {
                $('.icon-show-hide', this).text('+');
            } else {
                $('.icon-show-hide', this).text('-');
            }
        });
    });

The above code adds a new event handler each time you call expandCollapse(), perhaps it should be placed outside this function and only executed once...

A.O.
  • 3,733
  • 6
  • 30
  • 49
  • I just tried this. I moved the code you highlighted to document.ready and just called contentToggle(); in ajaxSuccess() and document.ready. The click event no longer works. – zer0 Dec 16 '15 at 18:39
  • @A.O. Your answer/example code is exactly the same as what OP already has? – An0nC0d3r Dec 16 '15 at 18:42
  • Did you also move your references to `$btnShowHide` and `$contentShowHide`? @AdamJeffers The code snippet is the same, I'm saying that it should be moved outside of the `expandCollapse()` function so that he isn't rebinding his click event handlers with each call to `expandCollapse()` – A.O. Dec 16 '15 at 18:43
  • @A.O. I believe the below restructuring of the code by AdamJeffers is what you are referring to. I tried it and the click events fail again. – zer0 Dec 16 '15 at 19:24