0

I have a series of divs that I've hidden with css display:none;.

<div id="" class="hidden_row">some code</div>
<div id="" class="hidden_row">some code</div>
<div id="" class="hidden_row">some code</div>

I also have button with an onclick "addRow();" function that is designed to sequentially display one hidden row on each click by changing its css declaration to "display:block;". The javascript looks like this:

function addRow(){
    var hiddenrow = getElementsByClassName(document, "*", "hidden_row");

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

However, the function doesn't change 1 member of the array on execution (which is what I'd like), it finds the hidden rows and displays them all. How can I tweak my function so that it works as intended?

kjarsenal
  • 934
  • 1
  • 12
  • 35

4 Answers4

5

you have a typo in your if statement

if(hiddenrow[i].style.display = "none"){

should be

if(hiddenrow[i].style.display == "none"){

EDIT: note that .style.display only checks the inline style, so you'll have to check for

if(hiddenrow[i].style.display != "block"){

instead, as this better supports what you want

and as Darhazer said, if you want to just show one at a time, you need to put a break; after you find the one you want.

Working JsFiddle: http://jsfiddle.net/STGhq/

and your revised function

function addRow(){
    var hiddenrow = document.getElementsByClassName("hidden_row");
    for(var i=0; i< hiddenrow.length; i++){
        if(hiddenrow[i].style.display != "block"){
            hiddenrow[i].style.display = "block";
            break;
        }
    }
}​


Addressing the issue that element.style only pulls the current inline style, I found this article that gets around it by using window.getComputedStyle or element.currentStyle, both of which pull in the current computed style, rather that just inline declarations.

Working jsFiddle with the new code http://jsfiddle.net/STGhq/2/

Thomas Jones
  • 4,892
  • 26
  • 34
  • actually, when I change the code as you suggest, it ceases to function at all. – kjarsenal May 13 '12 at 21:24
  • @kjarsenal - Because `var hiddenrow = getElementsByClassName(document, "*", "hidden_row");` should be `document.getElementsByClassName`. – Derek 朕會功夫 May 13 '12 at 21:28
  • and I don't any documentation anywhere that supports getElementsByClassName with 3 parameters – Thomas Jones May 13 '12 at 21:29
  • added document. (that's embarrassing). code still doesn't work, possibly because what Kieran is pointing at. Using a custom getElementsByClassName function because I thought javascripts generic getElementsByClassName had limited browser support. maybe I'll just go back to the generic implementation. – kjarsenal May 13 '12 at 21:34
  • @Kjarsenal you're not wrong....http://caniuse.com/#search=getElementsByClassName it seems IE8 and below dont support it well. – Thomas Jones May 13 '12 at 21:39
  • I salute you sir! Thank you for help; very appreciated. – kjarsenal May 13 '12 at 21:40
  • http://stackoverflow.com/questions/10484467/ie-8-object-doesnt-support-property-or-method-getelementsbyclassname has a polyfill for the ie family – Thomas Jones May 13 '12 at 21:40
  • @kjarsenal - it's worth running your JS code through a tool such as [JSHint](http://jshint.com), which will pick up errors like the single-equal/double-equal bug. – Spudley May 14 '12 at 17:56
1

First, correct your code, as you are assigning a value in the if, not checking the equality. Next, if you can break the loop, so only the first element, which display is "none" will be shown.

for(var i=0; i< hiddenrow.length; i++){
        if(hiddenrow[i].style.display == "none"){
            hiddenrow[i].style.display = "block";
            break;
        }
    }
Maxim Krizhanovsky
  • 26,265
  • 5
  • 59
  • 89
1

Hmm, did you mis-type '==' as '=' in the condition statement?

John Fisher
  • 22,355
  • 2
  • 39
  • 64
0

If you only want this to apply to the first row, and only if it's display is "none", then don't loop. Instead, just use the first item in the array, like so:

if (hiddenrow.length > 0 && hiddenrow[i].style.display == "none"){
    hiddenrow[0].style.display = "block";
}

OTOH, if you just want to apply this to the first item with display = "none", regardless of whether it's the first item in the list, then do:

for(var i=0; i< hiddenrow.length; i++){
    if(hiddenrow[i].style.display == "none"){
        hiddenrow[i].style.display = "block";
        break;
    }
}

The break operator stops the loop from continuing.

Benjamin Cox
  • 6,090
  • 21
  • 19