davidjerleke / embla-carousel

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

No transitions on first slide while using navigation buttons when destroy and reinit the carousel #672

Closed sadeghbarati closed 6 months ago

sadeghbarati commented 6 months ago

Bug is related to

Embla Carousel version

Describe the bug

No transition occurs on first slide on hitting next button when destroy and reinit the Embla Carousel

CodeSandbox / StackBlitz

https://stackblitz.com/edit/vitejs-vite-k4menh?file=src%2Fcomponents%2FCarousel.vue

Steps to reproduce

  1. Go to stackblitz repro
  2. Click on Destroy/Reinit Carousel
  3. Click next button
  4. No transitions occurs on first slide

Expected behavior

slide should have normal transitions

Additional context

None

sadeghbarati commented 6 months ago

Related

https://github.com/shadcn-ui/ui/issues/2230

davidjerleke commented 6 months ago

@sadeghbarati thank you for your bug report. Try this and see if you can reproduce the problem?

sadeghbarati commented 6 months ago

It's working fine now 🙏

What was the problem

davidjerleke commented 6 months ago

@sadeghbarati you'll see the code changes when I create the PR. I will release a patch release with the bug fix as soon as possible. It will be v8.0.0-rc18. Thanks for testing it!

sadeghbarati commented 6 months ago

Just one thing before the publish embla-carousel-vue

Isn't better to use onBeforeUnmount instead of onUnmounted?

davidjerleke commented 6 months ago

Isn't better to use onBeforeUnmount instead of onUnmounted?

@sadeghbarati yeah maybe that’s better? I’m not a seasoned Vue dev. Most of my experience is with React. Why is it better? And why is the current approach working as expected?

sadeghbarati commented 6 months ago

Both are working well, only just to separate the embla destroying process from Vue reactivity cleanup

davidjerleke commented 6 months ago

@sadeghbarati feel free to try it out in the latest StackBlitz fork I shared and submit a PR with the onBeforeUnmount hook instead of onUnmounted.

Best, David

sadeghbarati commented 6 months ago

Thank You ❤️‍🔥

davidjerleke commented 6 months ago

@sadeghbarati so you will create a PR 🙂?

sadeghbarati commented 6 months ago

Sure thing!

But It was easy to contribute if I was pnpm and pnpm workspaces (Adding Vue in playground directory) :grin:

sadeghbarati commented 6 months ago

Sry it took too long I was trying to figure out the yarn and creating playground for Vue and also styled-components injected styles which I ignored and copy static styles into main style file

Should I have to include embla-carousel-playground-vue in this PR https://github.com/davidjerleke/embla-carousel/pull/673? although the build files are different from what I see in npm


npmjs - embla-carousel-vue/esm/index.d.ts

export { EmblaOptionsType, EmblaEventType, EmblaPluginType, EmblaCarouselType } from 'embla-carousel/index.ts';
export { EmblaCarouselVueType } from './components/emblaCarouselVue.ts';
export { default } from './components/emblaCarouselVue.ts';

local build - embla-carousel-vue/esm/index.d.ts

export { EmblaCarouselVueType } from './components/emblaCarouselVue.ts';
export { default } from './components/emblaCarouselVue.ts';
davidjerleke commented 6 months ago

@sadeghbarati thanks for the PR 👍🏻! The build files differ because of this upcoming change which I haven’t released yet.

I think the playground should be a separate PR just to separate things. You can check out the vanilla and react playgrounds to see how the styles are imported from the documentation website examples. You don’t need to add your own that way. And I would recommend you to setup a Vite playground just like the vanilla and react to make it easier for devs to jump between the playgrounds when testing things.

Thanks for your efforts!

sadeghbarati commented 6 months ago

Ok will create new PR for playground later after the new release

Thanks for the opportunity ❤️

davidjerleke commented 6 months ago

@sadeghbarati a bug fix for this was just released in v8.0.0-rc18. Try it out and let me know if it's working as expected.