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?

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 '15 at 00:24
i declared all the in the head the error remains after i put it after the still the error remains .so? — nikolaosmparoutis, Jan 23 '15 at 00: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 '15 at 00: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 — nikolaosmparoutis, Jan 23 '15 at 00:42

score 9 · Accepted Answer · edited May 23 '17 at 12:02

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/

edited May 23 '17 at 12:02

Community

1
1

answered Jan 23 '15 at 00:39

nnnnnn

147,572
30
200
241

You are Right.That was a really weird error. Thanks. Did my comment on Css helped you to find this? – nikolaosmparoutis Jan 23 '15 at 00:59
No, I didn't see that comment until after putting the answer up. – nnnnnn Jan 23 '15 at 01: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 – nikolaosmparoutis Jan 23 '15 at 01: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. – Chinmay Ghule Oct 27 '21 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 '21 at 08:04
@nnnnnnn I didn't know that you can only change inline styles like this, but for external styles we have to use `getComputedStyle()`. Should we just always use `getComputedStyle()` since then it wouldn't matter whether the element has an inline style or an external one? – Chinmay Ghule Oct 29 '21 at 09:31

score 1 · Answer 2 · answered Jan 23 '15 at 01:09

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>

answered Jan 23 '15 at 01:09

André Werlang

5,839
1
35
49

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."? – nikolaosmparoutis Jan 23 '15 at 01: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. – André Werlang Jan 23 '15 at 01: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. – André Werlang Jan 23 '15 at 01:26
Ah, glad to see that you are bound to pure JS. My snippet is pure JS. – André Werlang Jan 23 '15 at 01:29
Why to break again?What are you thinking? Speak briefly just for informative purposes. – nikolaosmparoutis Jan 23 '15 at 01:30
@MilsZ for instance, your initial visibility configuration was based on CSS, and HTML element inheriting from its class. Then on-click you switched over to redefine element style (its own, not its class) on JS. Hope it is clearer now. – André Werlang Jan 23 '15 at 01:34
So you mean it is n't 100% right to use this approach and change temporary the Css style because the Basic class is still the same. I find on this lines some truth but my experience on JS/Css is only 1 month (university now).I learn. – nikolaosmparoutis Jan 23 '15 at 01:42
@MilsZ yes, but don't worry, you'll get it as you practice and try to solve problems like this. – André Werlang Jan 23 '15 at 01:48

score 0 · Answer 3 · answered Jan 23 '15 at 00:58

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

Use computed style

window.getComputedStyle(node_list[i]).display == "none"
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

Right.i didn't knew the "inline thing".Thank you too. – nikolaosmparoutis Jan 23 '15 at 01:07 — nikolaosmparoutis, Jan 23 '15 at 01:07

score 0 · Answer 4 · answered Jan 23 '15 at 01:22

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

score 0 · Answer 5 · answered Nov 29 '19 at 13:19

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

answered Nov 29 '19 at 13:19

Normajean

1,075
3
11
28

score 0 · Answer 6 · answered Aug 21 '21 at 11:11

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

Button does not function on the first click

6 Answers6

Linked

Related