2

I have an expandable menu with nested submenus to an infinite depth.

<ul>
    <li>One</li>
    <li class="contains-submenu">Two
        <ul>
            <li>Two A</li>
            <li class="contains-submenu">Two B
                <ul>
                    <li>Two B I</li>
                    <li>Two B II</li>
                    <li>Two B III</li>
                </ul>
            </li>
            <li>Two C</li>
        </ul>
    </li>
    <li>Three</li>
    <li class="contains-submenu">Four
        <ul>
            <li>Four A</li>
            <li class="contains-submenu">Four B
                <ul>
                    <li>Four B I</li>
                    <li>Four B II</li>
                    <li>Four B III</li>
                </ul>
            </li>
            <li>Four C</li>
        </ul>
    </li>
</ul>

I want to show / hide any sub-menu at any level, without influencing the show / hide status of any other sub-menu and especially not the show / hide status of any ancestor of the sub-menu.

What I found initially was that if I used a click event listener to toggle a class .show-submenu to show or hide a submenu, when a submenu lower down (e.g. Two B) was closed, then its parent, the submenu Two, would also close.

It became apparent that this was happening because at almost the same moment I was clicking on Two B, a click event was also registering on Two (...of course it was - Two B is nested inside Two - any click on Two B will also register as a click on Two).

I tried to fix this using .stopPropagation()... but then I realised that actually this wasn't (at least I don't think it was) a case of a single event registering on two different elements as the event bubbled up through Two from Two B, but two different events registering, one on Two almost immediately after the other on Two B.

My solution was the following:

var activateToggle = true;

function toggleSubmenu() {
    if (activateToggle === true) {
        this.classList.toggle('show-submenu');
        activateToggle = false;
    }

    setTimeout(function(){activateToggle = true}, 100);
}

Is this a reasonable approach to produce the desired effect?

Or is there a more conventional approach using event.target or similar?

Complete Working Example:

var querySelector = 'ul > li > ul';
var selectedElements = [... document.querySelectorAll(querySelector)];
var activateToggle = true;
var initialPadding = 6;
var subsequentPadding = 18;
var level = 0;


while (selectedElements.length > 0) {

    for (var j = 0; j < selectedElements.length; j++) {
        selectedElements[j].parentNode.classList.add('contains-submenu');
        selectedElements[j].style.marginLeft = (0 - (initialPadding + (level * subsequentPadding))) + 'px';
        [... selectedElements[j].children].forEach(function(childElement){
            childElement.style.paddingLeft = (initialPadding + ((level + 1) * subsequentPadding)) + 'px';
        });
    }

    level++;
    querySelector += ' > li > ul';
    selectedElements = [... document.querySelectorAll(querySelector)];
}

var containsSubmenus = document.getElementsByClassName('contains-submenu');


function toggleSubmenu() {
    if (activateToggle === true) {
        this.classList.toggle('show-submenu');
        activateToggle = false;
    }

    setTimeout(function(){activateToggle = true}, 100);
}


for (let i = 0; i < containsSubmenus.length; i++) {
    containsSubmenus[i].addEventListener('click', toggleSubmenu, false);
}
ul {
margin-left: 0;
padding-left: 0;
font-family: arial, helvetica, sans-serif;
color: rgb(255, 255, 255);
background-color: rgba(0, 75, 165, 1);
list-style-type: none;
}

li {
padding-left: 6px;
font-size: 16px;
line-height: 32px;
}

ul ul {
display: none;
}

.show-submenu > ul {
display: block;
}

li.show-submenu {
height: auto;
}

.contains-submenu {
font-weight: 700;
cursor: pointer;
}

.contains-submenu ul {
font-weight: 300;
cursor: default;
}
.contains-submenu,
.contains-submenu ul {
background-color: rgba(255, 255, 255, 0.1);
}
<ul>
    <li>One</li>
    <li class="contains-submenu">Two
        <ul>
            <li>Two A</li>
            <li class="contains-submenu">Two B
                <ul>
                    <li>Two B I</li>
                    <li>Two B II</li>
                    <li>Two B III</li>
                </ul>
            </li>
            <li>Two C</li>
        </ul>
    </li>
    <li>Three</li>
    <li class="contains-submenu">Four
        <ul>
            <li>Four A</li>
            <li class="contains-submenu">Four B
                <ul>
                    <li>Four B I</li>
                    <li>Four B II</li>
                    <li>Four B III</li>
                </ul>
            </li>
            <li>Four C</li>
        </ul>
    </li>
</ul>
Rounin
  • 27,134
  • 9
  • 83
  • 108

1 Answers1

2

Here's a solution combining event.target and event.stopPropagation.

You can check whether the event.target actually contains a submenu and if so, toggle it and stop the event's propagation.

Is that the expected behaviour?

var querySelector = 'ul > li > ul';
var selectedElements = [... document.querySelectorAll(querySelector)];
var initialPadding = 6;
var subsequentPadding = 18;
var level = 0;


while (selectedElements.length > 0) {

    for (var j = 0; j < selectedElements.length; j++) {
        selectedElements[j].parentNode.classList.add('contains-submenu');
        selectedElements[j].style.marginLeft = (0 - (initialPadding + (level * subsequentPadding))) + 'px';
        [... selectedElements[j].children].forEach(function(childElement){
            childElement.style.paddingLeft = (initialPadding + ((level + 1) * subsequentPadding)) + 'px';
        });
    }

    level++;
    querySelector += ' > li > ul';
    selectedElements = [... document.querySelectorAll(querySelector)];
}

var containsSubmenus = document.getElementsByClassName('contains-submenu');


function toggleSubmenu(event) {     
    if (event.target.classList.contains('contains-sublist')) {
     event.target.classList.toggle('show-sublist');
     event.stopPropagation();
    } else {
     console.log("LEAF"); 
    }
}


for (let i = 0; i < containsSubmenus.length; i++) {
    containsSubmenus[i].addEventListener('click', toggleSubmenu, false);
}
ul {
margin-left: 0;
padding-left: 0;
font-family: arial, helvetica, sans-serif;
color: rgb(255, 255, 255);
background-color: rgba(0, 75, 165, 1);
list-style-type: none;
}

li {
padding-left: 6px;
font-size: 16px;
line-height: 32px;
}

ul ul {
display: none;
}

.show-submenu > ul {
display: block;
}

li.show-submenu {
height: auto;
}

.contains-submenu {
font-weight: 700;
cursor: pointer;
}

.contains-submenu ul {
font-weight: 300;
cursor: default;
}
.contains-submenu,
.contains-submenu ul {
background-color: rgba(255, 255, 255, 0.1);
}
<ul>
    <li>One</li>
    <li class="contains-submenu">Two
        <ul>
            <li>Two A</li>
            <li class="contains-submenu">Two B
                <ul>
                    <li>Two B I</li>
                    <li>Two B II</li>
                    <li>Two B III</li>
                </ul>
            </li>
            <li>Two C</li>
        </ul>
    </li>
    <li>Three</li>
    <li class="contains-submenu">Four
        <ul>
            <li>Four A</li>
            <li class="contains-submenu">Four B
                <ul>
                    <li>Four B I</li>
                    <li>Four B II</li>
                    <li>Four B III</li>
                </ul>
            </li>
            <li>Four C</li>
        </ul>
    </li>
</ul>

EDIT : answers to your questions

The way you attach event listeners does two things :

  1. Since you're listening for clicks on elements containing a submenu, each element in a sublist will fire 1 event per ancestor. For instance, a click on "Two B II" (which has two 'contains-submenu' ancestor) will fire two events. And you don't want to toggle any flag twice.
  2. You are bubbling up. So, once you click on a "leaf" (an element that doesnt contain a submenu) you will be handling the event for the leaf element first. And you don't want to do anything when handling the click event on such an element. Instead you want it to bubble up.

For theses reasons, I'm checking for the classList before toggling the sublist and stopping the propagation. That way :

  1. We only want to stop the event's propagation when its target is a 'contains-submenu' element to prevent the bubbling up to another 'contains-submenu' ancestor.
  2. We don't want to prevent the bubbling when the target is a "leaf" element.
  3. If the event target is not a "contains-submenu" element, we don't want to toggle any sublist.
Guillaume Georges
  • 3,878
  • 3
  • 14
  • 32
  • Wow. That's very good. You've managed to get rid of the activateToggle boolean flag. But can you explain how is this working by simply using an `if` condition about the `classList`? Why won't `event.stopPropagation();` work outside the condition? – Rounin Oct 24 '17 at 15:03
  • 2
    The answers weren't that simple so I answered in an edit. I hope I made myself clear enough :) – Guillaume Georges Oct 24 '17 at 15:28
  • Ah... I think I get it. **event.target** _isn't_ (necessarily) the element which has the event listener added to it? I thought that in this situation `event.target` and `this` were essentially the same. Instead, in this situation, `event.target`is the deepest element clicked on - even if that's just a leaf. Is that right? – Rounin Oct 24 '17 at 15:52
  • 1
    That's right. event.target is the element the event was triggered from (the deepest). Not to be confused with event.currentTarget https://stackoverflow.com/questions/5921413/difference-between-e-target-and-e-currenttarget – Guillaume Georges Oct 24 '17 at 15:57
  • Many thanks, @RogerC - a much cleaner solution all round. – Rounin Oct 24 '17 at 16:04
  • 1
    No problem @Rounin. It was a fun one. Also, be careful when using the "this" keyword inside an event handler. It's actually a reference to event.currentTarget and often creates side effects. – Guillaume Georges Oct 24 '17 at 16:42