search

davidjerleke / embla-carousel

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

⭐ v7 #298

Closed davidjerleke closed 2 years ago

davidjerleke commented 2 years ago

v7

New features

Changes

Bugfixes

Breaking Changes

Features that will be removed

jordanpacheco1 commented 2 years ago

Hi, how's it going? Do you have an estimate to when this version will be released?

davidjerleke commented 2 years ago

Hi @jordanpacheco1,

You can follow the progress here. A checked checkbox means that the task is complete. But I can't give an estimate because I'm maintaining this library on my unpaid spare time. Thank you for understanding.

Best, David

brniamut commented 2 years ago

@davidjerleke Hey David!, if you need any help let me know. The organisation I work is kind of needs the update and I have the time to work on the library if needed. If you have a more detailed list, or a task I can work on, let me know.

davidjerleke commented 2 years ago

Hi @brniamut,

Thank you and very nice of you to offer help. I only have one thing left to finish actually: Because I'm introducing the breakpoints option, Embla will internally check if any options have changed depending on the media query you provide like so (when the window is resized):

// VanillaJS
const embla = EmblaCarusel(emblaNode, {
  loop: false,
  breakpoints: {
    '(min-width: 768px)': { loop: true }
  }
});

// ReactJS
const [emblaRef, emblaApi] = useEmblaCarousel({
  loop: false,
  breakpoints: {
    '(min-width: 768px)': { loop: true }
  }
});

For this to work I'm writing an options diff check function which will be used to determine if any options have changed. That's actually the only part left to do so getting close to wrapping up now. Note that plugins will also accept breakpoint options. I'll add CodeSandboxes soon so I would very much appreciate if you would like to test it so we can find any critical bugs before the release. I'll let you know when it's ready.

Best, David

brniamut commented 2 years ago

@davidjerleke at you service!

davidjerleke commented 2 years ago

@brniamut I've added CodeSandboxes to the original post now 👍. Please read the following specification before testing the breakpoints feature:

Best, David

ketijakrista commented 2 years ago

Hey, any chance this is released next week? And thank you for the hard work <3

davidjerleke commented 2 years ago

Hej @ketijakrista,

I don't have much left to do with the packages. The documentation website needs to be updated too so that will take some time. But I can't give any guarantees. I don't want to rush things because that's how you introduce a lot of bugs. If you want to speed up the process you can test the current state of v7. Both the new features and existing features. CodeSandboxes are to be found in the original post.

Kindly, David

davidjerleke commented 2 years ago

Hello folks,

I’m getting real close to a release candidate so stay tuned.

If anyone knows how to solve this:

Please let me know. It only affects CDN users but it’s still needed before a stable v7 release.

Cheers, David

madebyfabian commented 2 years ago

Hi there,

I am currently evaluating nearly all react sliders and yours is still the best. So I would love to use it, but I need especially the responsive feature in v7. Is there anything I could do to support you releasing v7?

Best, Fabian

davidjerleke commented 2 years ago

Hi @madebyfabian,

Thank you very much for offering your help. Yesterday I finished the responsive feature for plugins. At the time of writing, I only have some small things left to do to wrap v7 up. So maybe it's not optimal to join at this stage. But, as soon as the release candidate is out, I'm going to need help with the following:

Cheers, David

davidjerleke commented 2 years ago

Hello all (@brniamut, @madebyfabian, @ketijakrista ...),

I've published the v7 release candidate v7.0.0-rc01. Be sure to read the release details before trying it out. Any information about how the new features are supposed to work can be found in each individual issue listed in the release details.

Cheers, David

ruijdacd commented 2 years ago

Hi @davidjerleke,

Appreciate all the work you've put into Embla Carousel! It's absolutely amazing what you've built.

Do you have an estimate for when you think v7 will become stable? This week I'll try out the RCs across our usage at Plum Guide and raise any issues that I come through.

Cheers, Rui

davidjerleke commented 2 years ago

Hi @ruijdacd,

I don’t have an estimate because users have to test it for a period of time before it can be considered stable. Many people have their vacation in July so the number of users is significantly lower during this month. And the feedback I’ve got so far hasn’t been substantial really.

I can say that feature wise it’s pretty stable because I don’t plan to introduce more breaking changes at this point. But please bear in mind that this IS a release candidate so I can’t make any promises. If there’s a very good reason I might introduce more breaking changes, but I would say that it’s not likely. And if I were to add a feature or two, that will obviously not break anything for anyone accustomed to the v7 RC.

Best, David

davidjerleke commented 2 years ago

Hi all,

If any of you guys have tried the release candidate I would appreciate feedback if you’ve encountered potential bugs, or if you’ve tried it and it was working as expected because that’s also useful feedback.

Thanks in advance.

Best, David

gruberro commented 2 years ago

I've tested the latest release candidate, all worked as expected 🙌 .

MaxDesignFR commented 2 years ago

I've tested breakpoints, nothing to report. One thing though, and I'm not sure if it's due to this release candidate or prior, but in some cases, the scroll behavior seems inconsistent when combined slides width in viewport is slightly less than 100%.

For instance: 3 slides for 100% width (4 slides in total)

The second case scenario always behave well in my testings (since I presume total width of slides are just a notch above 100%). But at some screen sizes, the first case scenario works too 🤨. It could be tempting to use 33.33%, but my understanding is that it should be at least 33.4% to work properly (in some scenarios like the one I demonstrated, which is not far stretched). It's minor, but maybe this could be easily fixed internally.

davidjerleke commented 2 years ago

@MaxDesignFR and @gruberro thank you for the feedback 👍🏻.

@MaxDesignFR, would you mind sharing what options and CSS you’re using. Or even better, a CodeSandbox with the full setup?

Best, David

MaxDesignFR commented 2 years ago

@MaxDesignFR and @gruberro thank you for the feedback 👍🏻.

@MaxDesignFR, would you mind sharing what options and CSS you’re using. Or even better, a CodeSandbox with the full setup?

Best, David

Sure, I made 2 identical CodeSandboxes, the only difference being the flex-basis (33.33% vs 33.34%) :

33.33% version: https://codesandbox.io/s/embla-carousel-default-vanilla-forked-bzit5q?file=/src/css/embla.css:525-542 33.34% version: https://codesandbox.io/s/embla-carousel-default-vanilla-slides-3-slides-33-34-g684o7?file=/src/css/embla.css:525-542

Screen capture from the CodeSandboxes to illustrate my point: https://i.vgy.me/WEvkZF.gif

davidjerleke commented 2 years ago

Thank you @MaxDesignFR, very helpful 👍! Embla is actually behaving as expected here. Here's what the docs say about the loop option:

Automatically falls back to false if slide content isn't enough to loop.

What's happening in first example is that Embla makes the decision that the three slide widths combined aren't sufficent for the loop effect to work without any noticeable visual glitches. So it falls back to loop: false;.

You might think:

Ok, but I have three slides that cover 100% of the viewport? So it should be able to flip around the fourth slide without any visual glitches?

The answer is no:

Because the slides have to cover 100% of the viewport plus 1px-3px to take scrolling into account (animating). I hope my explanation makes sense. Here's what the upcoming v7 docs say about the loop option:

Enables infinite looping. Automatically falls back to false if slide content isn't enough to loop. Embla will apply translateX or translateY to the slides that need to change position in order to create the loop effect.

Please let me know if you have any ideas of how to improve the explanation if you think it's insufficient. This might help others to understand why this is happening better.

Cheers, David

MaxDesignFR commented 2 years ago

What's happening in first example is that Embla makes the decision that the three slide widths combined aren't sufficent for the loop effect to work without any noticeable visual glitches. So it falls back to loop: false;

@davidjerleke Thank you for further details, I came to this understanding as well. But it's taken me some time to figure this out, on my first project using Embla I got stuck thinking this couldn't loop because I didn't have enough slides outside of viewport, when actually I could just have updated the slides width — let's say 5 x 20.01% instead of 5 x 20% — in order for the loop to work.

So my feedback was to ask if the library could manage to detect and somehow take into consideration this very tiny margin of error. I don't know whether it's easily doable, but it would definitely be more developer-friendly, because I don't think this "issue" is obvious to understand unless messing around with the code for a while (at least I did).

Otherwise, maybe more than sufficient, update the docs to add a simple "how it works" explanation. Such as :

Enables infinite looping. Automatically falls back to false if slide content isn't enough to loop (total width of the slides must be greater than 100%). Embla will apply translateX or translateY to the slides that need to change position in order to create the loop effect.

Maybe not like that but you get my point. As far as I'm concerned it's not an issue since I understand this cog now, but I believe there is added value for the (new) users to document this behavior.

davidjerleke commented 2 years ago

@MaxDesignFR Thank you for your response.

But it's taken me some time to figure this out, on my first project using Embla I got stuck thinking this couldn't loop because I didn't have enough slides outside of viewport, when actually I could just have updated the slides width

I think you make a good point and we should do something to minimize the risk of confusion or running into this problem. As you already have made clear, there are two ways of tackle this:

  1. Do something with the Embla core.
  2. Update the docs so it communicates this better.

So my feedback was to ask if the library could manage to detect and somehow take into consideration this very tiny margin of error. I don't know whether it's easily doable, but it would definitely be more developer-friendly, because I don't think this "issue" is obvious to understand unless messing around with the code for a while (at least I did).

If the library were to do something when this happens it has to detect the problem and probably touch the DOM in a way which it doesn't normally do. v7 only manipulates transform: translate on the container and the slides that need to change position in order to create the loop effect. It would also increase bundle size a bit. There are probably other ways of dealing with this so I'm open to suggestions if you have something in mind.

Maybe not like that but you get my point. As far as I'm concerned it's not an issue since I understand this cog now, but I believe there is added value for the (new) users to document this behavior.

100%. We want to get rid of the confusion. By the way, have you tried setting the widths like this instead:

.embla__slide {
  flex: 0 0 calc(100% / 3); 
}

// ...or:

.embla__slide {
  flex: 0 0 calc(100% / 3);
  width: calc(100% / 3);
}

...does that behave like expected instead of the unpredictable flex: 0 0 33.333333333333336%? If yes, maybe the docs should recommend users to prefer the calc() approach instead? Please note that this only seem to work with v7 that uses px instead of % and not previous versions. This would be one way to communicate it:

docs

Let me know what you think and thank you for your time.

Best, David

MaxDesignFR commented 2 years ago

@davidjerleke Unfortunately I haven't familiarized myself much with Embla code base, so I don't have a clue if there's a clever way to do that in Embla core. But we enjoy Embla cause it's light, so I guess right now, updating the doc is better than nothing (it would allow devs to understand the requirement for loop to work properly, and eventually save time).

...does that behave like expected instead of the unpredictable flex: 0 0 33.333333333333336%? If yes, maybe the docs should recommend users to prefer the calc() approach instead? Please note that this only seem to work with v7 that uses px instead of % and not previous versions. This would be one way to communicate it:

The 33.333333333333336 was generated by a template language (liquid). But I think what matters is how it's computed by the browser. In the computed tab of the dev console, it seems to go up to 4 decimals. I tried with calc(100% / 3) and flex-basis is computed as 33.3333% which — multiplied by 3 — is not strictly superior to 100%, and therefore the slider will not work as expected if those things are combined: • Loop is activated • There is only one slide that do not fit into the viewport (eg: 3 slides in viewport at 33% + 1 slide outside viewport) • Total width of the slides in viewport is not strictly superior to 100%

This is what I highlight here: https://codesandbox.io/s/embla-carousel-default-vanilla-3-slides-calc-100-3-forked-y9py9w?file=/src/css/embla.css:525-550

calc(100% / 3) does not fix the issue since it does not solve bullet point number 3. But calc(100.01% / 3)` would (computed as 33.3367). So I see 2 approaches with my current understanding:

1/ The doc. states that their must be at least 2 extra slide outside of viewport for the loop to work properly. 2/ If there is only 1 extra slide outside of viewport, the total width of the slides in viewport must be slightly over 100% to loop properly.

I hope it makes sense. It nearly seems like a niche problem, and yet It's safe to say other people may encounter it.

davidjerleke commented 2 years ago

@MaxDesignFR thanks.

After the release of stable v7 I will spend some time to see if there's a simple way for Embla to handle this internally. But for now I will add something to the docs. I think this is a good starting point so thank you for this explanation (I might use a modified version of it if that's ok?):

1/ The doc. states that their must be at least 2 extra slide outside of viewport for the loop to work properly. 2/ If there is only 1 extra slide outside of viewport, the total width of the slides in viewport must be slightly over 100% to loop properly.

About this:

calc(100% / 3) does not fix the issue since it does not solve bullet point number 3. But calc(100.01% / 3)` would (computed as 33.3367).

Did you actually try calc() with the version 7 release candidate as I tried to communicate in my previous comment?

Because calc(100% / 3) did solve the problem for me when I tried v7rc. In other words it didn’t fall back to loop: false;. Because this CodeSandbox is using v6.2.0:

This is what I highlight here: https://codesandbox.io/s/embla-carousel-default-vanilla-3-slides-calc-100-3-forked-y9py9w?file=/src/css/embla.css:525-550

Because from my tests, v7 behaves different than v6.2.0 and lower. This is (as mentioned in my previous comment) because the move from % to px seems to make the internal calculations more precise since px is an absolute unit. Sorry for repeating myself if you did try calc() with v7 but I just wanted to make sure you picked that up.

Thank you for your valuable feedback!

Cheers, David

MaxDesignFR commented 2 years ago

After the release of stable v7 I will spend some time to see if there's a simple way for Embla to handle this internally. But for now I will add something to the docs. I think this is a good starting point so thank you for this explanation (I might use a modified version of it if that's ok?):

Yes sure, happy I could help.

Did you actually try calc() with the version 7 release candidate as I tried to communicate in my https://github.com/davidjerleke/embla-carousel/issues/298#issuecomment-1195891650?

I tried v7 pre-release on my end and I encountered the same issue unfortunately. Only it seems with v7 sometimes loop works, sometimes not, depending on the current screen size. I just changed this codesandbox to v7 (https://codesandbox.io/s/embla-carousel-default-vanilla-3-slides-calc-100-3-forked-y9py9w?file=/src/css/embla.css:525-550), and I notice that on narrower screen sizes (tablet/mobile), it is still not looping properly, whereas it loops well on larger screen size, so it still seems inconsistent. From what I observe in this codesandbox, the loop seems to fallback to false once the Embla viewport default size gets shrunken down by the screen viewport size, so that may be a starting point for you.

codesandbox gif preview before/after resize: https://i.vgy.me/Tom5Ce.gif

davidjerleke commented 2 years ago

Splendid @MaxDesignFR. Thanks for your time and trying it out.

After I’ve released v7, I’ll let you know when I have had the possibility to try modifying the core and we will see if this is easily fixed or not.

davidjerleke commented 2 years ago

Hi again @MaxDesignFR,

It might be an easy fix after all. Would you mind trying this CodeSandbox out? If you can't see any visual glitches when the carousel is looping, this might be the solution. If it works as expected, I'll add this to v7.

Thanks in advance.

Best, David

MaxDesignFR commented 2 years ago

Hi @davidjerleke,

That sounds about perfect, it did the trick! Really neat that you could fix this so quick before the next release. I'm curious what's the fix you found out to solve this problem? (don't give yourself a headache if there's too much concept to explain, I'm just curious, developer syndrom to know how things gets fixed aha)

davidjerleke commented 2 years ago

Hi @MaxDesignFR,

Glad to hear that it worked. I have some testing in other browsers left to do but let's hope that they will behave the same.

I'm curious what's the fix you found out to solve this problem?

I changed the value on this line to 0.1 (which equals 0.1px). What this line does is that it basically checks if the total width of the slides in viewport (while shifting positions of other slides) equals the size of the viewport - In other words if the carousel will be able to loop without any visual glitches.

And I thought setting it to 0.1 would result in a noticeable glitch when looping slides but it seems like I was wrong. I never thought it would work. I guess the fractional px value 0.1px is enough.

Cheers, David

davidjerleke commented 2 years ago

Released with v7.