Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

dispose() does not remove mouse wheel zoom #26

Closed
dotnetCarpenter opened this issue Mar 23, 2018 · 8 comments
Closed

dispose() does not remove mouse wheel zoom #26

dotnetCarpenter opened this issue Mar 23, 2018 · 8 comments

Comments

@dotnetCarpenter
Copy link
Contributor

I'm using the npm package, 4.3.1 and when calling instance.dispose() only pan handlers are removed. I can still zoom with the mouse wheel.

@dotnetCarpenter
Copy link
Contributor Author

Using Chrome 65.0.3325

@dotnetCarpenter
Copy link
Contributor Author

Heh. I just found this comment in the beginning of function _addWheelListener:

// TODO: in theory this anonymous function may result in incorrect
// unsubscription in some browsers. But in practice, I don't think we should
// worry too much about it (those browsers are on the way out)

@dotnetCarpenter
Copy link
Contributor Author

After debugging the issue in wheel, I have found that the elem is not the same on addEventListener and removeEventListener.

I'm using vue.js, and have to investigate if it's because vue.js is changing the DOM element or if it's in panzoom. What is strange if it's vue.js, is that the other event handlers seems to be removed just fine.

@dotnetCarpenter
Copy link
Contributor Author

Found it! wheel is listening to owner but trying to remove the wheel listener on domElement.

dotnetCarpenter added a commit to dotnetCarpenter/panzoom that referenced this issue Mar 23, 2018
@dotnetCarpenter dotnetCarpenter mentioned this issue Mar 23, 2018
@dotnetCarpenter
Copy link
Contributor Author

For everyone else with this issue, I have a published a release candidate here: https://www.npmjs.com/package/@dotnetcarpenter/panzoom

anvaka added a commit that referenced this issue Mar 24, 2018
anvaka added a commit that referenced this issue Mar 24, 2018
@anvaka
Copy link
Owner

anvaka commented Mar 24, 2018

Thanks for catching this and fixing it!

This is published as v4.4.0

@dotnetCarpenter
Copy link
Contributor Author

@anvaka nice that you added tests! I was wondering how to test this.

@anvaka
Copy link
Owner

anvaka commented Mar 28, 2018

you can run npm test in the project's folder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants