Splidejs / splide

Splide is a lightweight, flexible and accessible slider/carousel written in TypeScript. No dependencies, no Lighthouse errors.
https://splidejs.com
MIT License
4.89k stars 422 forks source link

Swiping on apple mac trackpad scrolls two slides and ignores FlickMaxPages setting #618

Closed waynep16 closed 2 years ago

waynep16 commented 2 years ago

Checks

Version

3.6.9

Description

When using TTB direction slider and WHEEL true and RELEASEWHEEL True , swiping past a slider on an apple track pad (not a physical mouse wheel) causes two slides to swipe instead of one at a time,

Settings are

var splide = new Splide( '#chapter_list', {
  direction: 'ttb',
height   : '70vh',
 wheel       : true,
 releaseWheel: true,
updateOnMove :true,
cloneStatus :false,
flickMaxPages :1,
waitForTransition :true,

} );

Reproduction Link

https://www.travelport.com/plus/simpler-travel-retailing

password = go6

Steps to Reproduce

  1. Scroll down to the slider where it says LATEST RELEASE
  2. swipe your trackpad naturally and you will see it swipe past two slides instead of one

Expected Behaviour

Only one slide should swipe at a time

realjoshharrison commented 2 years ago

I'm also experiencing this. It happens because the Apple trackpad and magic mouse emit wheel events for a while after the scroll gesture has been released. Here's a reduced test case: https://jsbin.com/saguqeqixu/edit?html,js,output

There's probably a better way to do this, but through trial and error in the past the only way I found to detect these "residual" wheel events was by debouncing the event with a timeout of 300ms before considering it to be a new gesture. This might give a good starting point for a solution? E.g. https://jsbin.com/newesorofo/1/edit?html,js,output (I've only included logic to advance the slide on wheel for this quick mockup)

idiotWu commented 2 years ago

My approach is to compare the difference of evt.deltaY between two wheel events, and scroll only if:

  1. previous.deltaY * current.deltaY < 0, which means the scroll direction changed, or
  2. abs(current.deltaY) > abs(previous.deltaY), which means the user was likely to start a new scroll, as the delta value should continue to descend in an ideal inertial scroll (e.g. 42 -> 24 -> 15 -> ... -> 0)

In practice, you may also need to set a threshold to reduce noise:

const threshold = 10;
let lastDeltaY = 0;
let isTrackpad = false;

el.addEventListener('wheel', (e) => {
  const { deltaY } = e;
  const diff = Math.abs(deltaY) - Math.abs(lastDeltaY);

  // additional check for trackpads
  if (!isTrackpad) {
    isTrackpad = diff !== 0;
  }

  const isNewScroll = isTrackpad ? diff > threshold : true;
  const isDifferentDirection = deltaY * lastDeltaY < 0;

  lastDeltaY = deltaY;

  if (
    splide.state.is(Splide.STATES.IDLE) &&
    (isDifferentDirection || isNewScroll)
  ) {
    splide.go(deltaY > 0 ? '>' : '<');

    // reset the flag in case that the user changes devices
    isTrackpad = false;
  }
});

Full-page demo: https://kgov1.csb.app Sandbox: https://codesandbox.io/s/splide-wheel-kgov1

waynep16 commented 2 years ago

full-page demo: https://kgov1.csb.app/

Unfortunately, I am still able to scroll multiple slides at once on your demo using the mouse wheel.

demo https://www.loom.com/share/615934a846f64e7dbf186cad812c6db1

idiotWu commented 2 years ago

@waynep16 it's a by-design behavior if you continuously scroll the mouse wheel. Otherwise I think it's a bit weird to see the slider stuck at one scroll. 🤣

ogelin commented 2 years ago

@idiotWu I was also facing this issue and the above solution works great. Thanks.

waynep16 commented 2 years ago
el.addEventListener('wheel', (e) => {

I am getting el is not defined ReferencError here.

BartekKonopka commented 2 years ago

@waynep16 please, check this solution:

var splide = new Splide( '#chapter_list', {
    type: 'fade',
    wheel : false,
} );

splide.mount();

function handleWheel(e) {
    wDelta = e.wheelDelta < 0 ? 'down' : 'up';
    if( wDelta == 'down' ) {
        splide.go('>');
    } else {
        splide.go('<');
    }

    window.removeEventListener("wheel", handleWheel);

    setTimeout(()=>{
        window.addEventListener("wheel", handleWheel);
    }, 1000); 
}

window.addEventListener("wheel", handleWheel);
waynep16 commented 2 years ago

@waynep16 please, check this solution:

That is in place here https://www.travelport.com/plus/simpler-travel-retailing but still no joy :(

idiotWu commented 2 years ago
el.addEventListener('wheel', (e) => {

I am getting el is not defined ReferencError here.

@waynep16 el is the element you initialized splider to, for example:

const splider = new Splide('#container');

const el = document.querySelector('#container');

el.addEventListener(...);
NaotoshiFujita commented 2 years ago

The key of this issue is how to deal with the decrementing delta of the inertial scroll caused by trackpads and some trackballs. Currently, Splide treats any delta as a scroll and moves the slider because if there is a delta, it should consider that scroll is requested regardless of its amount. On the other hand, Splide quantizes scrolling as the slide transition, it can be inconsistent to treat that all deltas have same values (to move the slider).

Since there is no common solution for all use cases, I'm planning to add the configurable threshold for the delta. You can try the demo here (I stole Wu's demo): https://codesandbox.io/s/wheel-threshold-g9snk

realjoshharrison commented 2 years ago

Thanks for taking a look @NaotoshiFujita!

It is a tricky issue. The configurable threshold works nicely to trigger just one slide change for a "strong" scroll on my macbook trackpad, but a gentle scroll now results in no slide change at all, unless the threshold is reduced to the point that the original issue occurs again on "strong" scrolls.

Building on the example, I've found what I hope is a good solution for all use cases... by comparing the current event delta with the previous event delta, we can detect an increase in momentum (ie a new scroll) and trigger a slide change accordingly regardless of the scroll strength and without needing to wait for a debounce timer to finish: https://jsbin.com/nejiyeqine/1/edit?js,output

I haven't tested this on a normal mouse wheel yet, only a trackpad. I will have a go tomorrow and report back.

NaotoshiFujita commented 2 years ago

I'm not aiming to limit the number of slides per "trackpad scroll" to 1. If the user makes a "strong" scroll, that means he/she wants to scroll a lot. Limiting the number to 1 often leads terrible UX. I understand the developer's intention to show slides one by one. It might be okay for the first visitor, but returning users may want to see the middle or the last slide. If there are 10 slides, the slider always forces users to swipe 10 times. Restricting the number kills the nice wheel advantage.

What I have to solve is that a "gentle scroll" unexpectedly skips a next slide, caused by the trackpad inertia. By the threshold above, developers can determine what is "gentle" and what is "strong" (they can also adjust the transition duration to wait the inertia somewhat).

Unfortunately, observing the delta change is untrustworthy. A mouse scroll sometimes gets stuck in your example. I think it's because my mouse emits different deltaY according to my scroll strength. Besides, surface/logicool trackpad emits both decrementing deltas and constant deltas. For example on the logicool (surface's is not like this):

300, 300, 300, 300, 300, 200, 200, 200, 200, 150, 150, ......

So, there is no way to detect trackpad-like devices just by observing the momentum.

realjoshharrison commented 2 years ago

Thanks for the detailed reply and taking the time to look and explain 👍 your rationale does make sense for most use cases. And yes, on my "normal" wheel mouse, my demo doesn't work so nicely... it gets stuck.

In my case I'm going to have to go with debouncing the wheel event, to give the behaviour that the designer is requesting, which in this case does make sense to me. In future, hopefully the threshold setting will be enough

NaotoshiFujita commented 2 years ago

What is the purpose of your debouncing? To detect 300ms duration without wheel input?

waynep16 commented 2 years ago

My initial theory was that flickMaxPages would always limit the number of slides that would be scrolled regardless but it seems a STRONG scroll on a mouse wheel or trackpad was ignoring this and scrolling x amount. When using releaseWheel: true and the slider vertically full width this is vital to ensure only one slide goes at a time - If we had the option to ensure this then that would be great. UX wise, scrolling one slide only despite the force of a scroll is expected behaviour give than you want to use releaseWheel: true, therefore sticking the content until the end.

realjoshharrison commented 2 years ago

What is the purpose of your debouncing? To detect 300ms duration without wheel input?

Basically yes. Essentially I need to collapse all wheel events per "scroll gesture" into a single wheel event, in order to move only one slide at a time. I've just re-tested and found 75ms seems to work for this.

My initial theory was that flickMaxPages would always limit the number of slides that would be scrolled regardless

Correct me if I'm wrong but I believe flickMaxPages is only for limiting touch gestures, not mousewheel events... but I think the idea is the same, we're looking for a way to achieve something like wheelMaxPages

NaotoshiFujita commented 2 years ago

Basically yes. Essentially I need to collapse all wheel events per "scroll gesture" into a single wheel event, in order to move only one slide at a time. I've just re-tested and found 75ms seems to work for this.

I understand. I've tested it on my side and found the apple trackpad looks okay, but the surface trackpad keeps emitting wheel events around 3sec per action. If I make next scroll in the duration, the slider gets stuck. How do you deal with the issue?

Correct me if I'm wrong but I believe flickMaxPages is only for limiting touch gestures, not mousewheel events... but I think the idea is the same, we're looking for a way to achieve something like wheelMaxPages

Right. flickMaxPages is for drag and touch events, not related with wheel events.

realjoshharrison commented 2 years ago

the surface trackpad keeps emitting wheel event around 3sec per action. If I make next scroll in the duration, the slider gets stuck. How do you deal with the issue?

Yep that's the unfortunate trade off with this solution. To help, I've given an alternative way to advance the slides that is not dependent on wheels, users can just click to advance one slide per click with no throttling.

NaotoshiFujita commented 2 years ago

I see. I should think about the solution more.

NaotoshiFujita commented 2 years ago

@waynep16 In your case, @realjoshharrison's solution perfectly matches what you desire. It ensures only one move per scroll. Integrate his demo to your project if you are in a hurry. I need time to solve the issue. https://jsbin.com/newesorofo/1/edit?html,js,output

waynep16 commented 2 years ago

I must be going mad because in the example https://jsbin.com/newesorofo/1/edit?html,js,output wheel : true is not specified yet it is working with the mouse wheel? Could it be because jsbin loads output in an iFrame.

IF I use the code without wheel : true and releaseWheel: true no scrolling works with the mouse.

If I put back in wheel : true, on to my example we get scrolling again BUT even using exact demo code, the debounce doesn't work as per the example.

See my scroller here https://www.travelport.com/scroll-demo

Using mouse wheel rapidly scrolls past all slides but it doesn't on the jsbin demo despite using the exact code - https://i.snipboard.io/Sh4G7P.jpg

realjoshharrison commented 2 years ago

@waynep16 you shouldn't specify wheel: true or releaseWheel: true if you're using my code. Because we're making our own wheel hander you don't want the default Splide wheel handling to conflict with it.

Here's an updated example which handles scrolling up and down too: https://jsbin.com/metuxiwoga/1/edit?html,js,output

waynep16 commented 2 years ago

@waynep16 you shouldn't specify wheel: true or releaseWheel: true if you're using my code. Because we're making our own wheel hander you don't want the default Splide wheel handling to conflict with it.

Here's an updated example which handles scrolling up and down too: https://jsbin.com/metuxiwoga/1/edit?html,js,output

Thank you - but If I don't specify releaseWheel : true users cant carry on scrolling past the slides, is there a way to detect the final slide then let the user scroll normally still like releaseWheeldoes?

realjoshharrison commented 2 years ago

@waynep16 Something like this? It won't do anything if Splide is on first slide and user scrolls up, or splide is on last slide and user scrolls down, so native page scrolling will happen. https://jsbin.com/bajavodisi/1/edit?html,js,output

waynep16 commented 2 years ago

@waynep16 Something like this? It won't do anything if Splide is on first slide and user scrolls up, or splide is on last slide and user scrolls down, so native page scrolling will happen. https://jsbin.com/bajavodisi/1/edit?html,js,output

That works, thanks you

NaotoshiFujita commented 2 years ago

wheelMinThreshold and wheelSleep are added in v4. https://splidejs.com/guides/options/#wheelminthreshold https://splidejs.com/guides/options/#wheelsleep