Note that there are some explanatory texts on larger screens.

plurals
  1. PO
    text
    copied!<p>You have a number of errors in your code; I'll try to go through them one-by-one to clarify those problems and offer solutions that should, hopefully, lead to better understanding (and practice) in future.</p> <p>First, an <code>id</code> "...assigns a name to an element. This name must be unique in a document."<sup>1</sup>. Note the 'must,' which means that a document with a duplicated <code>id</code> (more than one element <em>sharing the same</em> <code>id</code> value makes the document invalid, and causes problems with JavaScript, since it will only <em>ever</em> look for <em>one</em> element with a given <code>id</code>, and recover from invalid documents with unpredictable results).</p> <p>That being the case I've amended the <code>id</code>s of the <code>div</code> elements by, effectively, adding the <code>Content</code> string, and amended the <code>id</code>s of the <code>li</code> elements to be single word and lower-case so that they can be predictably made to reference each other. This gives the following HTML:</p> <pre><code>&lt;nav id="nav"&gt; &lt;ul class="main_nav"&gt; &lt;li id="about"&gt;&lt;a href="#" onclick="About_Me_Sel()"&gt;About Me&lt;/a&gt;&lt;/li&gt; &lt;li id="home"&gt;&lt;a href="#" onclick="Home_Sel()"&gt;Home&lt;/a&gt;&lt;/li&gt; &lt;/ul&gt; &lt;/nav&gt; &lt;div id="aboutContent"&gt; &lt;p&gt;Hello&lt;/p&gt; &lt;/div&gt; &lt;div id="homeContent"&gt; &lt;p&gt;hi&lt;/p&gt; &lt;/div&gt; </code></pre> <p><a href="http://jsfiddle.net/davidThomas/pvJhM/" rel="nofollow noreferrer">JS Fiddle</a> (this still doesn't work as you'd intend, because the <em>other</em> problems still exist; it's merely to show the corrected/amended HTML).</p> <p>Now, the JavaScript.</p> <p>The reason it <em>can't</em> work, as noted by @Jeffman in <a href="https://stackoverflow.com/a/18324547/82548">his answer</a> is because <code>element.style</code> references only the in-line styles of an element (those set with the <code>style</code> attribute), not the styles set with either a stylesheet or in the head of the document. This means you're comparing an <code>undefined</code> variable with a string, which will always be <code>false</code>.</p> <p>You're <em>also</em> using two functions to do the same thing, which is wasteful and unnecessary. This is the second reason I've modified the <code>id</code>s of the various elements. A modified (single) function to do what you want to do:</p> <pre><code>function sel(e, self) { // e is the click event, // self is the 'this' DOM node var id = self.parentNode.id, toggle = document.getElementById(id + 'Content'), display = toggle.style.display; toggle.style.display = display == 'block' ? 'none' : 'block'; } </code></pre> <p>The above requires the following HTML for the <code>a</code> elements:</p> <pre><code>&lt;a href="#" onclick="sel(event, this)"&gt;About Me&lt;/a&gt; </code></pre> <p><a href="http://jsfiddle.net/davidThomas/pvJhM/2/" rel="nofollow noreferrer">JS Fiddle demo</a>.</p> <p>Now, this <em>still</em> requires obtrusive JavaScript (using in-line event-handling within the HTML itself, which requires updates every time you may want to add further event-handling or change the function to call upon those events). Therefore I'd suggest moving to a more unobtrusive version, such as:</p> <pre><code>function sel(e, self) { // e is the click event, // self is the 'this' DOM node var id = self.parentNode.id, toggle = document.getElementById(id + 'Content'), display = toggle.style.display; toggle.style.display = display == 'block' ? 'none' : 'block'; } var links = document.getElementById('nav').getElementsByTagName('a'); for (var i = 0, len = links.length; i &lt; len; i++) { links[i].onclick = function (e){ sel(e, this); }; } </code></pre> <p><a href="http://jsfiddle.net/davidThomas/pvJhM/3/" rel="nofollow noreferrer">JS Fiddle demo</a>.</p> <p>Now, while this works, this still requires assigning event-handlers to multiple elements (despite being more easily done using a loop within JavaScript itself).</p> <p>The easier way is to delegate the event-handling to the parent element, and assess, in the function itself, where the event originated. Which would give the following JavaScript:</p> <pre><code>function sel(e) { // e is the click event, // self is the 'this' DOM node var self = e.target, id = self.parentNode.id, toggle = document.getElementById(id + 'Content'), display = toggle.style.display; toggle.style.display = display == 'block' ? 'none' : 'block'; } var nav = document.getElementById('nav'); nav.addEventListener('click', sel); </code></pre> <p><a href="http://jsfiddle.net/davidThomas/pvJhM/4/" rel="nofollow noreferrer">JS Fiddle demo</a>.</p> <hr /> <p>Notes:</p> <ol> <li><a href="http://www.w3.org/TR/html401/struct/global.html#h-7.5.2" rel="nofollow noreferrer">http://www.w3.org/TR/html401/struct/global.html#h-7.5.2</a>.</li> </ol> <p>References:</p> <ul> <li><a href="https://developer.mozilla.org/en-US/docs/Web/API/EventTarget.addEventListener?redirectlocale=en-US&amp;redirectslug=DOM%2FEventTarget.addEventListener" rel="nofollow noreferrer"><code>addEventListener()</code></a>.</li> <li><a href="https://developer.mozilla.org/en-US/docs/Web/API/document.getElementById" rel="nofollow noreferrer"><code>document.getElementById()</code></a>.</li> <li><a href="https://developer.mozilla.org/en-US/docs/Web/API/element.getElementsByTagName" rel="nofollow noreferrer"><code>getElementsByTagName()</code></a>.</li> <li><a href="https://developer.mozilla.org/en-US/docs/Web/API/Node.parentNode" rel="nofollow noreferrer"><code>Node.parentNode</code></a>.</li> </ul>
 

Querying!

 
Guidance

SQuiL has stopped working due to an internal error.

If you are curious you may find further information in the browser console, which is accessible through the devtools (F12).

Reload