Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The Slider is skipping other items #38

Closed
leah-cc opened this issue Dec 13, 2019 · 33 comments · Fixed by #180
Closed

The Slider is skipping other items #38

leah-cc opened this issue Dec 13, 2019 · 33 comments · Fixed by #180
Labels
feature request New feature or request resolved This issue is resolved

Comments

@leah-cc
Copy link

leah-cc commented Dec 13, 2019

Hello, when users swipe hard enough it looks like its skipping other slides, how can we limit it to only one slide per swipe? slidesToScroll: 1 doesn't seem to do anything

const wrap = document.querySelector('.js-home-slider');
  const viewPort = wrap.querySelector('.js-home-slider-viewport');
  const dots = wrap.querySelector('.js-home-slider-dots');
  const homesliderStart = EmblaCarousel(viewPort, {
    loop: true,
    draggable: true,
    dragFree: false,
    speed: 10,
    startIndex: 0,
    slidesToScroll: 1,
  });
@davidjerleke
Copy link
Owner

davidjerleke commented Dec 15, 2019

Hello @leah-cc,
Thank you for your question 👍.

The short answer is:

That this is by design and it's intended to work this way.

The long answer is:

That the Embla Carousel animation engine is different from other carousel plugins. It animates by simulating physics and I made this design choice because I wanted a carousel that feels natural to drag and interact with. If I were to limit swipe gestures to progress only one slide, no matter how vigorous the swipe gesture was, that would make the movement arbitrary as opposed to natural, and would go against the very core of how Embla works.

Most available carousel plugins only determine which direction the carousel was dragged and decide whether the carousel should move to the next or the previous slide. If you think about it, this behavior is very unnatural. Because if you were to drag something in real life with a lot of force, it would move further away than if you were to drag it with less force. So Embla not only determines which direction the carousel was dragged, it also measures the drag force. Based on the drag force it will or will not skip slides. It all depends how hard the users drag it.

We have a new browser standard emerging for horizontal (and vertical) scroll areas on websites called CSS scroll snaps, and they also work this way. If you're interested, take a look at this CSS scroll snap example and try dragging it on mobile. I decided to go with the same drag design choice for Embla Carousel. I hope I've managed to explain why I made this choice and that all of this makes sense to you. 

Kindly,
David

@davidjerleke davidjerleke added the awaiting response Issue is awaiting feedback label Dec 15, 2019
@gilbertlucas46
Copy link

Hi @davidcetinkaya Thank you for the explanation. But Is there a way to adjust the threshold swipe? Because even with very light swipe it sometimes skip items from 1 - 3 or 1 - 5. Many thanks!

@davidjerleke
Copy link
Owner

davidjerleke commented Dec 19, 2019

Hello @gilbertlucas46,
Thank you very much for your input on this 👍.

At the time of writing there's no way to control this externally. If you don't mind, I'd appreciate if you could share a CodeSandbox or a link that demonstrates the issue you're experiencing. Maybe there's something I'm missing here and a demonstration link may help. Because if it's from 1 - 5 it sounds like Embla may be setup in a way it's not intended to be used.

If you're interested, open this CodeSandbox demonstration in a mobile browser. It's a demonstration where you can compare swipe experience between Embla and CSS Scroll Snaps.

Best,
David

@gilbertlucas46
Copy link

Hello, thank you for your reply. If you try to open your. example on mobile. you can actually see what I mean. https://webm.red/view/wrFu.webm

@davidjerleke
Copy link
Owner

davidjerleke commented Dec 19, 2019

Hi @gilbertlucas46,

Thank you for the screen recording 👍.

One thing I noticed: I don't think it's fair to use the dev tools with mouse to simulate touch device swiping, because it’s not an accurate touch swipe simulation. This is because a mouse on a desktop can swipe with much more force than you could ever reproduce on a real touch device with your thumb. And your users will not use dev tools when swiping. Have you tried this on a real touch device? My recommendation is that you try this on an actual touch device like a mobile or tablet.

What I wanted to convey with the demonstration is that Embla Carousels's drag functionality is designed to work like CSS Scroll Snaps so try swiping in the same manner on the carousel below and they should behave pretty much the same (the carousel below is pure CSS). Both will skip slides if you provide enough drag force.

It wouldn't be natural to not skip slides when decent drag force is applied. That would make the carousel feel stiff, like moving a refrigerator. The movement would be arbitrary just like simple easing as opposed to fluid and natural.
Maybe we should consider exposing an option that makes it possible to configure the friction applied to the carousel, but even then it wouldn't be natural to prevent skipping slides if the user drags the carousel with decent force.

@davidjerleke davidjerleke added feature request New feature or request not planned Won't be investigated unless it gets lots of traction and removed awaiting response Issue is awaiting feedback labels Dec 24, 2019
@davidjerleke
Copy link
Owner

I’m closing this issue for now. Feel free to reopen it if you want to discuss it further. Thank you!

@davidjerleke davidjerleke added not planned Won't be investigated unless it gets lots of traction and removed not planned Won't be investigated unless it gets lots of traction labels Jan 20, 2020
@akardet
Copy link

akardet commented Sep 30, 2020

Hi David,

It's me again! You mention exposing an option to configure the friction of the drag force. Is this something that can be explored?

Thanks,
Sam

@davidjerleke
Copy link
Owner

Hello again Sam (@akardet),

Thank you for your question. That’s correct. I mentioned that as a suggestion in this issue but didn’t get any response from the issue author. So the discussion basically died and the friction option hasn’t been implemented.

What I had in mind when I mentioned that I could setup a CodeSandbox demonstrating what you want to achieve, was extending Embla with its undocumented method dangerouslyGetEngine(). It has been mentioned in issue #86 in this comment an basically is there for the following purposes:

About the dangerouslyGetEngine(), yes it’s not documented but it’s safe to use if you know what you’re doing 🙂. The method basically exposes most of the internal Embla Carousel engine, and is intended to aid plugin authors with maximum extensibility. Another purpose is that it enables for extending the carousel with features that either won’t be added to the core itself, or features that will be released later on.

Are you interested in a solution using this approach?

Kindly,
David

@akardet
Copy link

akardet commented Oct 1, 2020

Ahh I see, yeah it would be helpful to see a CodeSandbox example if it's not too much trouble.

Thanks,
Sam

@davidjerleke
Copy link
Owner

davidjerleke commented Oct 7, 2020

Hello Sam (@akardet),

As you mentioned in our previous conversation you told me that you’re aware of the design choice with the momentum scrolling. If you still want to use Embla but change that behavior I don’t mind trying to achieve this and create a CodeSandbox for you, like I suggested here.

I want to mention that I’ve not been able to work on this project for a period now and I’m hoping to be able to spend some time on it soon. But at the time of writing I don’t know when that will be possible.

Kindly,
David

@davidjerleke
Copy link
Owner

For anyone interested, maybe (@leah-cc, @gilbertlucas46, @farshidshahmoradi1996, @akardet), is this close to the desired behavior you would like to see?

embla-restrict-scroll.mp4

Kindly,
David

@jeiea
Copy link

jeiea commented Apr 5, 2021

@davidcetinkaya Watching at the clip, I expect threshold for scroll to be decreased. But maybe that will be problem of parameter.
I'll be satisfied with dangerouslyGetEngine() way if you don't want to expose public API for this. Could you provide any hint for this? Currently I stucked at how to get current scroll speed of embla. It seems engine.scrollBody have that, but I can't access to it.

@davidjerleke
Copy link
Owner

Hi @jeiea,

I expect threshold for scroll to be decreased

I'm not following. How do you mean?

I'll be satisfied with dangerouslyGetEngine() way if you don't want to expose public API for this.

It seems like a lot of people want this so I think I'm going to release this publicly, but I can't give an ETA for it. Recently, I've had a little time to look over some parts of the code that can be simplified so it will be possible to add this feature without adding to the current bundle size.

Best,
David

@jeiea
Copy link

jeiea commented Apr 5, 2021

I think required drag distance to swipe is too long. That maybe due to wide width but it can be shortened. And even if drag distance is short, I will admit it as swipe if speed is fast enough. Because it would be matter of parameter, it's consistent with my expectation overall.

@davidjerleke
Copy link
Owner

davidjerleke commented Apr 5, 2021

@jeiea, the required drag distance may look like it's long in the video, but it actually isn't. This is because you can't see where I released the pointer in the video.

@jeiea
Copy link

jeiea commented Apr 5, 2021

I expected all swipe cause scroll. But it didn't. What makes difference?

@davidjerleke
Copy link
Owner

@jeiea I'm just moving the mouse before I actually start dragging (performing a pointerdown and pointerup).

@davidjerleke
Copy link
Owner

davidjerleke commented Apr 5, 2021

@jeiea The bottom line is, you can’t see where I release the mouse button after I’ve dragged the carousel on the video, so you can’t actually tell how far I’ve dragged it. This is because there’s nothing that visually indicates this. I’m not using a grab cursor when the mouse button is down for example.

I think required drag distance to swipe is too long

So your assumption about drag distance is probably not accurate in this case.

@jeiea
Copy link

jeiea commented Apr 5, 2021

Ah, I had a illusion that 2, 3, 5th swipes failed to scroll to the next slides. Perhaps reverse elasticity gave me that illusion.

@davidjerleke
Copy link
Owner

@jeiea ah I see. Your comment makes sense now. Every drag move succeeds in the video. It’s just that the 1st and the 4th drag is with less force so it doesn’t bounce back. As you realized, it’s reversed elasticity 👍🏻.

@davidjerleke davidjerleke reopened this Apr 17, 2021
@davidjerleke davidjerleke added investigating Issue is being looked into and removed not planned Won't be investigated unless it gets lots of traction labels Apr 17, 2021
@davidjerleke
Copy link
Owner

@jeiea I have a working draft of this feature now. Care to try it out? If yes, please tell me if you prefer a VanillaJs or ReactJs CodeSandbox setup.

Best,
David

@jeiea
Copy link

jeiea commented Apr 18, 2021

I prefer react one.

@davidjerleke
Copy link
Owner

@jeiea thank you for your swift response. I'll let you know when it's ready.

@davidjerleke
Copy link
Owner

davidjerleke commented Apr 18, 2021

@jeiea here are the links:

The new option is called skipSnaps. When it's set to false, it will enable the described behavior in this issue. At the time of writing, I'm not sure if this is going to be an option or if it's going to be the default behavior when dragFree: false. You're welcome to share your thoughts about this (and anyone else interested in this of course).

Let me know how it goes.

@davidjerleke davidjerleke added the awaiting response Issue is awaiting feedback label Apr 19, 2021
@jeiea
Copy link

jeiea commented Apr 21, 2021

@davidcetinkaya Sorry for late reply. I showed it to my team, and it seems good.

@davidjerleke
Copy link
Owner

davidjerleke commented Apr 21, 2021

@jeiea thanks for taking time to test this. I'll release this as soon as I get the chance with v4.5.0. Note that you need to set your options to skipSnaps: false to enable this then.

In the next major version (which will be 5.0.0), this will be the default behavior though.

@jeiea
Copy link

jeiea commented Apr 21, 2021

I just applied it to the real project and found some strange behavior. It seems when loop: true both ends of container are not regarded as snaps. Could you check it?

@davidjerleke
Copy link
Owner

davidjerleke commented Apr 21, 2021

@jeiea sorry, I forgot to update the CodeSandbox code to the latest. I solved this issue yesterday. Try the CodeSandbox here again and let me know if we're talking about the same issue.

@jeiea
Copy link

jeiea commented Apr 21, 2021

I confirmed that it is the same issue. Thank you.

@davidjerleke davidjerleke added upcoming A feature or bug fix is on its way for this issue and removed investigating Issue is being looked into awaiting response Issue is awaiting feedback labels Apr 21, 2021
davidjerleke pushed a commit that referenced this issue Apr 22, 2021
@davidjerleke davidjerleke added resolved This issue is resolved and removed upcoming A feature or bug fix is on its way for this issue labels Apr 22, 2021
@davidjerleke
Copy link
Owner

davidjerleke commented Apr 22, 2021

@jeiea this feature has just been released with v4.5.0. Thank you for taking time to test this out.

Enjoy,
David

@jeiea
Copy link

jeiea commented Apr 22, 2021

It's nitpicking but the read more link in release description linked to localhost.

@davidjerleke
Copy link
Owner

davidjerleke commented Apr 22, 2021

Thank you @jeiea. That link is important so it's not nitpicking. I've updated the link now 👍.

@akardet
Copy link

akardet commented Apr 22, 2021

Just added the new option to my app and it's working flawlessly! Thanks for the great work, David 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request resolved This issue is resolved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants