Added destroy method for the PinchZoom instance
#129
Conversation
# Removes the global `resize` listener and removes the zooming container
| * Unmounts the zooming container and global event listeners | ||
| */ | ||
| destroy: function () { | ||
| window.removeEventListener('resize', this.resizeHandler); |
There was a problem hiding this comment.
If the instance is destroyed while image loading is in flight (or possibly other async stuff), we may have callbacks triggered on the now-destroyed instance. Those would error out.
Not a blocker, but something to be aware of.
There was a problem hiding this comment.
I don't think there's any async handling on the pinch-zoom-js side that we need to care about 🤔. As soon as the PinchZoom instance is created, it synchronously mounts (this.setupMarkdown()) all the required dom elements with events listeners (this.bindEvents()).
Therefore, IMHO, it's the responsibility of the library users to destroy the PinchZoom instance at the right time. Because only they can know how the image is asynchronously loaded in different ways. E.g.: plain img tag with onload handler or some more complicated images library with a different loading handler.
|
Thanks @oleksandr-danylchenko! |
|
You're welcome 🙌 |
|
@sandstrom, when the next lib version should be awaited? |
|
@oleksandr-danylchenko I've released 2.3.5 now (https://github.com/manuelstofer/pinchzoom/releases/tag/v2.3.5). |
|
Thank you! |
Previously when there were multiple invocations of the
PinchZoomconstructor, a newresizeevent listener was registered each time. It led to the improper resing of the images when the users rotated their devices.RPReplay_Final1656005250.1.online-video-cutter.com.mp4
But now developers can explicitly destroy the
PinchZoominstance on unmount which will prevent redundant stacking of the event handlers and memory leaks.Fixes #128