embla-carousel icon indicating copy to clipboard operation
embla-carousel copied to clipboard

Add prevent click when scrolling plugin

Open raulpesilva opened this issue 3 years ago • 2 comments

  • ✨ This plugin is to prevent clicking when scrolling, putting 'pointer-event' to 'none' and back to 'auto' when finishing scroll

raulpesilva avatar Jun 26 '22 00:06 raulpesilva

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 avatar Jul 17 '22 20:07 davidjerleke

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:

  • There might already be ways to achieve what you want so you might end up doing unnecessary work.
  • Maybe there's an upcoming major version with breaking changes that will change how parts of the core work and you might end up doing unnecessary work.

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

davidjerleke avatar Aug 06 '22 08:08 davidjerleke