Why does my button need to be clicked twice for the event handler to work the first time, but thereafter just once?

Question

I want the visitor to be able to expand/collapse some sections, and am using:

<input onclick="return toggleDiv('xx')" 
       type="button" 
       class="button" 
       value="click here to expand/collapse"/>

and in the I have the function:

function toggleDiv(a){
  var e=document.getElementById(a);
  if(!e)return true;
  if(e.style.display=="none"){
    e.style.display="block"
  } else {
    e.style.display="none"
  }
  return true;
}

The first time a button is clicked it doesn't work, subsequent clicks (on any of the buttons) work OK.

There is related conversation here: http://stackoverflow.com/questions/542247/button-needs-to-be-clicked-twice-to-trigger-function

but I don't understand the answer (too technical;-), could someone help explain it please?

Answer 1

The initial style on your 'xx' div may be causing some trouble...

Explanation

Say you have a stylesheet rule configured to make your divs initially hidden. Something like:

div { display: none }

(...where of course the selector (div) will probably be a little bit less broad)

This would appear to work correctly, in that the page will load with all of your div elements hidden ("collapsed"). However, there's no actual value for the style.display property on the elements themselves - they're merely inheriting the value from the stylesheet. So in your code, the test you're using to check if the element is hidden:

if(e.style.display=="none"){

...will fail, incorrectly identifying the target as being visible, and causing the style.display property to be set to "none" (which has no visible effect, since the element had already been hidden by the stylesheet). Then the next time the button is clicked, the value you set the last time around will cause the text to succeed, and your routine will set style.display to "block".

The easy way to correct for this is to simply reverse your test and check for "block":

  if(e.style.display=="block"){
    e.style.display="none"
  } else {
    e.style.display="block"
  }

...however, this will fall apart if you have some elements configured to be initially visible: you'll just run into the same problem in reverse, with the first button click failing to have any visible effect. For a more robust behavior, you'll need to test the style that's actually active on the element:

function getStyle(el, name)
{
  // the way of the DOM
  if ( document.defaultView && document.defaultView.getComputedStyle )
  {
    var style = document.defaultView.getComputedStyle(el, null);
    if ( style )
      return style[name];
  }
  // IE-specific
  else if ( el.currentStyle )
    return el.currentStyle[name];

  return null;
}

function toggleDiv(a){
  var e=document.getElementById(a);
  if(!e)return true;
  if(getStyle(e, "display") == "none"){
    e.style.display="block"
  } else {
    e.style.display="none"
  }
  return true;
}

Here we use a helper function, getStyle(), to retrieve the active value in a cross-platform manner. See also: getComputedStyle(), currentStyle

Answer 2

Something like this might work for you. But put it in the head of the document. the window onload function is important because the DOM needs to be loaded before you can mess around with the input field

<input id="divOpener" rel="xx" type="button" class="button" value="click here to expand/collapse"/>

and this

window.onload = function(){
            //  get input field from html
            var myInput = document.getElementById('divOpener');
            // add onclick handler
            myInput.onclick = function(){
               // if you really need to say which div in the document body, you can
               // add it to the rel or ref etc. attributes then get the value with
               // get attribute
               var myDiv = document.getElementById(this.getAttribute('rel'));
               if(!myDiv) return;
               if(myDiv.style.display=="none"){
                  myDiv.style.display="block";
               } else {
                 myDiv.style.display="none";
               }
            }
        }