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

Add prevent click when scrolling plugin #326

Closed raulpesilva closed 2 years ago

raulpesilva commented 2 years ago
davidjerleke commented 2 years ago

Hi @raulpesilva,

Thank you for your pull request. I'm currently working on v7 of Embla so the response time is longer than I would like it it be. I'll get back to you when I've had the chance to check this out.

Best, David

davidjerleke commented 2 years ago

Hi @raulpesilva, I'm sorry for the delayed response. I've released v7 now so I finally got some time off to check this PR out.

First of all, thank you for your contribution. I also want to give you kudos for following the Embla code style. You've clearly aligned your plugin with the way the other plugins are written.

A friendly reminder: Please read the contribution guidelines before making a contribution. You've clearly followed the code style so that's not the reason why I'm bringing this up. It's because of this:

Discuss: Open an issue before starting any significant work.

And the reason for this is actually to spare contributors from possibly doing unnecessary work. This is why:

About the plugin

I have two concerns about this plugin and both are related to breaking accessibility of the carousel:

  1. Users are not at all allowed to click when the carousel is scrolling. So even if the user uses a mouse and clearly wants to click, the click won't be registered. Even the slightest scroll before settling will not allow the user to click on the carousel.
  2. Users can not stop a moving carousel if they for example see something interesting. This becomes obvious when dragFree: true.

I've included a screen recording that demonstrates problem 1 and 2:

https://user-images.githubusercontent.com/11529148/183241041-06af5703-4f4d-4ca0-a923-c6f5c342df1e.mp4

Have you tried embla.clickAllowed() instead? Because it's there for this very reason. It checks if the user dragged or statically clicked the carousel. Here's embla.clickAllowed() in action:

https://user-images.githubusercontent.com/11529148/183241117-9a14b344-adc6-4e4c-908e-076909baf2ed.mp4

Example code:

embla.slideNodes().forEach((slideNode, index) => {
  slideNode.addEventListener('click', () => {
    if (!embla.clickAllowed()) return

    console.log(`Click allowed: You clicked on slide with ${index + 1}`)
  })
})

Is this what you want to achieve?

Best, David