8

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?

5
  • Post your code and mark up.
    – PM 77-1
    Jan 23, 2015 at 0:18
  • Make sure that the showCheckbox function is defined before the HTML. Else, the input tag will be asking for a function that doesn't exist(yet).
    – SirPython
    Jan 23, 2015 at 0:24
  • i declared all the <script> ... </script> in the head the error remains after i put it after the <body> still the error remains .so? Jan 23, 2015 at 0:31
  • Does the first click that doesn't work result in any errors in your browser's dev console? (And when you say it doesn't trigger the function, are you certain that the function is not called at all, as compared to it being called but not correctly showing/hiding the checkboxes?)
    – nnnnnn
    Jan 23, 2015 at 0:34
  • inspect element -> console->JS is blank so i guess no errors. On Css there are many errors but they are at the backgrounds on other classes Jan 23, 2015 at 0:42

6 Answers 6

9

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/

6
  • You are Right.That was a really weird error. Thanks. Did my comment on Css helped you to find this? Jan 23, 2015 at 0:59
  • No, I didn't see that comment until after putting the answer up.
    – nnnnnn
    Jan 23, 2015 at 1:40
  • haha ok you helped me to use the console...i was thinking it was useless to check CSS from console if JS has not errors Jan 23, 2015 at 1:49
  • Can you explain why node_list[i].style.display refers to inline style set? I used Google Dev Tools in the browser and first (no clicks) it shows something element = {}. After the first click, the property is saved/copied inside the element, and only after that does the programmed change occurs on the second click. Is there a difference between "style" and "element"? This has me confused. Oct 27, 2021 at 14:01
  • @ChinmayGhule - node_list[i] refers to the element. The node_list[i].style.display property refers to the inline display style on that element if that inline style has been set.
    – nnnnnn
    Oct 28, 2021 at 8:04
1

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>

8
  • I do not understand what you mean "brittle, easy to break."I am obligated to use only JS no Jquery What do you mean by "brittle, easy to break."? Jan 23, 2015 at 1:20
  • By brittle I mean that it is fragile at this point. Inverting conditions could make it work, but won't make your code more robust. A change on css would make it break again. Jan 23, 2015 at 1:24
  • And software should have only a single reason to break. Any change unrelated to the behaviour you have implemented shouldn't break that behaviour. It's all about code quality. Jan 23, 2015 at 1:26
  • Ah, glad to see that you are bound to pure JS. My snippet is pure JS. Jan 23, 2015 at 1:29
  • Why to break again?What are you thinking? Speak briefly just for informative purposes. Jan 23, 2015 at 1:30
0

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

1
  • Right.i didn't knew the "inline thing".Thank you too. Jan 23, 2015 at 1:07
0

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')
        }
    });
}
0

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" />
0

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'; }
    }

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

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