js line causes code above it not to execute

Question

Here's a pen with the full html: https://codepen.io/froggomad/pen/WLdzoB

I'm writing 2 functions - one to show hidden content, and one to hide it. I'm wanting the show() function to execute on the parent div and the hide() function to execute on the div with the selector .click-text.

However, I'm switching text on .click-text from show to hide so I don't want the hide function to remain on the text at all times. I also want it obvious that its interactive text when changing to a hide function, so I make it a link.

That's all well, but when attempting to set the onclick Attr of the parent back to the show() function, nothing in the hide block executes at all.

If I remove the line setting the parent's onclick Attr, the script executes as expected. If I set another element's onclick Attr, the script executes as expected.

However, with that line in there, nothing happens and there's no output in the console to indicate an error. I even set an alert with the type of element and classname to ensure I'm targeting the right element.

Get closest parent of element matching selector:

var getClosest = function (element, selector) {
    for ( ; element && element !== document; element = element.parentNode ) {
      if ( element.matches(selector) ) return element;
    }
    return null;
}

Show Hidden Element ul.service-category-menu

function show(elem) {    
    var menu = elem.querySelector("ul.service-category-menu"),
               click = elem.querySelector(".click-text"),
               parent = getClosest(elem, '.service-category');
    ;
    if (menu.style.display === "none" || menu.style.display === "") {
        menu.style.display = "block";
        click.innerHTML = "<a href=\"#\">Click to Hide<\/a>";
        click.setAttribute('onclick','hide(this);');
        elem.setAttribute('onclick', 'null');
    }
}

Hide Element

function hide(elem) {    
    var parent = getClosest(elem, '.service-category'),    
                menu = parent.querySelector("ul.service-category-menu"),
                click = parent.querySelector(".click-text")
    ;
    alert(parent + "\n" + parent.className);
    //Outputs div element with expected class name (class name is unique on each div)
    if (menu.style.display === "block") {
        menu.style.display = "none";
        click.innerHTML = "Click to Show";
        click.setAttribute('onclick', 'null');
        //the above lines don't execute when the following line is in place. There's no error in console.
        parent.setAttribute('onclick','show(this)');
    }
}

Answer 1

First off, I must confess that I'm against using onclick attributes. If you're not using a framework such as VueJS or React, I think HTML and JS should remain separated for better control and maintainability.

You can use addEventListener, removeEventListener, and e.stopPropagation() to avoid triggering multiple event handlers.

Events have two phases:

• Event capture: the event spreads from the document all the way down to the target element.

  • To catch an event during this phase, do:

    elm.addEventListener('click', myFunc, true);
    

• Event bubbling: the event bounces back from the target to the document.

  • To catch an event during this phase, do:

    elm.addEventListener('click', myFunc, false); /* or just omit the 3rd param */
    

Using e.stopPropagation() allows you to break that chain.

// When the DOM is ready
window.addEventListener("DOMContentLoaded", init);

function init() {
  // Get all categories
  var $categories = document.querySelectorAll(".service-category");
  // For each of them
  Array.from($categories).forEach(function($category) {
    // Add an event listener for clicks
    $category.addEventListener("click", show);
  });
}

function getClosest(element, selector) {
  for (; element && element !== document; element = element.parentNode) {
    if (element.matches(selector)) return element;
  }
  return null;
}

function show(e) {
  var $menu  = this.querySelector("ul.service-category-menu"),
      $click = this.querySelector(".click-text");
  if (["none", ""].includes($menu.style.display)) {
    $menu.style.display = "block";
    $click.innerHTML = '<a href="#">Click to Hide</a>';
    $click.addEventListener("click", hide);
    // Remove the `show` event listener
    this.removeEventListener("click", show);
  }
  e.stopPropagation();
}

function hide(e) {
  var $parent = getClosest(this, ".service-category"),
      $menu   = $parent.querySelector("ul.service-category-menu"),
      $click  = $parent.querySelector(".click-text");
  if (!["none", ""].includes($menu.style.display)) {
    $menu.style.display = "none";
    $click.innerHTML = "Click to Show";
    $click.removeEventListener("click", hide);
    $parent.addEventListener("click", show);
  }
  e.stopPropagation();
}

.service-category{display:inline-block;border:3px solid #ccc;margin:1%;font-weight:700;font-size:3.5vw;cursor:pointer;background-color:#fff;z-index:3;background-position:center;background-size:cover;color:#000}.click-text{text-align:right;font-size:1.25vw;font-style:italic;font-weight:700;padding-right:1%}.service-category:hover .click-text{color:#b22222}.service-category-menu{display:none;margin-left:8%;margin-right:8%;margin-top:1%;background-color:#fff;font-weight:700;font-size:1.6vw;border-radius:10px}

<div class="service-category web-back" id="web-back">
  <div class="row-overlay">
    Web <br /> Development
    <div class="click-text">Click to Show</div>
    <ul class="service-category-menu web">
      <li>
        Some text...
      </li>
    </ul>
  </div>
</div>

<div class="service-category web-front" id="web-front">
  <div class="row-overlay">
    Web <br /> Design
    <div class="click-text">Click to Show</div>
    <ul class="service-category-menu web">
      <li>
        Some text...
      </li>
    </ul>
  </div>
</div>

Answer 2

It is executed, it's just after you click that Click to Hide, the event continues to parent and the event handler of the parent executed. Thus, what exactly happen is (with that line), after hide() called, you inadvertently called show().

In javascript it's usually called bubbles (when you click the children, the click handler of parent will also be executed after click handler of children complete).

So the solution, you can add this line at the end of the hide() function

event.stopPropagation();

To stop the event from continuing to the parent