What is wrong with this javascript if statement

Question

Javascript Function to show/hide tabs and iFrames:

function ChangeStep(id)
{
    var i = 1;
    // hide all other tabs:
    while(i<3) {
        var divID = 'tabs' + i;
        if (divID !== null) {
            document.getElementById(divID).className = " hide";
        }
        i++;
    }

    // show this one
    document.getElementById(id).className = " show";
}

if (id == "tab2") {
    document.getElementById(iFrame).className = " iFrame2";
}
else if (id == "tab1") {
    document.getElementById(iFrame).className = " iFrame1";
}

The change of tab works, but the if statement at the bottom doesn't seem to work.

EDIT

The issue is that I have an iFrame with the id of 'iFrame'. Now I made 2 Classes in the css file called: 'iFrame1' and 'iFrame2' which have different settings and which make the iFrame look different. The function above has no problem in changing the 'tabs' (Add the class 'Show' to one and 'Hide' to all others). But it doesn't seem to change the iFrames class to 'iFrame2' and/or 'iFrame1' I can't put it on JSFiddle because my site heavily relies on images, so I'll just link you to where I have it uploaded: www.FeedtheSyrians.com

can you create a jsfiddle for this, would help us understand more of what the issue is — Kevin Nacios
– Kevin Nacios, Commented Jun 2, 2013 at 23:40
var divID = 'tabs' + i; if (divID !== null)... How would you expect it to be null when you've just assigned a string to it? You're supposed to perform document.getElementById first, or just remove that if statement altogether. — Fabrício Matté
– Fabrício Matté, Commented Jun 2, 2013 at 23:40
If you indent properly you will find it easier to debug. jsbeautifier.org — Paul S.
– Paul S., Commented Jun 2, 2013 at 23:43
Please don't edit your question so that the answers no longer make sense. (Hence the rollback) — Lee Taylor
– Lee Taylor, Commented Jun 2, 2013 at 23:57

Lerp · Accepted Answer · 2013-06-02 23:40:52Z

5

Your if statement isn't inside of the function body meaning it is not executed when the function is called.

I don't know when it will be executed. I'm no JS expert and I don't want to make an (educated) guess, maybe someone with more knowledge can say.

answered Jun 2, 2013 at 23:40

Lerp

3,1773 gold badges29 silver badges45 bronze badges

Sign up to request clarification or add additional context in comments.

1 Comment

Shivam Amin Over a year ago

Ok, I put the if statement in the function (Was changing it around a few minutes ago, and didn't notice I put it outside), but it still doesn't work.

Pebbl · Accepted Answer · 2013-06-03 07:39:48Z

1

You don't seem to be defining the variable iFrame anywhere?

function ChangeStep(id)
{
  var iframeClass, iFrame = '???', divID, i;
  // hide all other tabs:
  for ( i=1; i<3; i++ ) 
  {
    divID = 'tabs' + i;
    if (divID != id) 
    {
      document.getElementById(divID).className = 'hide';
    }
  }
  // show this one
  document.getElementById(id).className = 'show';
  // show the iframe
  switch (id) 
  {
    case 'tab2': iframeClass = 'iFrame2'; break;
    case 'tab1': iframeClass = 'iFrame1'; break;
  }
  if ( iframeClass ) {
    document.getElementById(iFrame).className = iframeClass;
  }
}

Also would you not want different iframes to be hidden and shown when a different tab is used? Rather than just applying a class to a singular iframe? Anyway, just my two cents.

update

From what I've seen of the site you've linked to you should be using this line of code:

document.getElementById('iFrame').className = iframeClass;

Either that or you should define a variable before that, as I've done in my example above, and set iFrame = 'iFrame' rather than '???'.

On an entirely separate note I would avoid using that favicon for your site ;)

update 2

The problem is down to this part:

switch (id) 
{
  case 'tab2': iframeClass = 'iFrame2'; break;
  case 'tab1': iframeClass = 'iFrame1'; break;
}

It seems from looking at your site that you actually need:

switch (id) 
{
  case 'tabs2': iframeClass = 'iFrame2'; break;
  case 'tabs1': iframeClass = 'iFrame1'; break;
}

note the added 's' to make 'tabs1' and 'tabs2'

edited Jun 3, 2013 at 7:39

answered Jun 2, 2013 at 23:58

Pebbl

36.1k6 gold badges66 silver badges65 bronze badges

4 Comments

Shivam Amin Over a year ago

If I use 2 iFrames, then the Form will be different. I'm trying to get them to fill out the 'Sign Petition' form on Change.org by showing them a part of it at a time.

Pebbl Over a year ago

@ShivamAmin Sure thing, now that I've seen the site it makes more sense what you are trying to do. My update should fix the issue you are having.

Shivam Amin Over a year ago

Ok, I changed the Function to what you said, but the iFrame still looks the same. I know my Css is fine because when I manually change the class name, it changes it. I changed the '???' and changed it to 'iFrame' after copying your code, but I can't get it to work.

Pebbl Over a year ago

@ShivamAmin it seems the problem is down to an typo in your original code. You should be checking for 'tabs1' and 'tabs2', not 'tab1' and 'tab2' (see my update above). The best way to debug problems like this is, if in doubt, console.log all the values you are dealing with... the console is your friend :)

Bergi · Accepted Answer · 2013-06-02 23:52:41Z

What is wrong with these javascript if statements?
var divID = 'tabs' + i;
if (divID !== null)

It's useless. divID is always a string and never null. You probably wanted to test whether document.getElementById(divID) did return an element or null.

}
if (id == "tab2")

It's outside the function body, and the id variable probably will be undefined.

Better:

function ChangeStep(id) {
    for (var i=1; i<3; i++) {
        var div = document.getElementById('tabs' + i);
        if (div !== null)
            div.className = "show"; // or += " show";
                                    // and didn't you want to hide these?
    }
    // show this one
    document.getElementById(id).className = "show";
    // is the `iFrame` variable defined somewhere?
    if (id == "tab2")
        document.getElementById(iFrame).className = "iFrame2";
    else if (id == "tab1") {
        document.getElementById(iFrame).className = "iFrame1";
}

Scott Selby · Accepted Answer · 2013-06-02 23:52:55Z

0

You had 2 problems , the closing bracket for the function was before the if statement , also I dont see anywhere that id is defined. you are getting element by id, but unless it is dfined somwhere else that is probaby the problem

function ChangeStep(id)
{
var i = 1;
// hide all other tabs:
while(i<3) {
    var divID = 'tabs' + i;
    if (divID !== null) {
   document.getElementById(divID).className = " hide";
    }
    i++;
    }

// show this one
document.getElementById(id).className = " show";
//} -- remove this
    if (id == "tab2") {
        document.getElementById(iFrame).className = " iFrame2";
    }
    else if (id == "tab1") {
        document.getElementById(iFrame).className = " iFrame1";
    }
} //add this

edited Jun 2, 2013 at 23:52

answered Jun 2, 2013 at 23:42

Scott Selby

9,58012 gold badges63 silver badges97 bronze badges

1 Comment

Shivam Amin Over a year ago

This is how it gets the id 'onClick="ChangeState(tabs1)" and onClick="ChangeState(tabs2)"'

Collectives™ on Stack Overflow

What is wrong with this javascript if statement

EDIT

4 Answers 4

1 Comment

update

update 2

4 Comments

Comments

1 Comment

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

EDIT

4 Answers 4

1 Comment

update

update 2

4 Comments

Comments

1 Comment

Your Answer

Sign up or log in

Post as a guest

Related