Having to click button twice to call a function

Hello,
I’m new to vue. I have this vue button

<button
              class="btn menu-link sub-menu-toggle"
              @click="menu('.sub-menu-toggle','.sub-men')"
            >Item
</button>

And this is the function it calls on click

  methods: {
    menu: function(toggle, element) {
      document.querySelectorAll(toggle).forEach(el => {
        el.onclick = el => {
          el.stopPropagation();

          document.querySelectorAll(element).forEach(el => {
            el.classList.add("view");
            el.parentElement.classList.add("on");

            document.onclick = event => {
              if (event.target.closest(".main-menu")) return;
              el.classList.remove("view");
              el.parentElement.classList.remove("on");
            };
          });
        };
      });
    }

It works as it should. The problem is a user has to click it twice for it to open the menu.
What am I doing wrong? What should I do?
Thank you in advance

Well, based on your code, the first click registers the click handlers. The second click will register them again but also execute the other handlers your registered in the first click.

Is there a reason you’re going about it this way? Why register the handlers in a click event? Why not register them when the menu is rendered initially?

3 Likes

Why register a click handler at all when @click="..." is registering a click handler for you? Just do:

<button class="btn menu-link" @click.stop="menu('.sub-men')">
  Item
</button>

  methods: {
    menu: function(element) {
          document.querySelectorAll(element).forEach(el => {
            el.classList.add("view");
            el.parentElement.classList.add("on");

            document.onclick = event => {
              if (event.target.closest(".main-menu")) return;
              el.classList.remove("view");
              el.parentElement.classList.remove("on");
            };
          });
    }

Even then, this is still not good Vue code because you are adding your own classes to elements instead of letting Vue do that for you. It would be better to set a property when the menu should be open and use that property to control the classes of the sub menu.

Then you need a better way of registering the click event outside the menu, because currently you are registering click event listeners on the root document which are never stopped…

2 Likes

Thanks for this. It worked perfectly.
I’d appreciate a code example of how to do it right( It would be better to set a property when the menu…)

It’s difficult to show you how to do it right without seeing the rest of your code.

Can you create a jsfiddle showing how your menu currently operates and then I can try to make it work the “Vue” way?

Also, have a read of the Vue guide for Class and Style Bindings.

OK, here’s the basics:

<button class="btn menu-link" @click.stop="menuOpen = true">
  Item
</button>

... somewhere else IN THE SAME COMPONENT, unless you store menuOpen somewhere shared...

<div :class="['menu', {on: menuOpen}]">
   <div :class="['sub-men', {view: menuOpen}]">
      ...

...
  data() { return {
    menuOpen: false,
  }},
  mounted() {
    document.addEventListener('click', this.onDocumentClick);
  },
  beforeDestroy() {
    // dont forget to remove the event listener!  (unless youre sure this component is permanent)
    document.removeEventListener('click', this.onDocumentClick); 
  },
  methods: {
    onDocumentClick() {
       this.menuOpen = false;
    },
  }

You’ll notice there are no instances of el.classList.add() or any direct manipulation of the DOM - it’s best (and shorter code!) to leave this to Vue.

1 Like

Thank you! I really appreciate the help :+1: