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

Add watchDrag option #416

Closed
Tracked by #321
peacepostman opened this issue Jan 9, 2023 · 14 comments · Fixed by #461
Closed
Tracked by #321

Add watchDrag option #416

peacepostman opened this issue Jan 9, 2023 · 14 comments · Fixed by #461
Labels
feature request New feature or request resolved This issue is resolved

Comments

@peacepostman
Copy link

Bug is related to

  • embla-carousel (core package)

Embla Carousel version

  • v7.0.5

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

  • Have the possibility to exclude some targets

Best regards,

@peacepostman peacepostman added the bug Something isn't working label Jan 9, 2023
@peacepostman
Copy link
Author

peacepostman commented Jan 10, 2023

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
Copy link
Owner

davidjerleke commented Jan 17, 2023

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
Copy link
Author

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

@davidjerleke davidjerleke added feature request New feature or request and removed bug Something isn't working labels Jan 23, 2023
@domin-mnd
Copy link

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

@domin-mnd
Copy link

domin-mnd commented Jan 30, 2023

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
Copy link
Author

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
Copy link
Owner

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 davidjerleke added the investigating Issue is being looked into label Apr 20, 2023
davidjerleke added a commit that referenced this issue Apr 24, 2023
@davidjerleke davidjerleke linked a pull request Apr 24, 2023 that will close this issue
@davidjerleke davidjerleke added resolved This issue is resolved and removed investigating Issue is being looked into labels Apr 24, 2023
@davidjerleke
Copy link
Owner

To be released with v8.0.0-rc01.

@davidjerleke davidjerleke mentioned this issue Apr 24, 2023
37 tasks
@davidjerleke davidjerleke changed the title Scroll ability inside a slide Add watchDrag option Apr 24, 2023
@davidjerleke
Copy link
Owner

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
Copy link
Owner

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

@domin-mnd
Copy link

thanks a lot @davidjerleke

@hanshank
Copy link

hanshank commented May 19, 2023

@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
Copy link
Owner

davidjerleke commented May 20, 2023

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request New feature or request resolved This issue is resolved
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants