[Bug]: Auto-height not detecting dynamic element changes

lucaspbordignon commented 2 weeks ago

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
[X] embla-carousel-auto-height
[ ] embla-carousel-class-names
[ ] embla-carousel-fade
[ ] embla-carousel-docs (Documentation)
[ ] embla-carousel-docs (Generator)

Steps to reproduce

The bug occurs when using the embla-carousel-auto-height package and the carousel elements have a dynamically set height

As an example, by using an <img /> tag with unknown dimensions upfront

What happens is that the carousel uses the initial height as the slide's height, prior to the element being loaded

Expected Behavior

The expected behavior is for embla-carousel to automatically calculate the element's height when navigating to it

Additional Context

Additionally, when resizing the window after the elements have their final height defined, we see the carousel being re-initialized, which ends up solving the problem

What browsers are you seeing the problem on?

All browsers

Version

v8.1.3

CodeSandbox

https://codesandbox.io/p/sandbox/embla-carousel-auto-height-react-lazy-images-p6f2f9

Before submitting

[X] I've made research efforts and searched the documentation
[X] I've searched for existing issues
[X] I agree to follow this project's Contributing Guidelines for bug reports

lucaspbordignon commented 2 weeks ago

@davidjerleke - Additionally, I've added #911 to the project, intending to solve the issue mentioned. Let me know if you think we should go to any other direction there

davidjerleke commented 2 weeks ago

Hi @lucaspbordignon,

The default behaviour for Embla is that it will only react to slide size changes that affect the chosen scroll axis. This means that a horizontal carousel only re-initialises when the container and/or slide widths change. In contrast, a vertical carousel only re-initialises when the container and/or slide heights change.

However, you can customize the resize behavior easily with the watchResize option. The most simple way to solve this would be to re-initialise the carousel when any dimension change, whether it’s heights or widths:

You should be able to solve it like this:

const emblaApi = EmblaCarousel({
  watchResize: (emblaApi, entries) => {
    for (const entry of entries) {
      if (emblaApi.containerNode() === entry.target) return true;

      window.requestAnimationFrame(() => {
        emblaApi.reInit();
        emblaApi.emit('resize'); 
      })

      break;
    }

    return false;
  }
}, [AutoHeight()]);

sarussss commented 2 weeks ago

Hi @davidjerleke There must be something wrong with this code, it makes the slider re-initialize continuously. https://codesandbox.io/p/sandbox/embla-carousel-auto-height-vanilla-forked-kdpkzm?file=%2Fsrc%2Fjs%2Findex.js%3A19%2C4

davidjerleke commented 2 weeks ago

@sarussss thanks for pointing that out! You’re right. I will adjust the code snippet to get rid of the undesired behavior.

davidjerleke commented 2 weeks ago

@sarussss and @lucaspbordignon I've updated the code snippet so it should work as expected now.

sarussss commented 2 weeks ago

@davidjerleke I tested the code, everything ran as expected. Thanks you.

lucaspbordignon commented 1 week ago

Hey @davidjerleke, pardon for the late reply!

Thanks for the time in explaining the issue and the solution with watchResize. That works like a charm, I've just customized the sandbox codebase and we can see the slides getting resized when images are loaded 🎉

Makes me think it might be good to include a piece of documentation mentioning this example and howwatchResize can solve it, so others don't stumble upon the same problem. What do you think?

Glad to write that piece down as well, lemme me know if you have any recommendations on where would be the best place to add it

We're probably fine closing this one then

davidjerleke commented 1 week ago

Thanks for getting back to me @lucaspbordignon 👍.

Makes me think it might be good to include a piece of documentation mentioning this example and howwatchResize can solve it, so others don't stumble upon the same problem. What do you think?

Glad to write that piece down as well, lemme me know if you have any recommendations on where would be the best place to add it

I would happily accept a PR for this. I think it should be mentioned somewhere here. Maybe we can show the following code snippet:

watchResize: (emblaApi, entries) => {
  for (const entry of entries) {
    if (emblaApi.containerNode() === entry.target) return true;

    window.requestAnimationFrame(() => {
      emblaApi.reInit();
      emblaApi.emit('resize'); 
    })

    break;
  }

  return false;
}

...with tabs for the different libraries like done here. What do you think?

davidjerleke / embla-carousel