Improving an Accordion script Part 1

13.05.2014 — JavaScript, jQuery — 2 min read

I was sitting in my office at work a while back, and a co-worker came by and asked me to take a look at some of their jQuery code. He had written some fairly simple code to create an accordion effect for a FAQ (Frequently Asked Questions to those who don't know) he was working on. Of course, I was glad to help. Anyone who is taking the time to have someone else validate their code is going to get my help. There are too few people like this and I wanted to promote this behavior.

Now, before I started, I informed him that this could hurt his feelings. If you write crappy code, I am more than happy to call you out, but it comes from a good place. I want you to get better and if I can help by showing you where you went wrong I will. He was more than happy to take the constructive criticism.

Our Intial Code

So let's start with the HTML:

1<div class="expand-all">Expand All</div>
2<div class="collapse-all">Collapse All</div>
3<ul class="accordion">
4    <li class="accordion-item">
5        <div class="accordion-header">Question one?</div>
6        <div class="accordion-content">Answer to this question.</div>
7    </li>
8    <li class="accordion-item">
9        <div class="accordion-header">Question two?</div>
10        <div class="accordion-content">Answer to this question.</div>
11    </li>
12    <li class="accordion-item">
13        <div class="accordion-header">Question three?</div>
14        <div class="accordion-content">Answer to this question.</div>
15    </li>
16</ul>

And here is the jQuery:

1function accordionLoad() {
2    $(".accordion-header").removeClass("expanded");
3    $(".accordion-content").hide();
4
5    $(".accordion-header").bind("click", function(){
6        $(this).toggleClass("expanded");
7        $(this).siblings(".accordion-content").slideToggle();
8    })
9
10    $(".expand-all").bind("click",function(){
11        $(this).siblings(".accordion").find(".accordion-content").slideDown();
12        $(this).siblings(".accordion").find(".accordion-header").addClass("expanded");
13
14    })
15
16    $(".collapse-all").bind("click",function(){
17        $(this).siblings(".accordion").find(".accordion-content").slideUp();
18        $(this).siblings(".accordion").find(".accordion-header").removeClass("expanded");
19    })
20}
21
22$(document).ready(function(){ accordionLoad(); });

So... not too bad. I can see a few things right off the bat that need to be done but I have seen much worse.

Updating the HTML

My first two suggestions were related to the HTML. I suggested we create a div to hold every part of the accordion and to preface each of our class names with the word accordion- so we would be less likely to collide with other peoples' CSS. This basically sets up the equivalent of a namespace in our CSS. Our HTML now looked like this:

1<div class="accordion">
2    <div class="accordion-expand-all">Expand All</div>
3    <div class="accordion-collapse-all">Collapse All</div>
4    <ul>
5        <li class="accordion-item">
6            <div class="accordion-header">Question one?</div>
7            <div class="accordion-content">Answer to this question.</div>
8        </li>
9        <li class="accordion-item">
10            <div class="accordion-header">Question two?</div>
11            <div class="accordion-content">Answer to this question.</div>
12        </li>
13        <li class="accordion-item">
14            <div class="accordion-header">Question three?</div>
15            <div class="accordion-content">Answer to this question.</div>
16        </li>
17    </ul>
18</div>

So now, with these changes made to our HTML, it was time to move into the jQuery code.

Updating the Code

My first suggestion was to move the .accordion-expanded class to the li element. This would allow us to style both the .accordion-header and the .accordion-content by adding the class to their parent element. This will also allow us to use CSS for showing and hiding the answers instead of javascript.

Also, since everything is now self-contained in the accordion div, we can use the jQuery .find() method and chain all the selections and methods together. Once we use the .find() method we will use the .end() method to return to our previous selection. For more information on this method, check out the jquery documentation.

So now we have this code:

1function accordionLoad() {
2    // Our single selector
3    var accordion =$('.accordion');
4
5    accordion
6    .find('.accordion-header')  // Get our questions
7        .on('click', function(){
8            $(this).parent().toggleClass('accordion-expanded')
9        })
10        .end()
11    .find('.accordion-expand-all') // Expand All
12        .on('click', function(){
13            accordion.find('.accordion-item').addClass('accordion-expanded');
14        })
15        .end()
16    .find('.accordion-collpase-all')  // Collpase All
17        .on('click', function(){
18            accordion.find('.accordion-item').removeClass('accordion-expanded');
19        })
20        .end(); // Not required but added for consistency
21}
22
23$(function() {
24    accordionLoad();  // Execute our script on page load
25});

Now, we can add these classes into our CSS for styling:

1.accordion-header {
2    /* basic styling for questions go here */
3}
4.accordion-content {
5    display: none;  /* hide our answers */
6    /* basic styling for answers go here */
7}
8.accordion-expanded > .accordion-header {
9    /* change how the question looks when answer is displayed */
10}
11.accordion-expanded > .accordion-content {
12    display:block; /*show our answer */
13}

Obviously, I am not going into the actual styling of the Questions and Answers here but you get the idea. Overall, I think that is a drastic improvement over our intial code. However, the are still many issues and optimizations we can do to this code and we will explore all of that in Part Two.

'Til next time,

-G