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

Touch events on splide slider broke smooth scroll #435

Closed
tdaulbaev opened this issue Jan 21, 2022 · 11 comments
Closed

Touch events on splide slider broke smooth scroll #435

tdaulbaev opened this issue Jan 21, 2022 · 11 comments

Comments

@tdaulbaev
Copy link

tdaulbaev commented Jan 21, 2022

Environment

  • Browser: Ubuntu Desktop Chrome 97, Android mobile chrome 97
  • Version of smooth-scrollbar: 8.7.3
  • Slider: https://splidejs.com (@splidejs/react-splide) ver. 0.6.17

Current Behavior

While scrolling the page, if the touch event hits at horizontal slider with drag = true, smooth scrolling breaks.
The smooth transition disappears and a small movement of the fingers can cause an instant jump (delta Y) of 1000+ pixels.

Expected Behavior

When smooth scroll hitting dragable elements should not break smooth scrolling.

Steps to Reproduce

<smooth-scrollbar>
     <SplideSlider />
<smooth-scrollbar>

I had the same problem with the swiper slider too.

@idiotWu
Copy link
Owner

idiotWu commented Jan 21, 2022

Thanks for providing the details! Libraries like the one you are using have to call touchMoveEvent.preventDefault() to perform customized touch scrolling action. However, as per the current design, we ignore any default-prevented events and therefore the touch moving speed will not be tracked. Can you try commenting out the return statement in the following code to see if it works in your environment?

function handler(event: any) {
// ignore default prevented events
if (event.defaultPrevented) {
return;
}
fn(event);
}

@idiotWu
Copy link
Owner

idiotWu commented Jan 21, 2022

After a little debugging I found they even call evt.preventDefault() in touch-end events, and this prevents touch trackers from being released. IMHO it's totally unnecessary to do so 🤔

@idiotWu
Copy link
Owner

idiotWu commented Jan 22, 2022

Fixed in v8.7.4.

I also created a related discussion in splide: Splidejs/splide#625

@idiotWu
Copy link
Owner

idiotWu commented Jan 22, 2022

As we still ignore the default-prevented touchend event, you may need to manually save&restore option.damping, otherwise it will be locked to 0.5:

let damping = 0;
let pointerCount = 0;

container.addEventListener('touchstart', (e) => {
  // save damping
  if (pointerCount === 0) {
     damping = scrollbar.options.damping;
  }

  pointerCount++;
  // more accurate?
  // pointerCount += e.changedTouches.length
});

container.addEventListener('touchend', () => {
  pointerCount--;
  // more accurate?
  // pointerCount -= e.changedTouches.length

  // restore damping
  if (pointerCount === 0) {
    scrollbar.options.damping = damping;
  }
});

See also:

addEvent(target, 'touchstart', (evt: TouchEvent) => {
// start records
touchRecord.track(evt);
// stop scrolling
scrollbar.setMomentum(0, 0);
// save damping
if (pointerCount === 0) {
damping = scrollbar.options.damping;
scrollbar.options.damping = Math.max(damping, 0.5); // less frames on touchmove
}
pointerCount++;
});

addEvent(target, 'touchcancel touchend', (evt: TouchEvent) => {
const delta = touchRecord.getEasingDistance(damping);
scrollbar.addTransformableMomentum(
delta.x,
delta.y,
evt,
);
pointerCount--;
// restore damping
if (pointerCount === 0) {
scrollbar.options.damping = damping;
}
touchRecord.release(evt);
activeScrollbar = null;
});

@tdaulbaev
Copy link
Author

in my case, after i update to 8.7.4 and manually restoring damping didn't change anything. still had broken behavior.

@idiotWu
Copy link
Owner

idiotWu commented Jan 24, 2022

@tdaulbaev have you tried saving&restoring options.damping as I commented above?

@tdaulbaev
Copy link
Author

tdaulbaev commented Jan 24, 2022

Yes!
I guess I'll just turn off smooth scrolling on mobile devices, bacouse mobile browsers provide similar scroll behavior by default

@idiotWu
Copy link
Owner

idiotWu commented Jan 24, 2022

Yeah, I totally agree. I'm not too fond of using it on mobile devices either 😆

@tdaulbaev
Copy link
Author

Thanks!

@idiotWu
Copy link
Owner

idiotWu commented Jan 24, 2022

By the way, it seems OK in my demo: https://codesandbox.io/s/splide-smooth-scrollbar-oxmxh?file=/src/index.js

@tdaulbaev
Copy link
Author

there was an error in my implementation, now everything works!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants