davidjerleke/embla-carousel

[Bug]: Scroll broken after direction update

Closed this issue · 15 comments

meirroth commented

Which variants of Embla Carousel are you using?

  • embla-carousel (Core)
  • embla-carousel-react
  • embla-carousel-vue
  • embla-carousel-svelte
  • embla-carousel-autoplay
  • embla-carousel-auto-scroll
  • embla-carousel-solid
  • embla-carousel-auto-height
  • embla-carousel-class-names
  • embla-carousel-docs (Documentation)
  • embla-carousel-docs (Generator)

Steps to reproduce

The bug occurs when triggering reInit after changing direction.

Click the button "Change dir" and try dragging the carousel, you'll see you can't scroll all the way as you would expect.

Expected Behavior

I expect Embla to work smoothly after switching direction.
Also would expect Embla to detect option change, as David mentioned options are reactive #744 (comment).

Additional Context

Setting direction RTL initially works as expected.

What browsers are you seeing the problem on?

Firefox, Chrome

Version

v8.0.2

CodeSandbox

https://stackblitz.com/edit/github-jafces?file=app.vue

Before submitting

  • I've made research efforts and searched the documentation
  • I've searched for existing issues
  • I agree to follow this project's Contributing Guidelines for bug reports
meirroth commented

I'm migrating from vueper-slides in my Nuxt.js production project. Embla looks very processing and I can't wait to get it up and running 😃

davidjerleke commented

@meirroth Embla makes use of native browser direction so you need to set <body dir="rtl"> before setting the carousel direction option to rtl. This is mentioned in the docs. Have you tried that?

meirroth commented

@davidjerleke I did that. Notice useHead().
You can inspect HTML and see dir="rtl" on the HTML element.

davidjerleke commented

@meirroth so that happens before direction option is set?

meirroth commented

@davidjerleke Not sure what you mean. useHead sets dir on the HTML tag, and updates it when the value changes.

davidjerleke commented

@meirroth log this AFTER direction has changed:

emblaApi.value.internalEngine().options.direction

What do you get? Does it match your direction state?

meirroth commented

@davidjerleke It logs the correct direction:
image

davidjerleke commented

@meirroth I don’t have access to my computer so will check your StackBlitz out tomorrow.

meirroth commented

@davidjerleke Thank you!

sarussss commented

Hi @meirroth I think it works normally
Are you sure?
Change dir in html tag
Runs before reinstalling embla

  • 👍1
  • ❤️1
davidjerleke commented

@meirroth as far as I can see, the document dir hasn't finished updating when you re-initialise the carousel with the direction "rtl". See this StackBlitz and check the console when you click on the button (and also see the comments in the code). You need to do things in the correct order:

  1. Update the document dir attribute.
  2. AFTER that, you have to update the options like this options.value = { direction: dir.value } and Embla will re-initialise because its options are reactive. No need to call reInit yourself.

I'm not a Vue expert by any means, so I don't know if there's a way to wait for the DOM to update before changing the options, in this case we want to wait for the document dir attribute. If there isn't, I would recommend you to use MutationObserver on the HTML element and change options when the dir attribute changes.

Best,
David

  • 👍1
meirroth commented

@davidjerleke Thank you investigating this issue and for the explanation!
You are right, i18n or useHead is not fast enough to update HTML dir for Embla, so I got it working by setting dir manually on the carousel and using watchPostEffect to update the Embla direction.
https://stackblitz.com/edit/github-gmh6db-bjqkxt?file=app.vue

Embla accepting reactive options is awesome!! I think this should be mentioned in the docs, unless it already is and I missed it. Would you like me to open a PR?

  • 👍1
davidjerleke commented

@meirroth I'm glad you solved it 🎉.

Embla accepting reactive options is awesome!! I think this should be mentioned in the docs, unless it already is and I missed it. Would you like me to open a PR?

Sure. I would appreciate a PR! Maybe add an additional note (admonition) here?

Note

One important thing to note though: The options for the core vanilla package embla-carousel are NOT reactive, but the React, Vue, Solid and Svelte wrappers have reactive options. I think it makes sense to make that clear so vanilla JS users don't expect their options to be reactive.

Best,
David

  • 👍1
davidjerleke commented

@meirroth thanks for taking time to improve the documentation!

  • 👍1