CODEWITHSUNDEEP
javascripthtmlbuttondom-events
nikolaos mparoutis
nikolaos mparoutis

Reputation: 450

Button does not function on the first click

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?

Upvotes: 8

Views: 28384

Answers (6)

theh4cker
theh4cker

Reputation: 1

Do this and try I will work because if statement first check the code and then runs

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

Upvotes: 0

Normajean
Normajean

Reputation: 1265

Instead of writing display: none in your css file, write it inline in your html document so it'd be more like:

<input style="display: none" type="checkbox" class="check" />

Upvotes: 0

nril
nril

Reputation: 568

The style.display property is not present the first time you do the loop, you could change the code as follows:

    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) {
            node_list[i].style.display = 'block';
        } else { node_list[i].style.display = 'none'; }
    }
}

A better solution might be (if you can use jQuery)

    function showCheckbox(){

    $( "input.check" ).each(function() {
        if ($(this).css('display') == 'none'){
            $(this).css('display','block')
        } else {
            $(this).css('display','none')
        }
    });
}

Upvotes: 0

Andr&#233; Werlang
Andr&#233; Werlang

Reputation: 5944

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>

Upvotes: 1

Andrew Matveev
Andrew Matveev

Reputation: 26

reason is pretty simple: when you get a "style" property, it represents inline styles of that element. On fist click there is no inline style for "display", so else fork fires and sets inline style for "display" to "none"

What you may do is next

  1. Use computed style

    window.getComputedStyle(node_list[i]).display == "none"

  2. Or switch "if" statement to

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

https://developer.mozilla.org/en-US/docs/Web/API/window.getComputedStyle

Upvotes: 0

nnnnnn
nnnnnn

Reputation: 150030

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/

Upvotes: 9

Related Questions