0

I wanna open a div and close it with this function but it doesnt work where is the wrong part i couldnt fix it, can anyone help me ?

function Element(id)
{    
    if(document.getElementById(id).style.display = 'block')
    {
        document.getElementById(id).style.display = 'block';
    }
    else
    {
        document.getElementById(id).style.display = 'none';
    }
}
1
  • Please format your code correctly by clicking the Format Code button in the toolbar. Commented Dec 17, 2010 at 17:39

5 Answers 5

3

Your if statement assigns the property instead of comparing it.
Change = to ===.

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

2 Comments

Just to clarify @SLaks' answer, = is an assignment operator (e.g. foo = "bar"). == and === are comparison operators (e.g. if ( foo == "bar").
Specifically, the = in the if(...) should be changed, but the others shouldn't.
3

I would suggest you rewrite it with the ternary operator, its much more readable and maintainable:

var element = document.getElementById(id);
element.style.display = element.style.display === 'block' ? 'none' : 'block';

2 Comments

Move the ? and the : up a line or you could be a victim of semicolon insertion.
@indieinvader: I don't think semicolon insertion would have occured in his first version, not even if he did put a return before the statement.
1
function Element(id) { 
        if( document.getElementById(id).style.display == 'block') {
             document.getElementById(id).style.display = 'block';
       } else { document.getElementById(id).style.display = 'none'; }
} 

Comments

1

Thank you ! This one worked.! but I changed the code inside of if. Because it says if you find block which should be none.

function Element(id) { 
    if( document.getElementById(id).style.display == 'none') {
      document.getElementById(id).style.display = 'block';
    } else { 
      document.getElementById(id).style.display = 'none'; 
    } 
}

Comments

0

You don't need an else or a ternary operator as your first condition has no effect. So this will suffice...

if(document.getElementById(id).style.display != 'block')
{
    document.getElementById(id).style.display = 'none';
};

1 Comment

Ah, I see your own answer has the action reversed so this no longer applies.

Your Answer

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

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.