Button does not function on the first click

Question

function showCheckbox(){  
    var node_list = document.getElementsByClassName('check');
    for (var i = 0; i < node_list.length; i++) 
    {  
      if(node_list[i].style.display == 'none') { 
        node_list[i].style.display = 'block';
      } else { node_list[i].style.display = 'none'; }
    }
}

input[type=checkbox]{
display:none;
position:relative;
}

<input type="button"  value="Εμφάνιση" onclick="showCheckbox()" />
<img src="form-images\trash.png"  onclick="" style="width:21px;height:24px;margin-left:20px; "/>

    <input type="checkbox" class="check" />   
        <label>Ψάρεμα</label>
        <input type="text"  />
   	</br>
	<input type="checkbox" class="check" />   
        <label>Γήπεδο</label>
        <input type="text"/>
      </br>

When the page loads for first time and I press the button on the first click it does not triggers the onclick function. If I press it the second time it triggers the event.

Other <input type="button"/> buttons triggers the event on the first click without problem. Does anyone know what is the problem or did it have the same?

Answer 1

What I think is happening is that your click handler is being called on the first click, but your if test isn't working the way you expect. This line:

if(node_list[i].style.display == 'none')

...is testing whether the element has an inline style set. Which it doesn't: it's hidden via a CSS rule that applies to all such inputs. So then your else case executes and the .display is set to 'none'. Then on the next click, the if works as expected and changes .display to 'block'.

You can see this for yourself if you actually debug your function a little to see if it is getting called and test the value of that .display property - as you can see here: http://jsfiddle.net/uLjxp3ha/ (note: I don't recommend alert()s for debugging).

Checking the current visibility as set by stylesheet rules is a bit trickier because it doesn't work consistently across browsers. You may need to test for existence of .currentStyle and .getComputedStyle() to allow for whichever one the current browser might support. Have a look at this answer to another question for more information about that.

But in your case given that you know the checkboxes are hidden to begin with you can simply invert your if/else:

  if(node_list[i].style.display == 'block') { 
    node_list[i].style.display = 'none';
  } else {
    node_list[i].style.display = 'block';
  }

The .display will not be 'block' to start with, so the else will be executed and the elements will be displayed.

Demo: http://jsfiddle.net/uLjxp3ha/1/

Answer 2

Your code is brittle, easy to break. I suggest you to create a clear separation of concerns between css and javascript.

Also, your use of check class was twofold: select elements and hide them. That are two things better managed not coupled to each other.

Changing an element class could be as easy as:

node_list[i].classList.toggle('check-hidden');

You'll need to create a new css class for the actually hidden checkboxes.

function showCheckbox(){  
    var node_list = document.getElementsByClassName('check');
    for (var i = 0; i < node_list.length; i++) 
    {  
        node_list[i].classList.toggle('check-hidden');
    }
}

.check {
    position:relative;
}

.check-hidden {
    display:none;
}

<input type="button"  value="Εμφάνιση" onclick="showCheckbox()" />
<img src="form-images\trash.png"  onclick="" style="width:21px;height:24px;margin-left:20px; "/>

    <input type="checkbox" class="check" />   
        <label>Ψάρεμα</label>
        <input type="text"  />
   	</br>
	<input type="checkbox" class="check" />   
        <label>Γήπεδο</label>
        <input type="text"/>
      </br>