Accordion/collapsible

Question

The code I wrote to do this effect stays "fixed" on top or on bottom. I did it the manual hard-coded way and wanted some advice on the best way to write something like this.

jsFiddle

<script type="text/javascript">
function toggle_visibility(id) {
if (id == 'firstDivHeader')
{

document.getElementById('firstDivResults').style.display = 'block';
document.getElementById('firstDivHeader').style.top = '0';
document.getElementById('firstDivHeader').style.position = 'fixed';

document.getElementById('secondDivResults').style.display = 'none';
document.getElementById('secondDivHeader').style.top = '';
document.getElementById('secondDivHeader').style.bottom = '82px';
document.getElementById('secondDivHeader').style.position = 'fixed';

document.getElementById('thirdDivResults').style.display = 'none';
document.getElementById('thirdDivHeader').style.top = '';
document.getElementById('thirdDivHeader').style.bottom = '41px';
document.getElementById('thirdDivHeader').style.position = 'fixed';

document.getElementById('forthDivResults').style.display = 'none';
document.getElementById('forthDivHeader').style.top = '';
document.getElementById('forthDivHeader').style.bottom = '0px';
document.getElementById('forthDivHeader').style.position = 'fixed';



} else if (id == 'secondDivHeader')
{

document.getElementById('firstDivResults').style.display = 'none';
document.getElementById('firstDivHeader').style.top = '0';
document.getElementById('firstDivHeader').style.position = 'fixed';

document.getElementById('secondDivResults').style.display = 'block';
document.getElementById('secondDivHeader').style.top = '41px';
document.getElementById('secondDivHeader').style.bottom = '';
document.getElementById('secondDivHeader').style.position = 'fixed';

document.getElementById('thirdDivResults').style.display = 'none';
document.getElementById('thirdDivHeader').style.top = '';
document.getElementById('thirdDivHeader').style.bottom = '41px';
document.getElementById('thirdDivHeader').style.position = 'fixed';

document.getElementById('forthDivResults').style.display = 'none';
document.getElementById('forthDivHeader').style.top = '';
document.getElementById('forthDivHeader').style.bottom = '0px';
document.getElementById('forthDivHeader').style.position = 'fixed';



} else if (id == 'thirdDivHeader')
{


document.getElementById('firstDivResults').style.display = 'none';
document.getElementById('firstDivHeader').style.top = '0';
document.getElementById('firstDivHeader').style.bottom = '';
document.getElementById('firstDivHeader').style.position = 'fixed';

document.getElementById('secondDivResults').style.display = 'none';
document.getElementById('secondDivHeader').style.top = '41px';
document.getElementById('secondDivHeader').style.bottom = '';
document.getElementById('secondDivHeader').style.position = 'fixed';

document.getElementById('thirdDivResults').style.display = 'block';
document.getElementById('thirdDivHeader').style.top = '82px';
document.getElementById('thirdDivHeader').style.bottom = '0';
document.getElementById('thirdDivHeader').style.position = 'fixed';

document.getElementById('forthDivResults').style.display = 'none';
document.getElementById('forthDivHeader').style.top = '';
document.getElementById('forthDivHeader').style.bottom = '0px';
document.getElementById('forthDivHeader').style.position = 'fixed';



} else if (id == 'forthDivHeader')
{

document.getElementById('firstDivResults').style.display = 'none';
document.getElementById('firstDivHeader').style.top = '0';
document.getElementById('firstDivHeader').style.bottom = '';
document.getElementById('firstDivHeader').style.position = 'fixed';

document.getElementById('secondDivResults').style.display = 'none';
document.getElementById('secondDivHeader').style.top = '41px';
document.getElementById('secondDivHeader').style.bottom = '';
document.getElementById('secondDivHeader').style.position = 'fixed';

document.getElementById('thirdDivResults').style.display = 'none';
document.getElementById('thirdDivHeader').style.top = '82px';
document.getElementById('thirdDivHeader').style.bottom = '';
document.getElementById('thirdDivHeader').style.position = 'fixed';

document.getElementById('forthDivResults').style.display = 'block';
document.getElementById('forthDivHeader').style.top = '123px';
document.getElementById('forthDivHeader').style.bottom = '';
document.getElementById('forthDivHeader').style.position = 'fixed';



}
}

</script>
<div id=menuContainer>
<a href="#" onclick="toggle_visibility('firstDivHeader');"> <div id="firstDivHeader" style="z-index: 100; width: 100%; height: 30px; background-color: #560000; border-width: 0px 0px 1px 0px; border-style: solid; border-color: black; color: white; font-weight: bold; font-size: 14px;padding-top: 10px; text-align: center;">
    First Div
</div>
    </a>
    <div class=item id="firstDivResults" style="display: none;">
blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
        blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />

    </div>


<a href="#" onclick="toggle_visibility('secondDivHeader');"> <div id="secondDivHeader" style="z-index: 100; width: 100%; height: 30px; background-color: #560000; border-width: 0px 0px 1px 0px; border-style: solid; border-color: black; color: white; font-weight: bold; font-size: 14px;padding-top: 10px; text-align: center;">
    Second Div
</div>
    </a>
        <div class=item id="secondDivResults" style="display: none;">
        blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
        blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />

    </div>

<a href="#" onclick="toggle_visibility('thirdDivHeader');"> <div id="thirdDivHeader" style="z-index: 100; width: 100%; height: 30px; background-color: #560000; border-width: 0px 0px 1px 0px; border-style: solid; border-color: black; color: white; font-weight: bold; font-size: 14px;padding-top: 10px; text-align: center;">
    third Div
</div>
    </a>
        <div class=item id="thirdDivResults" style="display: none;">blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
        blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />

    </div>


<a href="#" onclick="toggle_visibility('forthDivHeader');"> <div id="forthDivHeader" style="z-index: 100; width: 100%; height: 30px; background-color: #560000; border-width: 0px 0px 1px 0px; border-style: solid; border-color: black; color: white; font-weight: bold; font-size: 14px;padding-top: 10px; text-align: center;">
    Fourth Div
</div>
    </a>

<div class=item id="forthDivResults" style="display: none;">
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />
       blah<br />
    blah<br />
    blah<br />

    </div>

</div>

Hi. Welcome to Code Review! Couldn't you come up with better content than repeated blah separated by line breaks? — Brythan, Commented Feb 1, 2015 at 11:36

Brythan · Accepted Answer · 2015-02-01 16:04:43Z

Helper Functions

You can make the code shorter with a helper function. Replace code like

document.getElementById('secondDivHeader').style.top = '';
document.getElementById('secondDivHeader').style.bottom = '82px';
document.getElementById('secondDivHeader').style.position = 'fixed';

with

            set_position('secondDivHeader', '', '82px');

Which is defined as

  function set_position(id, top, bottom) {
    document.getElementById(id).style.top = top;
    document.getElementById(id).style.bottom = bottom;
    document.getElementById(id).style.position = 'fixed';
  }

Naming

You misspelled fourth.

Names like fourthDiv are brittle and difficult to maintain. It's better to make the names reflect the actual content. That's somewhat hampered here by the lack of actual content.

`toggle_visibility`

<a href="#" onclick="toggle_visibility('firstDivHeader');"> <div id="firstDivHeader" style="z-index: 100; width: 100%; height: 30px; background-color: #560000; border-width: 0px 0px 1px 0px; border-style: solid; border-color: black; color: white; font-weight: bold; font-size: 14px;padding-top: 10px; text-align: center;">

If you change this code to

    <a href="#" onclick="toggle_visibility('firstDivResults');"> <div id="firstDivHeader">

Then you can just say

document.getElementById(id).style.display = 'block';

to display a section. Also, you should define your CSS in a <style> block (or external file), not inline.

<style>
#firstDivHeader, #secondDivHeader, #thirdDivHeader, #fourthDivHeader {
    z-index: 100;
    width: 100%;
    height: 30px;
    background-color: #560000;
    border-width: 0px 0px 1px 0px;
    border-style: solid;
    border-color: black;
    color: white;
    font-weight: bold;
    font-size: 14px;
    padding-top: 10px;
    text-align: center;
}
</style>

You could simplify that by declaring a CSS class that covered those four cases. Or possibly a different element, e.g. an <h2>.

Hiding

document.getElementById('secondDivResults').style.display = 'none';

Every occurrence like this can be replaced by a single hide_all function:

function hide_all() {
  var elements = document.getElementsByClassName('item');
  for ( var i = 0; i < elements.length; ++i ) {
    elements[i].style.display = 'none';
  }
}

    <div class=item id="firstDivResults" style="display: none;">

Rather than hide it in the HTML, hide in JavaScript:

    hide_all();

This will also allow people who don't have JavaScript to view the content.

`switch`

if (id == 'firstDivHeader')
} else if (id == 'secondDivHeader')
} else if (id == 'thirdDivHeader')
} else if (id == 'forthDivHeader')

This form of if/else is often better done with a switch statement.

        switch (id) {
          case 'firstDivResults':
            break;
          case 'secondDivResults':
            break;
          case 'thirdDivResults':
            break;
          case 'fourthDivResults':
            break;
        }

The switch makes it clearer that all the cases are based on the value of id. An if/else is more flexible.

Final Version

<!DOCTYPE html>
<html>
  <head>

    <style>
    #firstDivHeader, #secondDivHeader, #thirdDivHeader, #fourthDivHeader {
        z-index: 100;
        width: 100%;
        height: 30px;
        background-color: #560000;
        border-width: 0px 0px 1px 0px;
        border-style: solid;
        border-color: black;
        color: white;
        font-weight: bold;
        font-size: 14px;
        padding-top: 10px;
        text-align: center;
    }
    </style>

  </head>

  <body>
    <div id=menuContainer>
    <a href="#" onclick="toggle_visibility('firstDivResults');"> <div id="firstDivHeader">
        First Div
    </div>
        </a>
        <div class="item" id="firstDivResults">
    blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
            blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />

        </div>


    <a href="#" onclick="toggle_visibility('secondDivResults');"> <div id="secondDivHeader">
        Second Div
    </div>
        </a>
            <div class="item" id="secondDivResults">
            blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
            blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />

        </div>

    <a href="#" onclick="toggle_visibility('thirdDivResults');"> <div id="thirdDivHeader">
        Third Div
    </div>
        </a>
            <div class="item" id="thirdDivResults">blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
            blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />

        </div>


    <a href="#" onclick="toggle_visibility('fourthDivResults');"> <div id="fourthDivHeader">
        Fourth Div
    </div>
        </a>

    <div class="item" id="fourthDivResults">
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />
           blah<br />
        blah<br />
        blah<br />

        </div>

    </div>

    <script type="text/javascript">
      function hide_all() {
          var elements = document.getElementsByClassName('item');
          for ( var i = 0; i < elements.length; ++i ) {
            elements[i].style.display = 'none';
          }
      }

      function set_position(id, top, bottom) {
        document.getElementById(id).style.top = top;
        document.getElementById(id).style.bottom = bottom;
        document.getElementById(id).style.position = 'fixed';
      }

      function toggle_visibility(id) {
        hide_all();

        document.getElementById(id).style.display = 'block';
        document.getElementById('firstDivHeader').style.top = '0';
        document.getElementById('firstDivHeader').style.position = 'fixed';

        switch (id) {
          case 'firstDivResults':
            set_position('secondDivHeader', '', '82px');
            set_position('thirdDivHeader', '', '41px');
            set_position('fourthDivHeader', '', '0px');
            break;
          case 'secondDivResults':
            set_position('secondDivHeader', '41px', '');
            set_position('thirdDivHeader', '', '41px');
            set_position('fourthDivHeader', '', '0px');
            break;
          case 'thirdDivResults':
            set_position('secondDivHeader', '41px', '');
            set_position('thirdDivHeader', '81px', '');
            set_position('fourthDivHeader', '', '0px');
            break;
          case 'fourthDivResults':
            set_position('secondDivHeader', '41px', '');
            set_position('thirdDivHeader', '81px', '');
            set_position('fourthDivHeader', '121px', '');
            break;
        }
      }

    hide_all();
    </script>

  </body>
</html>

Very very very nice explanation on things that all helped me tremendously understand a lot about java script formatting and gives me great ideas moving forward. I extremely appreciate your help — eqiz, Commented Feb 2, 2015 at 15:10

Stack Exchange Network

Accordion/collapsible

1 Answer 1

Helper Functions

Naming

`toggle_visibility`

Hiding

`switch`

Final Version

Your Answer

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

Hot Network Questions

Accordion/collapsible

1 Answer 1

Helper Functions

Naming

toggle_visibility

Hiding

switch

Final Version

Your Answer

Sign up or log in

Post as a guest

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

Related

Hot Network Questions

`toggle_visibility`

`switch`

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