3

I have a class like this:

class Hanoi{

   constructor(canvas) {
      //construcor things
   }

   onMouseDown(e) {
      for (var i = 0; i < this.pieces.length; i++) {
         let piece = this.pieces[i];
         if (piece.isClicked(e)) {
            this.isDragging = true;
            this.dragPiece = piece;
            this.bound = evt => this.onMouseMove(evt);
            this.canvas.addEventListener("mousemove", ev => {this.onMouseMove(ev)});
            this.canvas.addEventListener("mouseup", ev =>{this.onMouseUp(ev)});
            this.draw();
         }
      }
   }

   onMouseMove(e) {
      this.dragPiece.x = e.clientX;
      this.dragPiece.y = e.clientY;
      this.draw();
   }

   onMouseUp(e) {
      this.canvas.removeEventListener("mousemove", this.onMouseMove);
      this.canvas.removeEventListener("mouseup", this.onMouseUp);
      this.isDragging = false;
      this.draw();
   }
}

onMouseDown add two event listeners, but because of the arrow functions I cannot remove them when onMouseUp is called.

What is the best way to handle this?

Thomas
  • 3,348
  • 4
  • 35
  • 49
  • Is there a reason not to add your event handlers like `this.canvas.addEventListener("mouseup", this.onMouseUp);`? You're basically using an ordinary function with a reference already, so why insert an arrow function in between? – arbuthnott Sep 02 '17 at 13:15
  • because the "this." of this.onMouseUp will reference canvas and not the Class – Rodrigo Vázquez Sep 02 '17 at 13:20
  • Why are you binding several of the same handler in a loop all to the same element? If you use the solution below, `addEventListener` will prevent the duplicate bindings anyway because they're the same function object. – spanky Sep 02 '17 at 14:24

2 Answers2

6

Try the following:

...

constructor(canvas) {
  this.onMouseMove = this.onMouseMove.bind(this);
  this.onMouseUp = this.onMouseUp.bind(this);
}

onMouseDown(e) {
  ...
  this.canvas.addEventListener("mousemove", this.onMouseMove);
  this.canvas.addEventListener("mouseup", this.onMouseUp});
  ...
}

onMouseUp(e) {
  this.canvas.removeEventListener("mousemove", this.onMouseMove);
  this.canvas.removeEventListener("mouseup", this.onMouseUp);
  ...
}
Alexander Elgin
  • 6,796
  • 4
  • 40
  • 50
  • sir, thank you so much! you rock! – Rodrigo Vázquez Sep 02 '17 at 13:21
  • 1
    Please explain the cause of the problem to help beginners finding this question understand their mistake. Downvote until explained (due to my wall-of-code-with-no-explanation rule). – try-catch-finally Sep 02 '17 at 13:44
  • For what its worth, my understanding is in the constructor you are binding "this" to be your class for those methods, so your scope will be as you wish, but this way you assign the listener to the method this.onMouseMove instead of an anonymous function, so you can remove the listener later. Alternatively, as suggested below, you can define a variable to use the arrow function and use that variable later. – jacob.mccrumb Sep 13 '18 at 01:18
0

It would be best to use a named normal function definition if you want to reference that function later, for instance, to remove. However if you insist on using arrows you may still do as follows;

You may assign the arrow function expression to a variable while passing it as a callback. Then remove by using that name. So if you don't want to litter the global namespace you can always use a the surrounding function object to store your arrow functions. In this particular case i named the "click" event listeners callback as X and store "muouseup" arrow function under that function object's muel property which removes itself when fired.

Like;

btt.addEventListener("click", function X(e){
  e.target.textContent === "Add Listener" ? (e.target.addEventListener("mouseup", X.muel = e => {
                                                                          e.target.removeEventListener("mouseup", X.muel);
                                                                          console.log("mouse up fired and event listener removed");
                                                                        }),
                                             e.target.textContent = "Remove Listener")
                                          : e.target.textContent = "Add Listener";
                              });
<button id="btt">Add Listener</button>
Redu
  • 25,060
  • 6
  • 56
  • 76