CODEWITHSUNDEEP
javascripteventsvue.jsvuejs2
justinw
justinw

Reputation: 3976

removeEventListener not working as expected

I have a fairly simple component in my Vue app. When the user opens the menu, I add an event listener to the #app element to detect "outside" clicks.

When the user clicks outside, it closes the menu and removes the event listener. This is working as expected. The issue is, when the user clicks the button to open the menu, and then clicks it again to close it, I also would like to removeEventListener because the menu is now closed.

This however is not working. When I open the menu via the button and then close it via the button the event listener stays on the #app element even though I am removing it.

If you click the button multiple times and then click outside of the button while the value displays true you will see the console log outside click multiple times.

new Vue({
    el: "#app",
    data: {
        menuVisible: false
    },
    methods: {
        click(e) {
            var clickSource = e.target;
            var app = document.getElementById("app");
            var that = this;

            function clickOutside(e) {
                if (e.target != clickSource) {
                    console.log("outside click");
                    that.menuVisible = false;
                    app.removeEventListener("click", clickOutside);
                }
            }

            if (this.menuVisible === false) {
                this.menuVisible = true;
                app.addEventListener("click", clickOutside);
            } else if (this.menuVisible === true) {
                this.menuVisible = false;
                app.removeEventListener("click", clickOutside);
            }
        }
    }
});
body {
   margin: 0;
}

#app {
min-height: 100vh;
}
<script src="https://unpkg.com/vue"></script>

<div id="app">
  {{menuVisible}}
  <button @click="click">
  click
  </button>
</div>

Upvotes: 1

Views: 2061

Answers (2)

Terry
Terry

Reputation: 66228

This feels like an X-Y problem to me. Ideally speaking you should not be dynamically binding/unbinding event listeners in JS (or VueJS even). If you simply want the following behavior:

  • Button click toggles the menu
  • Click on the app outside the button closes the menu

...then you don't even need messy event handling. The first behavior can be implemented by a simple true/false switch, i.e.

this.menuVisible != this.menuVisible

Remember to use .stop on the @click event binder on the button, so you can stop the VueJS click event from bubbling up to the app.

Meanwhile, the second behavior can be done by simply setting this.menuVisible to false, when a click event is detected on the #app but outside the button.

new Vue({
  el: "#app",
  data: {
    menuVisible: false
  },
  methods: {
    click(e) {
      console.log('button clicked');
      this.menuVisible = !this.menuVisible;
    },

    appClick(e) {
      console.log('outside clicked');
      this.menuVisible = false;
    }
  }
});
body {
  margin: 0;
}

#app {
  min-height: 100vh;
}
<script src="https://unpkg.com/vue"></script>

<div id="app" @click="appClick">
  {{menuVisible}}
  <button @click.stop="click">
  click
  </button>
</div>

Upvotes: 1

T.J. Crowder
T.J. Crowder

Reputation: 1075835

The problem is that on every click, you're creating a new clickOutside function; if you use that function with removeEventListener, since there's no event registration with that exact function object, nothing is removed.

Instead, remember the handler object so you can use it for removal, see notes:

new Vue({
    el: "#app",
    data: {
        menuVisible: false,
        clickOutside: null
    },
    methods: {
        click(e) {
            var clickSource = e.target;
            var app = document.getElementById("app");
            var that = this;
            
            // Only create it if we don't already have it
            if (!this.clickOutside) {
                // Create it
                this.clickOutside = function clickOutside(e) {
                    console.log("clickOutside triggered");
                    if (e.target != clickSource) {
                        console.log("It's an outside click");
                        that.menuVisible = false;
                        // Remove it
                        app.removeEventListener("click", that.clickOutside);
                    }
                };
            }

            if (this.menuVisible === false) {
                this.menuVisible = true;
                app.addEventListener("click", this.clickOutside);
            } else if (this.menuVisible === true) {
                this.menuVisible = false;
                app.removeEventListener("click", this.clickOutside);
            }
        }
    }
});
body {
   margin: 0;
}

#app {
  min-height: 100vh;
  background-color: #ddd;
}
<script src="https://unpkg.com/vue"></script>

<div id="app">
  {{menuVisible}}
  <button @click="click">
  click
  </button>
</div>

Side note: It's more idiomatic to use

if (this.menuVisible) {
    this.menuVisible = false;
    app.removeEventListener("click", this.clickOutside);
} else {
    this.menuVisible = true;
    app.addEventListener("click", this.clickOutside);
}

Or even

this.menuVisible = !this.menuVisible;
if (this.menuVisible) {
    app.addEventListener("click", this.clickOutside);
} else {
    app.removeEventListener("click", this.clickOutside);
}

...rather than comparing a boolean to true or false with ===. You already have a boolean, so... I mean, where do you stop? if ((this.menuVisible) === true === true)? :-)

Upvotes: 1

Related Questions