Accordion Buttons Not Closing (Panel) after Minor Change

Question

Using JavaScript. So, I have set this up in two ways. One way everything works perfectly and the other will not, despite the fact that I can see no difference between them.

I'm displaying tables after user selection on a map. Each new selection populates a new accordion button that should be opened and closed. The tables are set up after an Ajax call to GeoServer so all of this must be dynamic (like the solution that works but the button's do not close). There is a count attached to it so that new panels/table containers/tables are created each time.

Scenario 1) I hard coded the html in which works fine. Like this:

    <div class="container" id="accordionContainer">      
        <button class="accordion">Selection 1</button>
            <div class="panel" id="panel1">
                <div class="container" id="tableContainer">
                     <table id="featureTable" class="display" width="100%">
                        <thead></thead><tfoot></tfoot>
                     </table>
              </div>
            </div>
    </div>

I set up 6 or 7 of those in the body section (head section works as well) and hard coded the count number (like panel1 above). Works great. User selects a map area and the features are displayed in an accordion button. I can't have hard coded anything though. There needs to be no limit on how many selections/tables.

Which brings me to:

Scenario 2) Everything is set up dynamically after each Ajax request. This works, but the buttons do not close. They are open and display all the tables.

    var btnPart = '<button class="accordion">' + 'Selection ' + tableCount.toString() + ' ' + activeOverlay+ '</button>';
    var panelPart = '<div class="panel" id="panel' + panelCount.toString() + '">';          
    var tabContPart = '<div class="container" id="tableContainer' + tableContainerCount.toString() +'">';
    var tablePart = '<table id="featureTable' + tableCount.toString() +  '" class="display" width="100%"><thead></thead><tfoot></tfoot></table></div></div></div>';

        $('#accordionContainer').append(btnPart, panelPart, tabContPart, tablePart);
                         var newTable = '#featureTable' + tableCount.toString();
                                $(newTable).DataTable( {
                                    data: data.features,   //get the data under features
                                    columns: columns,
                                   destroy: true
                                } );

I cannot see a difference between hard coding the DOM ahead of time or setting it up on the fly that would cause the buttons to lose their open/close functionality.

UPDATED CODE AS REQUESTED:

<body>

    <div id="map"></div>
    <div id="sidebar"></div>
    <div class="container" id="accordionContainer"></div>   
</body>  

<script>
    var acc = document.getElementsByClassName("accordion");
                var i;
                for (i = 0; i < acc.length; i++) {
                    acc[i].onclick = function(){
                        /* Toggle between adding and removing the "active" class,
                        to highlight the button that controls the panel */
                        this.classList.toggle("active");

                        /* Toggle between hiding and showing the active panel */
                        var panel = this.nextElementSibling;
                        if (panel.style.display === "block") {
                            panel.style.display = "none";
                        } else {
                            panel.style.display = "block";
                        }
                    }
}
        var btnPart = '<button class="accordion">' + 'Selection ' + tableCount.toString() + ' ' + activeOverlay+ '</button>';
        var panelPart = '<div class="panel" id="panel' + panelCount.toString() + '">';          
        var tabContPart = '<div class="container" id="tableContainer' + tableContainerCount.toString() +'">';
        var tablePart = '<table id="featureTable' + tableCount.toString() +  '" class="display" width="100%"><thead></thead>                        <tfoot></tfoot></table></div></div></div>';

            $('#accordionContainer').append(btnPart, panelPart, tabContPart, tablePart);
                             var newTable = '#featureTable' + tableCount.toString();
                                    $(newTable).DataTable( {
                                        data: data.features,   //get the data under features
                                        columns: columns,
                                       destroy: true
                                    } ); 
</script>

Where is the javascript code that opens and closes your items. Most likely you are running into a listener issue. Listeners either look at elements on load OR you can use delegation to listen for events on elements that are added to the DOM. My guess is you are doing it the first way. Additionally, your code is making DOM elements with duplicate IDs which is not valid HTML and can cause issues. — Leeish, Jul 21 '17 at 16:57
I have the open/close script outside the document body button stuff ... The only changed variable in all this is how the div stuff is generated. Either manually ahead of time or on the fly. — , Jul 21 '17 at 17:01
Post it so we can see it because most likely that is where the problem is. — Leeish, Jul 21 '17 at 17:02
Updated the Post with the structure. Also, I am not making duplicate DOM elements. There is a counter that is attached to the name when generated. The first example was just to test I can append my tables into the button/panels. — , Jul 21 '17 at 17:14
Sorry I didn't see the `tableCount` part. Not sure how I missed it. — Leeish, Jul 21 '17 at 17:44

Leeish · Accepted Answer · 2017-07-21T18:02:40.273

0

So it's odd you are using jquery for some parts and vanilla JS for others. I'm a jQuery guy and you are using jQuery so I'll post my answer in jQuery.

//Document listens for clicks on accordions
$(document).on('click','.accordion',function(){ 
    //Add or Remove the active class on the clicked accordion
    $(this).toggleClass('active');
    //If the clicked accordionhas the active class, then show the panel
    if($(this).hasClass('active')){
        $(this).next('.panel').show();
    } else { //Otherwise hide the panel
        $(this).next('.panel').hide();
    }
});

This is how I would do it. Remove everything from var acc = document.getElementsByClassName("accordion"); to the btnPart.

Explination. What onclick does when you run your code is tells Javascript to find all the elements on the page that match your select and listen for clicks. It doesn't look for anything added AFTER the listener is created. In your example simply moving your code down after you add everything would work. But the preferred method is to use event delegation and delegate the listener to a parent element, or in my example the document itself.

My code says:

Hey, tell the document that if it gets a click, to check and see if it was on an .accordian, and if it is run this code.

Your code says, find the .accordians that are on the page RIGHT NOW and if they get clicked, run this code.

The benefit of delegation is that you can add more elements any anytime via ajax or anything, and the listeners will still work.

edited Jul 21 '17 at 18:02

answered Jul 21 '17 at 17:38

Leeish

5,203
2
17
45

Here is an example of using event delegation (like I'm doing with `on`) using vanialla JS so you can understand what exactly is going on. https://stackoverflow.com/questions/24117369/vanilla-js-event-delegation-dealing-with-child-elements-of-the-target-element – Leeish Jul 21 '17 at 17:43
Still the same behavior .. :-( .. but still ONLY when the button, panel, table container, and table are set up dynamically. I don't see how that would change anything. It's truly baffling. If I hard code the count's and names and set the DOM elements up ahead of time, everything works perfectly. – Jul 21 '17 at 17:52
I spelled accoridan wrong. Did you catch that? Also, with append, you are appending partial elements, and looking at the DOM jQuery is auto closing them, probably not what you expected. See: https://jsfiddle.net/odwa9az6/ for some changes and the fact that my code works. – Leeish Jul 21 '17 at 18:00
Update just for example purposes with multiple buttons: https://jsfiddle.net/odwa9az6/1/ – Leeish Jul 21 '17 at 18:01
Oh man.. I just figured it out by looking at your jfiddle.... I had this: $('#accordionContainer').append(btnPart, panelPart, tabContPart, tablePart); It SHOULD BE: $('#accordionContainer').append(btnPart + panelPart + tabContPart + tablePart); '+' not ',' That is infuriating! But such a relief that it's working. Leeish, I really appreciate your help. – Jul 21 '17 at 18:05
Np. I'd still read up on delegation and probably not use onclick https://developer.mozilla.org/en-US/docs/Web/API/GlobalEventHandlers/onclick as per MDN (You may be inclined to use the EventTarget.addEventListener() method instead, since it is more flexible and part of the DOM Events specification.) Instead use `on` and tie it to a parent if you are going to add stuff dynamically. It's just a better way, but sometimes whatever works is fine as well. – Leeish Jul 21 '17 at 18:08
Absolutely. I just started with javascript a couple of months ago so I'm still figuring things out. Thanks again for your help. – Jul 21 '17 at 18:09

Accordion Buttons Not Closing (Panel) after Minor Change

1 Answers1