FormidableLabs / react-swipeable

React swipe event handler hook
https://commerce.nearform.com/open-source/react-swipeable
MIT License
2k stars 147 forks source link

onSwiped not triggered in some cases #230

Closed Robloche closed 2 years ago

Robloche commented 3 years ago

I use react-swipeable in 2 different places in my app. In the first one, it perfectly works but in the second one, onSwiped is not triggered.

I used the event listener breakpoint of Chrome inspector and when I release the button, it correctly breaks. But the internal onUp function of react-swipeable is never called.

Several things worthy to note:

Here's how I use the hook:

const Swipeable = ({children, ...props}: SwipeablePropType) => {
  const handlers = useSwipeable(props);

  return <div className='swipeable' {...handlers}>{children}</div>;
};

Using onSwiping I set the transform CSS rule to make a div follow the mouse, then using onSwiped I actually perform an action on the div's content.

I put console.logs into both handlers and here's what I get when it works:

swiping
swiping
...
swiping
swiped

And when it doesn't:

swiping
swiping
...
swiping

The swiped log actually prints when I close the modal (i.e. unmount the component).

I tested this with the latest versions of Chrome and Firefox.

Robloche commented 3 years ago

I tried to reproduce this issue in a small CodePen but didn't succeed. I was hoping someone else had run into a similar bug...

hartzis commented 3 years ago

@Robloche I genuinely appreciate you attempting to create an example. I've only updated the labels to help organize and manage these issues at a higher level.

We really can't start to fix this until we can reproduce but we can keep this issue open as a light 🔦 for others that may run into something similar. And then maybe someone else will have a case that can be reproduced that we can move forward with.

Thank again for taking the time to create the issue and describe in detail what you're seeing, i'm hopeful this will help others and we can get the issue identified.

Robloche commented 3 years ago

Update: The issue is still present in my app.

I'm sorry I cannot still reproduce this bug in a small example but I can now share my app. Please go to https://viva.videofutur.fr/, then:

  1. Check that you can drag the movie covers.
  2. Click on a movie. This opens a detailed view of the movie.
  3. Scroll down a bit and try dragging the covers.
  4. Notice how the dragging never ends.

@hartzis It would be super nice from you if you could just have a look at the bug and see if it rings some bell. Thanks!

laclance commented 3 years ago

Also not working for me and cannot reproduce or share app.

hartzis commented 2 years ago

Update: The issue is still present in my app.

I'm sorry I cannot still reproduce this bug in a small example but I can now share my app. Please go to https://viva.videofutur.fr/, then:

  1. Check that you can drag the movie covers.
  2. Click on a movie. This opens a detailed view of the movie.
  3. Scroll down a bit and try dragging the covers.
  4. Notice how the dragging never ends.

@hartzis It would be super nice from you if you could just have a look at the bug and see if it rings some bell. Thanks!

I am able to replicate this on my desktop in chrome. 😳 that is weird...

I have no idea, lol. I'd need a reproducible example that i can edit/manipulate the code to investigate further.

Could you maybe share the code of just the Swipeable usage for that element and how you're handling the translate for the carousel?

Robloche commented 2 years ago

Thanks for having taken the time to check my issue. I'm glad you experienced it either. I really tried to build a CodePen but even with an intricate DOM structure, I couldn't reproduce the bug...

Regarding my code, I had to write the following React component since my codebase is a little bit old and does not use React hooks:

import * as React from 'react';
import {useSwipeable} from 'react-swipeable';

type SwipeablePropType = {
  +children: React.Element<*>
};

export const Swipeable: React.ComponentType<SwipeablePropType> = ({children, ...props}: SwipeablePropType) => {
  const handlers = useSwipeable(props);

  /* eslint-disable react/jsx-props-no-spreading */
  return <div
    className='swipeable'
    {...handlers}>{children}</div>;
  /* eslint-enable react/jsx-props-no-spreading */
};

And here's how I use it:

        <Swipeable
          onSwiped={this.handleOnSwiped}
          onSwiping={this.handleOnSwiping}
          trackMouse>
          <div
            className='sectionSlider'
            onTransitionEnd={this.handleOnTransitionEnd}
            ref={(instance) => {
              this.slider = instance;
            }}>
            {tiles}
          </div>
        </Swipeable>
handleOnSwiping = (eventData) => {
    const {deltaX} = eventData;
    const {displayType, maxPageIndex, pageIndex} = this.state;
    const {slider, sliderPositions} = this;

    if (!slider || maxPageIndex <= 0 || displayType === SECTION_DISPLAY_GRID) {
      return;
    }

    // Trick explained below
    this.setState({isSwiping: true});

    const posX = sliderPositions[pageIndex];
    const newPosX = posX + deltaX;

    slider.style.transform = `translateX(${newPosX}px) translateZ(0)`;
  };
  handleOnSwiped = (eventData) => {
    const {dir, velocity} = eventData;
    const {displayType, maxPageIndex} = this.state;

    // Trick explained below
    this.resetSwipeTimeout();
    this.swipeTimeout = setTimeout(() => this.setState({isSwiping: false}), SWIPE_COOLDOWN_TIME);

    if (maxPageIndex <= 0 || displayType === SECTION_DISPLAY_GRID) {
      return;
    }

    const pageStep = Math.ceil(velocity / PAGE_VELOCITY_STEP);

    if (dir === 'Left') {
      this.goToNextPage(pageStep);
    } else if (dir === 'Right') {
      this.goToPreviousPage(pageStep);
    }
  };

Without the isSwiping trick, releasing the button that triggers onSwiped event also triggers the opening of a tile's detailed view (depending on where the mouse cursor ended). So I wait 10 ms (SWIPE_COOLDOWN_TIME) before allowing a click on a tile.

hartzis commented 2 years ago

howdy again,

Was able to take a look today at your code above.

Couple things i noticed:

Additionally i took another look at the page and interaction and noticed that the swiping stops if you release the mouse when you're not in the modal, so this tells me there may be something specific within the modal that is causing this weirdness.

i hope this helps you figure this out. Closing for now but please re-open and let me know if you're able to get an example working where i can test things out.

Robloche commented 2 years ago

Hello,

First of all: thanks for taking a deeper look at my issue.

I tried event.preventDefault() but it did not work (detail view still opens when mouse button is released), plus it did not solved my main issue.

However, I had totally missed the fact that the swiping stops if the button is release outside the modal. That gives me a solid track to explore.

I'll re-open this issue if something new occurs.

Thanks again,