Fix event listener cleanup in destroy() method

[killerwolf]

Background

The current implementation of the destroy() method in VisualImageTool does not correctly remove event listeners. The code uses .bind(this) to create new function references when removing listeners, but these are different references than the ones used when adding the listeners, so they don't get properly removed.

This can lead to memory leaks and unexpected behavior when instances are destroyed and recreated, as demonstrated in some of the demo files.

Problem

When the event listeners are added in _setupEventListeners():

window.addEventListener("resize", this._updateScaling.bind(this));
document.addEventListener("mouseup", this._handleMouseUp.bind(this));
document.addEventListener("mousemove", this._handleMouseMove.bind(this));

And later removed in destroy():

window.removeEventListener("resize", this._updateScaling.bind(this));
document.removeEventListener("mouseup", this._handleMouseUp.bind(this));
document.removeEventListener("mousemove", this._handleMouseMove.bind(this));

Each call to .bind(this) creates a new function reference, so the removed listeners are not the same as the ones that were added.

Solution

The correct pattern is to store bound function references in the constructor and use the same references for both adding and removing event listeners:

constructor(options) {
  // ... existing code ...
  
  // Pre-bind event handlers
  this._boundUpdateScaling = this._updateScaling.bind(this);
  this._boundHandleMouseUp = this._handleMouseUp.bind(this);
  this._boundHandleMouseMove = this._handleMouseMove.bind(this);
  this._boundHandleFocusMarkerMouseDown = this._handleFocusMarkerMouseDown.bind(this);
  this._boundHandleCropOverlayMouseDown = this._handleCropOverlayMouseDown.bind(this);
  
  // ... rest of existing code ...
}

_setupEventListeners() {
  // Use pre-bound handlers
  window.addEventListener("resize", this._boundUpdateScaling);
  document.addEventListener("mouseup", this._boundHandleMouseUp);
  document.addEventListener("mousemove", this._boundHandleMouseMove);
}

_createFocusMarker() {
  // ... existing code ...
  
  // Use pre-bound handler
  marker.addEventListener("mousedown", this._boundHandleFocusMarkerMouseDown);
}

_createCropOverlay() {
  // ... existing code ...
  
  // Use pre-bound handler
  overlay.addEventListener("mousedown", this._boundHandleCropOverlayMouseDown);
}

destroy() {
  // ... existing code ...
  
  // Use pre-bound handlers to remove event listeners
  window.removeEventListener("resize", this._boundUpdateScaling);
  document.removeEventListener("mouseup", this._boundHandleMouseUp);
  document.removeEventListener("mousemove", this._boundHandleMouseMove);
  
  // ... rest of existing code ...
}

Additional Considerations

• The load event listener for the image is not removed in the current implementation
• Event listeners on the crop overlay handles would need to be managed in a similar way
• Proper cleanup is especially important for libraries that might be used in long-running applications

This addresses item #3 from the review in issue #6.