Javascript onclick function with if and else not working the way its supposed to

Question

I'm trying to have a javascript function in my HTML page that shows more text whenever you click on a name, and then hides it if you click on the same name again or -eventually- shows different text when you click on another name. This is my HTML code:

<section>
    <p id="one" onclick="myFunctionOne()">Cheap table X800</p>
    <p id="two" onclick="myFunctionTwo()">Luxury table Z2100</p>
    <p id="three" onclick="myFunctionThree()">Chair 101</p>
</section>

And this is the Javascript:

<script>
    var oneS =  "<ul>"+"<li>ID: 200</li>"+"<li>Name: Cheap table X800</li>"+"<li>Description: This is the cheapest table you can find that still meets its goal. 4 people max.</li>"+"</ul>";
    var twoS =  "<ul>"+"<li>ID: 201</li>"+"<li>Name: Luxury table Z2100</li>"+"<li>Description: A long table, suitable for a meeting room.</li>"+"</ul>";
    var threeS =    "<ul>"+"<li>ID: 202</li>"+"<li>Name: Chair 101</li>"+"<li>Description: A very basic but functional chair.</li>"+"</ul>";
    var booleanOne=false;
    var booleanTwo=false;
    var booleanThree=false;
    function myFunctionOne() {
        //alert("outside if "+booleanOne);          
        if (!booleanOne){
            alert("inside if "+booleanOne);
                document.getElementById("one").innerHTML += oneS;
            booleanOne=true;
            alert(booleanOne);
            if (booleanTwo) myFunctionTwo();
            if (booleanThree) myFunctionThree();
            booleanOne=true;
            return;
        }
        //alert("outside else "+booleanOne);
        else if (booleanOne) {
            alert("inside else "+booleanOne);
            document.getElementById("one").innerHTML = "<p id="+"\"one\""+" onclick="+"\"myFunctionOne()\""+">Cheap table X800</p>";
            booleanOne=false;
            alert(booleanOne);
            return;
        }
    }
    function myFunctionTwo() {
        if (!booleanTwo){
                document.getElementById("two").innerHTML += twoS;
            booleanTwo=true;
            if (booleanThree) myFunctionThree();
            if (booleanOne) myFunctionOne();
        }
        else {
            document.getElementById("two").innerHTML = "<p id="+"\"two\""+" onclick="+"\"myFunctionTwo()\""+">Luxury table Z2100</p>";
            booleanTwo=false;
        }
    }
    function myFunctionThree() {
        if (!booleanThree){
                document.getElementById("three").innerHTML += threeS;
            booleanThree=true;
            if (booleanOne) myFunctionOne();
            if (booleanTwo) myFunctionTwo();
        }
        else{
            document.getElementById("three").innerHTML = "<p id="+"\"three\""+" onclick="+"\"myFunctionThree()\""+">Chair 101</p>";
            booleanThree=false;                 
        }
    }
</script>

I know this may not be the best way of doing things, but it's the first time I try to use Javascript/HTML. What I've been trying to do for testing purposes is to click on Cheap table X800 for three times in a row, and following the logic of the program what it should do is show the product description on the first click, close it on the second and then open it again on the third one. When I try to do it, though, the first two clicks work as expected but the third one - from what I get thanks to the alerts - calls myFunctionOne, goes inside the if statement, executes everything, goes inside the else statement and executes that bit of code too. This is only visible with the alerts stalling the script on the page, otherwise the third click seems to do nothing. How can I fix this?

Why do you need to add the onClick() code to the innerHTML when you replace it in the myFunctionx? — Simon Sellick
– Simon Sellick, Commented Mar 11, 2015 at 17:21
why do you call the other functions (myFunctionTwo, myFunctionThree) inside myFunctionOne? — Nikos M.
– Nikos M., Commented Mar 11, 2015 at 17:21
@NikosM. I check if the other two variables are true (meaning their bit of text is showing) and if it is call the function that should remove it. — Luca Giorgi
– Luca Giorgi, Commented Mar 11, 2015 at 17:25
@SimonSellick Actually when I add the text I'm trying to append it to the innerHTML, when instead I'm removing it I replace everything in that innerHTML with the line I had at the start - or at least that's what I'm trying to do, could that be the problem? — Luca Giorgi
– Luca Giorgi, Commented Mar 11, 2015 at 17:25
document.getElementById("one").innerHTML = "<p id="+"\"one\""+" onclick="+"\"myFunctionOne()\""+">Cheap table X800</p>";.it will create another p element inside p with id=one.instead just use document.getElementById("one").innerHTML = "Cheap table X800; — balajisoundar
– balajisoundar, Commented Mar 11, 2015 at 17:32

Greg Lafrance · Accepted Answer · 2015-03-11 18:09:06Z

Diego's answer was very clean and efficient, but while he was posting it, I was working on another solution, which is not as good as Diego's, but may have some illustrative benefits:

<html>
    <body>
        <section>
            <p id="one" onclick="mainFunc('one')">Cheap table X800</p>
            <ul id="oneList" style="display: none;">
                <li>ID: 200</li>
                <li>Name: Cheap table X800</li>
                <li>Description: This is the cheapest table you can find that still meets its goal. 4 people max.</li>
            </ul>
            <p id="two" onclick="mainFunc('two')">Luxury table Z2100</p>
            <ul id="twoList" style="display: none;">
                <li>ID: 201</li>
                <li>Name: Luxury table Z2100</li>
                <li>Description: A long table, suitable for a meeting room.</li>
            </ul>
            <p id="three" onclick="mainFunc('three')">Chair 101</p>
            <ul id="threeList" style="display: none;">
                <li>ID: 202</li>
                <li>Name: Chair 101</li>
                <li>Description: A very basic but functional chair.</li>
            </ul>
        </section>
        <script>
            function mainFunc(arg) {
                myFunctionOne(arg);
                myFunctionTwo(arg);
                myFunctionThree(arg);
            }
            function myFunctionOne(arg) {
                var list = document.getElementById('oneList');

                if ('one' == arg && list.style.display == 'none'){
                    list.style.display = 'block';
                }
                else {
                    list.style.display = 'none';
                }
            }
            function myFunctionTwo(arg) {
                var list = document.getElementById('twoList');

                if ('two' == arg && list.style.display == 'none'){
                    list.style.display = 'block';
                }
                else {
                    list.style.display = 'none';
                }
            }
            function myFunctionThree(arg) {
                var list = document.getElementById('threeList');

                if ('three' == arg && list.style.display == 'none'){
                    list.style.display = 'block';
                }
                else {
                    list.style.display = 'none';
                }
            }
        </script>
    </body>
</html>

Diego López · Accepted Answer · 2015-03-11 17:42:17Z

What you are trying is to concatenate a list inside a paragraph and then splitting it again at run-time, this is not the best way to hide and show a dom element and also semantically incorrect since is not valid to have a list inside a paragraph on the HTML specification. The following example is a cleaner way to achieve the results you are looking (try it here https://jsfiddle.net/p8gkvtkj/):

CSS:

#moreInfo1, #moreInfo2, #moreInfo3 {
 display: none;
}

JS:

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

HTML:

<section>
<div onclick="showOrHideMore('moreInfo1')"><p>Cheap table X800</p>
    <ul id="moreInfo1">
            <li>ID: 201</li>
            <li>Name: Cheap table X800</li>
            <li>Description: This is the cheapest table you can find that still meets its goal. 4 people max.</li>
    </ul>
</div>
<div onclick="showOrHideMore('moreInfo2')"><p>Luxury table Z2100</p>
    <ul id="moreInfo2">
            <li>ID: 202</li>
            <li>Name: Luxury table Z2100</li>
            <li>Description: This is the cheapest table you can find that still meets its goal. 4 people max.</li>
    </ul>
</div>
    <div onclick="showOrHideMore('moreInfo3')"><p>Chair 101</p>
    <ul id="moreInfo3">
            <li>ID: 203</li>
            <li>Name: Chair 101</li>
            <li>Description: This is the cheapest table you can find that still meets its goal. 4 people max.</li>
    </ul>
</div>
</section>

cpolito · Accepted Answer · 2015-03-11 17:53:33Z

1

This is clearly not the best way to do what you're trying to do. But to answer your immediate question for why it doesn't work: when you reset each dropdown to the original, use

document.getElementById("one").innerHTML = "Cheap table X800";

etc. instead of

document.getElementById("one").innerHTML = "<p id="+"\"one\""+" onclick="+"\"myFunctionOne()\""+">Cheap table X800</p>";

what you ended up doing was creating a paragraph tag inside the original instead of replacing it. It's how .innerHTML works in reality.

answered Mar 11, 2015 at 17:53

cpolito

763 bronze badges

Collectives™ on Stack Overflow

Javascript onclick function with if and else not working the way its supposed to

3 Answers 3

Comments

Comments

Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

3 Answers 3

Comments

Comments

Comments

Your Answer

Sign up or log in

Post as a guest

Related