search

davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
5.97k stars 180 forks source link

Fix leak in slidesInView event #613

Closed fcasibu closed 11 months ago

fcasibu commented 11 months ago

Hello first time trying out this package and I love everything about it but there's just one thing that's causing me some issues regarding the slidesInView event. I've created this Sandbox to demonstrate the issue.

To reproduce:

  1. Try not to resize the sandbox and see the counter "called" gets incremented by 1 perfectly fine
  2. Resize the sandbox
  3. Wait for the autoplay to slide automatically or slide it yourself
  4. Counter "called" gets incremented by A LOT and this shouldn't happen

I've added engine.slidesInView.destroy() in the deactivate function to fix the issue in embla-carousel package.

We can also listen to the reinit event to destroy the event but I feel like this shouldn't be done on the end user but on the package's side.

davidjerleke commented 11 months ago

@fcasibu thank you very much for your effort! Have you tried the changes to see if it solves the problem?

Best, David

fcasibu commented 11 months ago

Hey @davidjerleke I've tested it on the playground for react and it does indeed solve the problem. Thanks 🙇

davidjerleke commented 11 months ago

@fcasibu thank you for your swift response. Nice work and welcome to the contributors club 🎉!

fcasibu commented 11 months ago

Seems like it broke a lot of tests regarding intersectionObserver should've added disconnect on the mock, sorry about that @davidjerleke

davidjerleke commented 11 months ago

@fcasibu no worries. It's just a minor thing. I can fix it. Thanks for informing me though 👍.

davidjerleke commented 11 months ago

@fcasibu done 🙂.

davidjerleke commented 10 months ago

@fcasibu I've released v8.0.0-rc15 which includes this PR.