davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
6.02k stars 180 forks source link

Add `watchDrag` option #416

Closed peacepostman closed 1 year ago

peacepostman commented 1 year ago

Bug is related to

Embla Carousel version

Describe the bug

When a slide has a scrollable content, I am unable to scroll via touch handles. It would be great to have a way to allow this on custom elements/targets.

Bonus question ^^ my slides content are inside a shadow DOM, if there is a possibility to counter my first question, would it be applicable on this context.

CodeSandbox

https://codesandbox.io/s/embla-carousel-y-axis-react-forked-0032js?file=/src/js/EmblaCarousel.tsx

Steps to reproduce

  1. Go to sandbox
  2. First slide is classic overflow auto
  3. Second slide scrollable content is inside a shadow DOM
  4. Scroll works perfectly via scroll but not via touch handler

Expected behavior

Best regards,

peacepostman commented 1 year ago

I've had a look on how to achieve such a thing and I think the most versatile way of doing this would be to add a new options in order to allow a touch target check function, something like this:

TS

skipTarget: (node:Node, target: Event) => boolean

Example

const [sliderRef, sliderAPI] = useEmblaCarousel({
    skipTarget: (node, event) => {
      const path = event.composed ? event.composedPath() : event.path
      const found = path.find((nod: Element) => {
        return nod.nodeName && !['#document', 'HTML', '#document-fragment'].includes(nod.nodeName) && nod.getAttribute('class') && nod.getAttribute('class').includes('scrollable-area')
      })
      return !!found
    },
  })

ATM it only impact the isFocusNode function, which is a pretty minimal impact. I'm now struggling on how to prevent dragging on embla NODE and allow drag on the desired element.

I would be happy to submit a PR if this feature present an interest to you and the community.

Best regards,

davidjerleke commented 1 year ago

Hi @peacepostman,

Thank you for your bug report or feature request? I can't decide what to call it 🙂. However, I think the following needs to be considered at this stage:

Adding a scrollable element inside a slide can cause problems, for example, if devs make it too big. In these cases, there won't be anywhere to grab the carousel itself because the scrollable element will swallow touch events.

How would one solve that? Checking if the scrollable element has reached its end? This will significantly increase complexity. I'm open to other suggestions if you have something else in mind.

Best, David

peacepostman commented 1 year ago

Hello @davidjerleke ,

Thanks for your answer,

I agree that it is more a feature request than a bug but it started has a surprise and lead to a potential need ^^

I don't know if this is some something doable but another possibility would be to extend the draggable option, in order to conditionally pass a function like this: draggable: boolean || (node:Node, target: Event) => boolean

By doing so, the user will still be free to continu to provide a boolean value or make a possible complex checking in order to disable dragging and drag/scroll a nested child.

In my point of view the library doesn't have to check all the potential complexity you mentioned previously, but if this is something needed, exposing a way of doing it can be interesting.

Best regards, Ben

domin-mnd commented 1 year ago

would like to see this feature asap. It's just what embla is missing right now

domin-mnd commented 1 year ago

Just thought of the temporary solution which is not perfect ofc. The issue in that solution is that a 3d translation (scroll effect) disappears if you touch the element while carousel is scrolled + you can't scroll carousel down/up if you touch element & reached its end of the scroll area.

Example: https://domin.pro/ (white section cards)

// Query select all the scrollable elements we want to apply our draggable state to
document.querySelectorAll('An element to scroll')?.forEach((element: Element) => {
  // When you hold that element the carousel isn't scrollable
  element.addEventListener('touchstart', () => modScroll(false));
  element.addEventListener('touchend', () => modScroll(true));
  // Also you could add the same exact event for mouse
});

/**
 * Enable or disable dragging on the carousel based on the state
 * @param {boolean} state Whether to enable or disable dragging
 * @returns {void}
 */
function modScroll(state: boolean): void {
  emblaApi?.reInit({
    draggable: state,
  });
}
peacepostman commented 1 year ago

Hello @davidjerleke , thanks for your answer, i'm not able to see it here anymore but nevertheless the provided way to do this is not applicable in my case.

Anyway, big thanks for the time you study upon it. If one day you have time to implement something similar to the proposal discussed above, maybe i will be able to solve my issue.

In the meantime I was forced to portal out the content inside my slide in order to be able to scroll it.

Have a nice day, Ben,

davidjerleke commented 1 year ago

Hi @peacepostman,

Sorry for the confusion. I created a sandbox but I realized that it wasn't finished and working as expected. That's why I deleted my comment.

I'm the sole dev on this project and with the huge backlog I have, the response time on issues isn't optimal right now. I can't tell when I have time to investigate this further. Thanks for understanding!

Best, David

davidjerleke commented 1 year ago

To be released with v8.0.0-rc01.

davidjerleke commented 1 year ago

Specification

watchDrag

This new option adds the possibility to cancel the default drag behaviour. For example, it's useful if you need to disable dragging when a user wants to scroll an element inside a slide. Default is:

{
  watchDrag: true,
}

You can opt out of the default Embla drag behaviour like so:

{
  watchDrag: false,
}

However, you can also pass a callback function to watchDrag like so:

{
  watchDrag: (emblaApi: EmblaCarouselType, event: MouseEvent | TouchEvent) => {
    // do your own thing here BEFORE Embla runs its internal drag logic

    return true // <-- Return true if you want Embla to continue with its default drag behaviour

    // ...OR:

    return false // <-- Return false if you want Embla to skip its default drag behaviour
  },
}
davidjerleke commented 1 year ago

@peacepostman and @domin-mnd this feature has been released with v8.0.0-rc01.

domin-mnd commented 1 year ago

thanks a lot @davidjerleke

hanshank commented 1 year ago

@davidjerleke - This is pretty awesome! I was hoping this option could solve my problem, but looks like it doesn't. I'm trying to disable dragging when clicking or holding next/previous buttons that fire emblaApi.scrollNext() and emblaApi.scrollPrev() functions respectively. The buttons are layered over the slides which is probably the issue. I tried doing this:

  const [emblaRef, emblaApi] = useEmblaCarousel(
    {
      watchDrag: () => {
        return isDragEnabledRef.current;
      },
    },

Then I set that ref (in React btw) to false on the button's onMouseDown event and back to true on onMouseUp event. Unfortunately it seems like the drag event is started before the onMouseDown event has time to fire, so it doesn't really work.

Is there a way to define a scrollable area? Or maybe disable scrolling if the mouse event was initiated inside a certain element?

davidjerleke commented 1 year ago

Hi @hanshank,

I'm trying to disable dragging when clicking or holding next/previous buttons that fire emblaApi.scrollNext() and emblaApi.scrollPrev() functions respectively.

I think you might be misunderstanding the use cases of watchDrag. There is a better way to solve this without using the watchDrag option. Please read this guide carefully in order to understand why this is happening and how to solve it.

Or maybe disable scrolling if the mouse event was initiated inside a certain element?

The second argument to watchDrag is the mousedown or touchstart event as specified here. You can use that event argument to check where the mousedown or touchstart happened using event.target, and prevent scrolling if needed.

Best, David

hanshank commented 1 year ago

Thanks @davidjerleke, this is exactly what I’ve been looking for! A little more digging around the docs and I would have found it. Thanks for pointing me in the right direction. Really appreciate it.