0

I'm making a simple accordion-like menu using plain JS. The algorithm is:

  1. When the users click one of li elements, add a class called .open to its sibling element called list seq="two" for expanding.
  2. If the targeted element's sibling hasn't the class .open, remove the same other level's .open class first, then expands the sibling.

A problem is the removeClass() function doesn't work when I set the function as a parameter like this:

register(name, fx) {
    name.forEach(cur => {
        cur.addEventListener('click', (e) => fx(e));
    })
} // when I set the event register like this ...

...

removeClass(name, className) {
    name.forEach(cur => {
        cur.classList.remove(className);
    })
} // removeClass() function doesn't work

The weird thing that I can't understand is, the other functions are working fine such as toggling the .open class, or a static console function. Only this one, removeClass() doesn't work.

I'm going to use that register() in various ways so please don't answer me to remove the fx parameter unless this trick is officially illegal in JS.

I would like to know why this trick does't work in Class Syntax with an explanation and how to resolve this problem.

CodePen:

Class Syntax w/o fx

Class Syntax w/ fx

The Code (Class Syntax w/ fx):

'use strict';
class Mobile {
    constructor(elem) {
        this.elem = document.querySelector(elem);
        this.list = this.elem.querySelectorAll('li[data="hasChild"]');
        this.listSecond = this.elem.querySelectorAll('ul[seq="two"]');
        this.register(this.list, this.deploy);
    }
    register(name, fx) {
        name.forEach(cur => {
            cur.addEventListener('click', (e) => fx(e));
        })
    }
    removeClass(name, className) {
        name.forEach(cur => {
            cur.classList.remove(className);
        })
    }
    // static removeClass(name, className) {
    //     name.forEach(cur => {
    //         cur.classList.remove(className);
    //     })
    // }
    static staticFx(param) {
        console.log(param);
    }
    deploy(e) {
        Mobile.staticFx('test'); // This works
        let sib = e.target.nextElementSibling,
            isSib = sib.classList.contains('open'),
            ternary = !isSib ? {
                remove: this.removeClass(this.listSecond, 'open'), // But this doesn't work
                // static: Mobile.removeClass(this.listSecond, 'open'), // Static fx also doesn't work 
                toggle: sib.classList.toggle('open') // even this one works
            } : null;
    }
}
const proMob = new Mobile('.panel');
      * {
        margin: 0;
        padding: 0;
      }
      div, section {
        display: block;
        position: relative;
        color: white;
        font-family: Helvetica;
      }
      a, a:active, a:visited, a:link {
          color: white;
          text-decoration: none;
      }
      ul, li {
          list-style-type: none;
      }
      li {
          padding: 1rem 0;
      }
      .panel {
          width: 600px;
          height: 600px;
          background-color: black;
      }
      .list {
          padding: 0 4rem;
      }
      .list[seq="two"] {
          max-height: 0;
          overflow: hidden;
          transition: max-height 400ms;
      }
      .open {
          max-height: 1000px !important;
      }
      <div class="panel">
        <ul class="list" seq="one">
            <li data="hasChild">
                <a href="#">Category 1</a>
                <ul class="list" seq="two">
                    <li>Category 1-1</li>
                    <li>Category 1-2</li>
                    <li>Category 1-3</li>
                    <li>Category 1-4</li>
                </ul>
            </li>
            <li data="hasChild">
                <a href="#">Category 2</a>
                <ul class="list" seq="two">
                    <li>Category 2-1</li>
                    <li>Category 2-2</li>
                    <li>Category 2-3</li>
                    <li>Category 2-4</li>
                </ul>
            </li>
            <li>
                Category 3
            </li>
        </ul>
      </div>
sniffingdoggo
  • 398
  • 4
  • 16
  • What is this unused local `ternary` variable, why are you creating an object literal there? – Bergi Jun 29 '19 at 13:14
  • 1
    You are not calling the `deploy` method on the `Mobile` instance, `this` within the method body will be `undefined`. You'll want to bind the method to the current instance when passing it to `register(…)`. – Bergi Jun 29 '19 at 13:16
  • @Bergi `ternary` is not an unused variable. I've created the object there for grouping up the results. You can check it out Here the `ternary` works perfectly fine. – sniffingdoggo Jun 29 '19 at 13:27
  • @Bergi Great advice. I've found the solution. – sniffingdoggo Jun 29 '19 at 13:40
  • `ternary` is declared and initialised, but not used anywhere. You really should just write `if (!isSib) { this.removeClass(this.listSecond, 'open'); sib.classList.toggle('open'); }`. – Bergi Jun 29 '19 at 13:44
  • @Bergi How did you check the `ternary` is not used anywhere in the code? Personally and for me, `ternary` style works fine and looks more better than if statement. – sniffingdoggo Jun 29 '19 at 13:57
  • It simply doesn't appear more than once in the code that you posted, here or on the codepen you linked. I was assuming you posted the complete contents of your `destroy` method, of course. It doesn't use the object you're building anywhere. – Bergi Jun 29 '19 at 14:00

0 Answers0