search

mharris717 / ember-drag-drop

Drag and Drop Addon for Ember CLI
MIT License
204 stars 141 forks source link

bind context to fix error #174

Closed Jbcampbe closed 4 years ago

Jbcampbe commented 4 years ago

Fix error that was missed in #171

dgavey commented 4 years ago

Thanks, I'll do a release later tonight

boris-petrov commented 4 years ago

@Jbcampbe, @dgavey - I don't believe this works as intended. this.handleMouseEnter.bind(this) returns a new function every time it is called. So the call to removeEventListener won't remove the added listener because the function is different. The result from the binding in addEventListener should be saved as an instance variable or, better yet, this change to be reverted to be just this.handleMouseEnter and the handleMouseEnter to be marked as action from '@ember/object':

import { action } from '@ember/object';

...
  handleMouseEnter: action(function (e) {
    let mouseEnter = this.get('onMouseEnter');
    if (mouseEnter) {
      mouseEnter(e);
    }
  }),
Jbcampbe commented 4 years ago

@Jbcampbe, @dgavey - I don't believe this works as intended. this.handleMouseEnter.bind(this) returns a new function every time it is called. So the call to removeEventListener won't remove the added listener because the function is different. The result from the binding in addEventListener should be saved as an instance variable or, better yet, this change to be reverted to be just this.handleMouseEnter and the handleMouseEnter to be marked as action from '@ember/object':

import { action } from '@ember/object';

...
  handleMouseEnter: action(function (e) {
    let mouseEnter = this.get('onMouseEnter');
    if (mouseEnter) {
      mouseEnter(e);
    }
  }),

Thanks for pointing out! I can get that fix in a PR