Content switching with Javascript

Question

Hi!

I have 3 sections of specific interest on my home page. What I'd like to do is set up links that call a javascript function that makes sure 2 sections are hidden and ONLY the section whose button was clicked is displayed.

Here's my code:

            <div id="idc" class="leftFloat"><span id="title" class="title1">Introduction</span></div>
            <div class="rightFloat">
                <div id="agri"><a onclick="ContentSwitch('Agri');">Agri Industries</a></div>
                <div id="ict"><a onclick="ContentSwitch('ict');">ICT Investments</a></div>
                <div id="intro"><a onclick="ContentSwitch('intro');">Introduction</a></div>
            </div>

            <div id="agriContent" style="display: none;">
                <div class="vrtlay_both">AGRI INDUSTRIES</div>
            </div>

            <div id="ictContent" style="display: none;">
                <div class="vrtlay_both">ICT INVESTMENTS</div>
            </div>

            <div id="introContent">
                <div class="vrtlay_both">ICT INVESTMENTS</div>
            </div>

            <script type="text/javascript">
                function ContentSwitch(id) {
                    if (id = "Agri") {
                        if (document.getElementById("agriContent").style.display = "none") {
                            document.getElementById("agriContent").style.display = "block";

                            // Hide other content
                            document.getElementById("ictContent").style.display = "none";
                            document.getElementById("introContent").style.display = "none";

                            // Change the look of the title
                            document.getElementById("idc").style.backgroundColor = "rgb(0, 100, 0)";
                            document.getElementById("idc").style.color = "rgb(255, 255, 255)";
                            document.getElementById("title").innerHTML = "Agri Industries";
                        } else {
                            return;
                        }
                    }
                    if (id = "ict") {
                        if (document.getElementById("ictContent").style.display = "none") {
                            document.getElementById("ictContent").style.display = "block";

                            // Hide other content
                            document.getElementById("agriContent").style.display = "none";
                            document.getElementById("introContent").style.display = "none";

                            // Change the look of the title
                            document.getElementById("idc").style.backgroundColor = "rgb(36, 46, 111)";
                            document.getElementById("idc").style.color = "rgb(255, 255, 255)";
                            document.getElementById("title").innerHTML = "ICT Investments";
                        } else {
                            return;
                        }
                    }
                    if (id = "intro") {
                        if (document.getElementById("introContent").style.display = "none") {
                            document.getElementById("introContent").style.display = "block";

                            // Hide other content
                            document.getElementById("agriContent").style.display = "none";
                            document.getElementById("ictContent").style.display = "none";

                            // Change the look of the title
                            document.getElementById("idc").style.backgroundColor = "rgb(255, 255, 255)";
                            document.getElementById("idc").style.color = "rgb(0, 0, 0)";
                            document.getElementById("title").innerHTML = "Introduction";
                        } else {
                            return;
                        }
                    }
                }
            </script>

The javascript isn't firing.
I'm aware that this isn't the most elegant (or necessarily efficient) way of doing this, so if anyone can suggest a better way, I'm all ears.

Right now though, I'd just really like for this to work, but I can't see the problem.

Answer 1

Comparisons in Javascript use the double equals operator (==), not a single equals, so you're assigning the values in your if statements. Try changing them and see if it works.

Answer 2

<deep breath>

Ok...

  if (id = "Agri") { 

Well there's your actual problem. You're making an assignment = not a comparison ==.

Now to improve the code.

"id" is exclusive and a constant expression so you should arguably use a switch here, but you should at least be using elseif.

  if (document.getElementById("agriContent").style.display = "none") { 

Again, wrong operation here, but you're also failing to capture a reference to the element you're intertested in so you're losing performance when you run getElementById over and over again. Further, you're testing for a particular style property so you're tightly coupled to an implementation. This makes your code much less flexible and much more error prone. CSS classes are your friends: use them, change their application, leave style alone.

  } else { 
    return; 
  } 

Else do nothing? You're doing this to compensate for the lack of elseif statements in previous logic - this block is just confusing.

Answer 3

I'd suggest using CSS classes and jQuery to do this.

Instead of passing in a string value do something like this:

<div id="agri"><a onclick="ContentSwitch($("#agriContent"));">Agri Industries</a></div>
<div id="ict"><a onclick="ContentSwitch($("#ictContent"));">ICT Investments</a></div>
<div id="intro"><a onclick="ContentSwitch($("#introContent"));">Introduction</a></div>

In your content panels, do this:

    <div id="agriContent" class="invisible">
        <div class="vrtlay_both">AGRI INDUSTRIES</div>
    </div>

    <div id="ictContent" class="invisible">
        <div class="vrtlay_both">ICT INVESTMENTS</div>
    </div>

    <div id="introContent" class="visible">
        <div class="vrtlay_both">ICT INVESTMENTS</div>
    </div>

Then, in ContentSwitch, do something like this:

function ContentSwitch(div) {
      $(".visible").addClass("invisible").removeClass("visible");
      div.removeClass("invisible").addClass("visible");
}

Might not be perfect, but the best I can come up with off the top of my head.

Answer 4

This is just a suggestion but you could refactor your code a bit.

var agriContent, ictContent, introContent, idc, title;

function ContentSwitch(id) {
    agriContent = agriContent || document.getElementById("agriContent");
    ictContent = ictContent || document.getElementById("ictContent");
    introContent = introContent || document.getElementById("introContent");
    idc = idc || document.getElementById("idc");
    title = title || document.getElementById("title");          
    switch (id) {
        case "Agri":
            if (agriContent.style.display !== "none") {
                return;
            }
            agriContent.style.display = "block";
            ictContent.style.display = "none";
            introContent.style.display = "none";
            idc.style.backgroundColor = "rgb(0, 100, 0)";
            idc.style.color = "rgb(255, 255, 255)";
            title.innerHTML = "Agri Industries";
            break;
        case "ict":
            // ...
            break;
        case "intro":
            // ...
            break;
    }
}