5

I need to remove an event listener set on window, but it doesn't work, the listener keeps firing on scroll. I've tried to set the listener with and without lodash's throttle but it doesn't make any difference. Here's my code:

setupListener() {
    window.addEventListener('resize', _.throttle(this.handler.bind(this), 750));
    window.addEventListener('scroll', _.throttle(this.handler.bind(this), 750));
}

removeListener() {
    window.removeEventListener('resize', _.throttle(this.handler.bind(this), 750));
    window.removeEventListener('scroll', _.throttle(this.handler.bind(this), 750));
    window.addEventListener('load', this.handler.bind(this), false);
}

static isElementInViewport (el) {
    let rect = el.getBoundingClientRect();
    return (
        rect.bottom <= (window.innerHeight || document.documentElement.clientHeight) &&
        rect.right <= (window.innerWidth || document.documentElement.clientWidth)
    );
}

handler() {
    if (this.options.url === undefined) {throw new Error('no url specified');}
    if (InfiniteScroll.isElementInViewport(this.elementToWatch)) {
        this.removeListener();
        this[this.options.transport]();
    }
}

I've also tried to promisify the removal:

handler() {
    if (this.options.url === undefined) {throw new Error('no url specified');}
    if (InfiniteScroll.isElementInViewport(this.elementToWatch)) {
        Promise.resolve(this.removeListener())
            .then(val => {
                this[this.options.transport]();
            });
    }
}

which didn't make a difference either.

Later in the code, I want to reassign the listeners:

handleResponse(data) {
        console.log('handleResponse' + data);
        Promise.resolve(this.addElementsToDOM(data))
            .delay(1000)
            .then(() => {
                this.page++;
                this.elementToWatch = document.getElementById(this.element).rows[document.getElementById(this.element).rows.length - this.options.loadTiming];
                //this.setupListener();
            });
    }

I've logged every step of the and am unable to find the reason for it. Can someone please assist?

Sidenote: Is there a better way of handling the scroll event than to remove and add the listeners all the time?

  • 1
    Thanks @Amit, I've seen this question and answer. I think I'm using a named function allready - or do I miss something there? –  Sep 28 '15 at 23:30
  • Yes, you're definitely missing the point. You need to `remove` ***exactly*** what you `add`ed, that is, not a value returned from a `bind()` and then wrapped in another call. – Amit Sep 28 '15 at 23:32
  • 1
    Have you tried my answer, @J.Doe? – Buzinas Sep 28 '15 at 23:34
  • Wow, thanks. Now I get it. @Amit. I'll close it. –  Sep 28 '15 at 23:36

1 Answers1

6

The thing is: when you add an event listener, the browser saves as its 'key' the function reference that you're passing as a parameter. So, when you want to remove it, you must send that reference.

There are two ways to solve your problem. The first is to create named functions:

setupListener() {
  this.listener = function() {
    _.throttle(this.handler.bind(this), 750);
  }.bind(this);

  window.addEventListener('resize', this.listener);
  window.addEventListener('scroll', this.listener);
}

removeListener() {
  window.removeEventListener('resize', this.listener);
  window.removeEventListener('scroll', this.listener);
  window.addEventListener('load', this.handler.bind(this), false);
}

The other, is by overriding the Window's addEventListener method, but I don't suggest you to do that.

Buzinas
  • 11,597
  • 2
  • 36
  • 58
  • 1
    To the downvoter: could you leave a comment to explain why is that? – Buzinas Sep 28 '15 at 23:28
  • 2
    Probably because this question should be closed, not answered. Also, that last point with overriding `addEventListener`, it's a good suggestion not to do that, so is not shooting yourself in the head, but it's much better not to mention these ideas in the first place. – Amit Sep 28 '15 at 23:34