davidjerleke / embla-carousel

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

Navigation bug #376

Closed 7iomka closed 1 year ago

7iomka commented 1 year ago

Bug is related to

Embla Carousel version

Describe the bug

About 2 weeks ago I found an issue with Mantine wrapper for embla-carousel The issue was that the formula that was supposed to take into account the gap between the slides did not work as expected. Because of what I thought that the navigation is not working as expected. Today I writing the code that partially fixes the problem with the calculation formula, but the navigation still works incorrectly

CodeSandbox

Steps to reproduce

  1. Open demo
  2. Try to navigate with arrow buttons
  3. 1nd, 2nd click - nothing happens, after 3rd click - it runs, and etc.

Expected behavior

davidjerleke commented 1 year ago

Hi @7iomka,

Thank you for your bug report. If you’re experiencing problems with the Embla Mantine wrapper, you should create an issue in the Mantine repository and not here. I don’t provide Mantine support too.

If you can reproduce the problem with the raw Embla API, that is without the Mantine wrapper, then the place to create a bug report is here.

Let me know how it goes.

Best, David

7iomka commented 1 year ago

@davidjerleke I tried to reproduce, so there is something similar behavior I think: https://codesandbox.io/s/embla-carousel-arrows-dots-react-forked-4w43qt?file=/src/js/EmblaCarousel.js

7iomka commented 1 year ago

Also I would like to ask a question - why is the initial negative indent on the left side set?

image

I think this is related with approach which I used to include gap in slide width But it is a very common approach https://codepen.io/7iomka/pen/abGBQmd

Note: when you use padding approach, this negative indent will not appear I mean in context of provided demo from above

flex: 0 0 calc(var(--slide-size));
padding-left: calc(var(--slide-gap) / 2);
padding-right: calc(var(--slide-gap) / 2);
davidjerleke commented 1 year ago

Thanks for the CodeSandbox @7iomka,

Embla just reads slide dimensions and the distance between slides when initialised, it doesn't modify any margins or slide widths. You shouldn't have negative margin on the container. Just set margins on the slides and it will work:

.embla__container {
  display: flex;
  margin-left: calc(-1 * var(--slide-gap) / 2); /* <-- REMOVE this */
  margin-right: calc(-1 * var(--slide-gap) / 2); /* <-- REMOVE this */
}

.embla__slide {
  position: relative;
  flex: 0 0 calc(var(--slide-size) - var(--slide-gap));
  margin-left: calc(var(--slide-gap) / 2);  /* <-- USE this */
  margin-right: calc(var(--slide-gap) / 2);  /* <-- USE this */
}

As an alternative approach you can use the negative container margin left, and positive slide padding left trick:

.embla__container {
  display: flex;
  margin-left: calc(-1 * var(--slide-gap));
}

.embla__slide {
  position: relative;
  flex: 0 0 var(--slide-size);
  padding-left: var(--slide-gap);
}

Which is why this works:

Note: when you use padding approach, this negative indent will not appear

This is purely CSS related and has nothing to do with Embla. I hope this helps.

Best, David

7iomka commented 1 year ago

@davidjerleke First approach from proposed give me initial negative offset https://codesandbox.io/s/embla-carousel-arrows-dots-react-forked-ebgo4b?file=/src/css/embla.css Second (alternative) approach is the only working approach in terms of a correct initial offsets and fit. https://codesandbox.io/s/embla-carousel-arrows-dots-react-forked-6kzu72?file=/src/css/embla.css

Thanks for explanations.

davidjerleke commented 1 year ago

Hi @7iomka,

⚠️ Please note that the margin feature was released with Embla v6 and up. Your CodeSandbox is using v5 which is an old version. I recommend you to use the latest version.

This is why you can’t get the margins to work. Here’s a working CodeSandbox:

https://codesandbox.io/s/embla-carousel-arrows-dots-react-forked-wd5b9y

Best, David

7iomka commented 1 year ago

@davidjerleke , yes, it works, but it also produce unexpected spaces

Снимок экрана 2022-09-27 в 07 45 06

It could be compensated by negative margin values on the container, but since you say they are not allowed on the container, the only viable approach at the moment is the alternative one with padding, proposed above.

And by the way, it would be nice to update the examples from the documentation for the latest versions of the plugin if they include any critical updates.

davidjerleke commented 1 year ago

Hi again @7iomka,

No it's actually working as expected. As I explained here:

Embla just reads slide dimensions and the distance between slides when initialised, it doesn't modify any margins or slide widths.

This is described here in the docs.

So Embla leaves your container exactly as you set it up with CSS. What happens to a container where the first child has margin-left and the last child has margin-right, without the Embla script being involved? You get these spacings you're describing as "unexpected" of course. It seems like many devs have a hard time understanding that Embla gives you nearly total freedom when setting up your HTML and CSS, in contrast to many other carousel libs that add all sorts of styling on your elements. There are benefits to the Embla approach:

One downside to this approach is that:

Now sorry for not being clear. In your example, the first slide has margin-left and last slide has margin-right. That's why you're seeing these gaps.

And by the way, it would be nice to update the examples from the documentation for the latest versions of the plugin if they include any critical updates.

There's an issue that will sync the example CodeSandboxes with the latest documentation website examples which will prevent examples from getting out of sync, but I haven't had the time to do much there yet:

It's easy for devs to just come here and say that it should be like this or that. I'm the sole developer and I simply don't have enough time to maintain the core library, all the wrappers, almost all the plugins, the documentation and the examples. It's an incredible amount of work with no salary. So if you want everything to be up to date you can either make contributions or consider funding this project.

Kindly, David