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

Slide gaps will not automatically adjust the scroll snaps #656

Closed martinsjek closed 6 months ago

martinsjek commented 6 months ago

Bug is related to

Embla Carousel version

Describe the bug

CodeSandbox

https://codesandbox.io/p/sandbox/embla-carousel-generator-vanilla-forked-cvmp97?file=%2Fsrc%2Fcss%2Fembla.css

davidjerleke commented 6 months ago

Hi @martinsjek,

When switching from padding spacing to margins, you probably forgot to remove this line on the container. Try removing it and let me know how it goes.

Best, David

martinsjek commented 6 months ago

@davidjerleke Thanks for your quick response.

But when i remove margin-left from .embla__container, it does not solve the issue.

Martins

davidjerleke commented 6 months ago

@martinsjek what's the issue then? What are you trying to achieve?

martinsjek commented 6 months ago

@davidjerleke I am trying to achieve that slider is not broken, as you can see in attached photo - the slide 4 is clipped when it shouldn't be. Maybe I am doing something wrong? image

davidjerleke commented 6 months ago

@martinsjek you've set align option to start, meaning that slides should be aligned to the left edge of the carousel viewport, and the selected snap is 2 (third slide) which is as expected aligned to the left. Both slides don't fit inside the viewport so slide 4 has to be clipped?

martinsjek commented 6 months ago

@davidjerleke Yes, the issue is that the slides do not fit inside the viewport.

davidjerleke commented 6 months ago

@martinsjek that's because your CSS math isn't correct. I'm guessing that this is what you want to achieve?

You might be misunderstanding the docs. The docs say that any margin or spacing you add to your slides will be included in the Embla calculations. But Embla won't adjust or correct stuff for you, it will just read the spacings and include them. You still have to get the CSS math right for it to work.

martinsjek commented 6 months ago

@davidjerleke Yes, almost that. Except i want the container to prevent overflow. And adding overflow:hidden, breaks it. Maybe we need to add that in the docs with example with spacings?

davidjerleke commented 6 months ago

@martinsjek adding overflow to the container isn’t a valid setup. The docs makes this 100% clear. What overflow do you want to prevent? I’m not following?

Maybe you could share a link to an example carousel (like a website or a different carousel library) so I can better understand what you want to achieve?

martinsjek commented 6 months ago

@davidjerleke Thanks, i made a fix, and now I want 3 slides in viewport, but it's broken again: https://codesandbox.io/p/sandbox/embla-carousel-generator-vanilla-forked-rwmghf?file=%2Fsrc%2Fcss%2Fembla.css%3A3%2C28

davidjerleke commented 6 months ago

This is because you need to adjust this line if you change the slide sizes. This is a working example.

However, I would recommend you to use the padding approach instead of margins, because then you don't have to worry about getting the math right as soon as you change slide sizes. Please read this:

One solution is to use the negative margin on the container and a corresponding amount of padding-left on each slide. This way, the spacings between the items will always be included in the slide widths, so you can just set different slide widths based on different breakpoints without worrying about calculating stuff. Here’s an example demonstrating that approach. Just go to that sandbox and change the --slize-size variable (in the css/embla.css file) to anything that adds up to 100%. For example: 20%, 25%, calc(100% / 3) and you'll see.

Originally posted by @davidjerleke in https://github.com/davidjerleke/embla-carousel/issues/593#issuecomment-1760771682

martinsjek commented 6 months ago

@davidjerleke Big thanks to you, your code example fixed my issue. I'm closing this issue :)