2

Earlier, I did an assignment where I was supposed to write code in Javascript in order to toggle visibility for the submenus each belonging to their own topmenu in a navigation bar for a webpage. The visibility should be set to hidden by default and should be shown when a topmenu is clicked on. I know how to toggle visibility for ONE submenu belonging to a topmenu, but fail to make my code work for multiple elements. With help from here, I got my code to work. However, my teacher was not pleased over the fact that I used onclick in my HTML-code. So my question is now: How do I move all functionality to javascript, and thereby not use onclick in my HTML?

Note: Of course I gave it a try myself but I cannot make the pairing between header and div work correctly... By the pairing I mean that visibility of the div with the class "left_submenu_1" should be toggled when you click the topmenu "left_top1". Thus should the visibilily of the div with the class "left_submenu_2" be toggled when you click the topmenu "left_top2".

I guess I should start something like this:

var left_headings = document.getElementsByClassName("left_top");
for(var k = 0; k < left_headings.length; k++) {
   ??????
}

Earlier related question: Toggle visibility for multiple divs with one function: navigation bar

HTML

 <a class="left_top" onclick = "toggle('.left_submenu_1')">Opinion</a><br>
        <div class="left_submenu_1" style="display: none;">
        <a class="left_sub1">Leaders</a><br>
        <a class="left_sub1">Debates</a><br>
        </div> 
<br>

<a class="left_top" onclick = "toggle('.left_submenu_2')">Economy</a><br>
        <div class="left_submenu_2" style="display: none;">
        <a class="left_sub2">News</a><br>
        <a class="left_sub2">Your Economy</a><br>
        </div>

Javascript

function toggle(qs) {
   var e = document.querySelector(qs);
   e.style.display = e.style.display === 'block' ? 'none' : 'block';
}

Please note: We are NOT allowed to use jQuery or to give the topmenus id:s, as the idea is to use one general function to toggle the visibility.

Community
  • 1
  • 1
Isus
  • 143
  • 2
  • 16
  • 4
    I always disliked professors that forced you to "reinvent the wheel" on assignments * sigh*. No one would try to do this in a professional scenario lol. Sorry for the unhelpful comment, but brings back some nostalgia from coding classes @ uni. – Evan Bechtol May 20 '16 at 11:34
  • I agree completely! Though I can see the use of knowing how to do both inline coding and completely seperate HTML and javascript. BUT, if you manage to find a way that is easy and effective enough, I think that solution should suffice. There is _never_ only one solution to a problem. – Isus May 20 '16 at 11:42
  • I copy/pasted your code on jsfiddle but nothing happens so I can't tell what is wrong. Can you create fiddle that SHOULD be working in your opinion but has errors? – llamerr May 20 '16 at 11:42
  • That´s weird, it worked perfectly before... @Remysc suggestion is working though (https://jsfiddle.net/pkptn4gf/). The only problem there is that we are not allowed to give the topmenus ID:s – Isus May 20 '16 at 11:55
  • I don't think it's about "reinveinting the wheel", it's about teaching something specific, if you want to teach how to add event handlers dynamically, for example, you must first teach how to add event handlers, but adding them via HTML doesn't teach you how to do that. – Remysc May 20 '16 at 12:19
  • Sometimes it can be like "reinveinting the wheel", but I also see your point. I think it is most important to understand different solutions rather than to sit hour after hour figuring out a solution just to discover that "oh I could have read about that in this book and then used that knowledge to practically practice this method". The explanation as to _why_ we are using a specific method, is too often left out, in my opinion. – Isus May 20 '16 at 12:55

1 Answers1

2

EDIT: Changing the html was out of the question, updated answer.

Instead of using onclick to handle the event, assign the eventhandler via javascript, like this (Note that I added IDs to the elements in order to be able to select them properly):

jsfiddle: https://jsfiddle.net/pkptn4gf/

<a class="left_top">Opinion</a><br>
        <div class="left_submenu_1" style="display: none;">
        <a class="left_sub1">Leaders</a><br>
        <a class="left_sub1">Debates</a><br>
        </div> 
<br>

<a class="left_top">Economy</a><br>
        <div class="left_submenu_2" style="display: none;">
        <a class="left_sub2">News</a><br>
        <a class="left_sub2">Your Economy</a><br>
        </div>

function toggle(qs) {
   var e = document.querySelector(qs);
   e.style.display = e.style.display === 'block' ? 'none' : 'block';
}

var clickables = document.getElementsByClassName("left_top");

clickables[0].addEventListener("click", function(){
    toggle('.left_submenu_1');
});
clickables[1].addEventListener("click", function(){
    toggle('.left_submenu_2');
});
Remysc
  • 187
  • 6
  • I like your solution and would gladly use it, however, we are not allowed to give the topmenus ID:s which makes the task a little bit more difficult... – Isus May 20 '16 at 11:52
  • 1
    It kind of makes sense though, it prepares you for the case in which you can't touch the HTML, try this instead https://jsfiddle.net/pkptn4gf/1/ The idea is that instead of assigning IDs to the HTML, you get every link in an array and then add the events to the ones you want, a more elegant solution could be implemented with a for loop and it would work with whatever number of elements but it's a bit more complex Also, note that this solution requires the js to come AFTER the html content, otherwise it won't be able to find the nodes when it's generated. – Remysc May 20 '16 at 12:12
  • Thanks for the advice! I used your solution and everything seems to be working fine now. In this case, I have 8 topmenus in total, but only used 2 of them here, since using all 8 here would be confusing and it would be harder to read and sort out the info. I had a problem in an earlier task where I misplaced the js-tag in the HTML-document which caused the clicks to not register. Good that you mentioned that! Thanks for the help! – Isus May 20 '16 at 12:39
  • If you have to add event handlers to several elements, you might want (or your teacher might want you to) use a for loop with closures, but I don't know if you are familiar with how it works (I myself had to review for a while in order to make this) https://jsfiddle.net/pkptn4gf/2/ if you want to learn about closures or how that code works, I'd recommend [reading this](http://stackoverflow.com/questions/111102/how-do-javascript-closures-work), which is going to explain it better than what I possibly can, specifically the seventh example. – Remysc May 20 '16 at 14:36
  • I am a little familiar with it, though I have barely used it. It sounds interesting and like it would be useful to learn more about so I´m definitely going to read the link you provided. I really appreciate you taking the time to explain and create jsfiddles, so thank you for your further advice! :) – Isus May 21 '16 at 07:09