search

atlassian / react-beautiful-dnd

Beautiful and accessible drag and drop for lists with React
https://react-beautiful-dnd.netlify.app
Other
33.32k stars 2.55k forks source link

Dragging doesn't end, intermittently #1662

Open pimterry opened 4 years ago

pimterry commented 4 years ago

Expected behavior

When dropping an element, onDragEnd is called, and it stops dragging

Actual behavior

Intermittently (maybe every 50 drops or so, no clear patterns), onDragStart is called, but onDragEnd is not. I'm dragging & dropping the item with a mouse, using a drag handle.

After being dropped, the item retains its fixed styling, and stays where it was. This means it stays outside normal DOM flow & the drop itself is never really preserved by my app though, which results in all sorts of odd behaviour.

No messages in the console, no other obvious clues. I have only one context and one droppable in my app.

Steps to reproduce

Not sure! I know that's not helpful. I'm working on this, and I'll update once I come up with a clearer cause or repro case.

Suggested solution?

Assuming this isn't my mistake, this seems like a clear bug imo, so that's easy enough.

In the meantime, while I try to work out how to reproduce this more clearly, any ideas on what might cause this, or things I should investigate?

It's quite possible this is my bug, but I can't see anything obvious that would cause it, and I've checked through all the common setup issues already.

What version of React are you using?

16.8.5

What version of react-beautiful-dnd are you running?

12.2.0 (pretty sure I've seen this in 12.1.1 too)

What browser are you using?

Chrome 78

Demo

Not a minimal repro by any definition, but if you download HTTP Toolkit, go to the Mock page in the app, and then drag the rules back and forth for a little bit, you'll hit it soon enough.

danieldelcore commented 4 years ago

Hey @pimterry,

Have you been able observe this in our storybook instance https://react-beautiful-dnd.netlify.com/ ? If not, could you please set up a minimal reproduction in codesandbox for us to have a look at?

Here's a boilerplate: https://codesandbox.io/s/k260nyxq9v

Cheers :)

pimterry commented 4 years ago

Have you been able observe this in our storybook instance https://react-beautiful-dnd.netlify.com/ ?

No, I haven't.

Could you please set up a minimal reproduction in codesandbox for us to have a look at?

I've been trying to do this, but struggling so far. It is reproducible in my application, but it's a large & complex codebase and an intermittent issue, so it's been challenging trying to extract my DND usage to a minimal reproduction with the same behaviour.

@danieldelcore Do you have any ideas for the kind of thing that could cause this? Ever seen any other issues like this? I'll keep working on it, but any pointers you could give me would really help with putting together the repro.

danieldelcore commented 4 years ago

Yeah things like this can sometimes be caused when react re-renders are triggered by components higher up the tree.

You can debug this by enabling paint flashing in the Chrome dev tools. Give that a try and see if you can find anything 😄

pimterry commented 4 years ago

Ok, I think I've managed to reproduce this. It's down to some of the unorthodox style overriding I'm doing, but I'm still a little surprised that it's an issue, so it seems like a bug or at least docs problem to me.

Repro: https://codesandbox.io/s/vertical-list-u0nhn?fontsize=14&hidenavigation=1&theme=dark

To reproduce, drag an item into a new spot, and drop it at the exactly correct vertical position (to the pixel - it takes some practice), but with your mouse slightly off horizontally. The item will stop and stay there, but keep its green dragging styling, because the drag never ends. It does reset if you start dragging something else.

The difference with the boilerplate is all in getItemStyle:

const getItemStyle = (isDragging, style) => {
  const overrideStyles = {};

  const currentTransformMatch = style && style.transform &&
      style.transform.match(/translate\((-?\d+)px,\s+(-?\d+)px\)/);
  if (currentTransformMatch) {
      console.log('overriding transform:', style && style.transform);
      const yTransform = parseInt(currentTransformMatch[2]);
      overrideStyles.transform = `translate(0, ${yTransform}px)`;
  }

  return {
    // some basic styles to make the items look a bit nicer
    userSelect: "none",
    padding: grid * 2,
    margin: `0 0 ${grid}px 0`,

    // change background colour if dragging
    background: isDragging ? "lightgreen" : "grey",

    // styles we need to apply on draggables
    ...style,
    ...overrideStyles
  };
};

Here I'm locking to move just on the Y axis. I've seen this discussed & this approach suggested in a few other issues (https://github.com/atlassian/react-beautiful-dnd/issues/538#issuecomment-486876308, https://github.com/atlassian/react-beautiful-dnd/issues/958), although I do understand it's not officially supported. Still, it's clearly a popular use case, and it would be nice to avoid it completely breaking like this.

I'll keep digging, but I suspect this is due to the animation finishing instantly, because it's already in the right place, similar to the issue that the skipping animation docs mention.

This doesn't seem to happen without this change though in the boilerplate if the item is already in exactly the right position, which is surprising. Any idea why? Is there some code somewhere that handles that case?

alexreardon commented 4 years ago

I think I know what is happening

alexreardon commented 4 years ago

We are in work around territory now.

https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/guides/types.md#draggable

type DropAnimation = {|
  duration: number,
  curve: string,
  moveTo: Position,
  opacity: ?number,
  scale: ?number,
|};

You could manually fire a transition end event move rbd along. Would you like some input on how to do that?

xReaven commented 4 years ago

Hello there.

Using your really cool D&D, and I do think I'm facing a similar issue, or at least related.

Actually, I am handling a button, which appears on each draggable item, only when hovering them. Almost everything works fine.

I'm hovering the item, dragging the button that appeared on hover, and so, I'm now dragging the Item. The onDragStart callback fired, so I hide every buttons thanks to a state (it become useless to display them while already dragging an other item). It is all ok at moment. Nonetheless, if I drop it at the exact same place (same index) where it was on drag start, the onDragEnd callback is not fired, and so I can't update the state that handle the dragging.

It makes impossible the update of my state, knowing if anything is being dragged. Therefore, it stays "dragging" and now, no more buttons will appears.

sidharthancr commented 4 years ago

We are in work around territory now.

https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/guides/types.md#draggable

type DropAnimation = {|
  duration: number,
  curve: string,
  moveTo: Position,
  opacity: ?number,
  scale: ?number,
|};

You could manually fire a transition end event move rbd along. Would you like some input on how to do that?

Yes please!!

safaaleigh commented 4 years ago

@sidharthancr @alexreardon I'm in a similar situation with a project I'm working on and would be interested in how to manually fire a transition end event. Could you share that work around in this thread?

I know that there's an onTransitionEnd on the provided.draggableProps object provided by <Draggables/>'s render function, but I'm not sure if that's the function @alexreardon was alluding to, or how to call it properly.

longhaha1998 commented 4 years ago

We are in work around territory now.

https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/guides/types.md#draggable

type DropAnimation = {|
  duration: number,
  curve: string,
  moveTo: Position,
  opacity: ?number,
  scale: ?number,
|};

You could manually fire a transition end event move rbd along. Would you like some input on how to do that?

Yes please！ How can i do this please!

longhaha1998 commented 4 years ago

I have the same problem now！How can I manually fire a transition end event move rbd along？

longhaha1998 commented 4 years ago

Ok, I think I've managed to reproduce this. It's down to some of the unorthodox style overriding I'm doing, but I'm still a little surprised that it's an issue, so it seems like a bug or at least docs problem to me.

Repro: https://codesandbox.io/s/vertical-list-u0nhn?fontsize=14&hidenavigation=1&theme=dark

To reproduce, drag an item into a new spot, and drop it at the exactly correct vertical position (to the pixel - it takes some practice), but with your mouse slightly off horizontally. The item will stop and stay there, but keep its green dragging styling, because the drag never ends. It does reset if you start dragging something else.

The difference with the boilerplate is all in getItemStyle:

const getItemStyle = (isDragging, style) => {
  const overrideStyles = {};

  const currentTransformMatch = style && style.transform &&
      style.transform.match(/translate\((-?\d+)px,\s+(-?\d+)px\)/);
  if (currentTransformMatch) {
      console.log('overriding transform:', style && style.transform);
      const yTransform = parseInt(currentTransformMatch[2]);
      overrideStyles.transform = `translate(0, ${yTransform}px)`;
  }

  return {
    // some basic styles to make the items look a bit nicer
    userSelect: "none",
    padding: grid * 2,
    margin: `0 0 ${grid}px 0`,

    // change background colour if dragging
    background: isDragging ? "lightgreen" : "grey",

    // styles we need to apply on draggables
    ...style,
    ...overrideStyles
  };
};

Here I'm locking to move just on the Y axis. I've seen this discussed & this approach suggested in a few other issues (#538 (comment), #958), although I do understand it's not officially supported. Still, it's clearly a popular use case, and it would be nice to avoid it completely breaking like this.

I'll keep digging, but I suspect this is due to the animation finishing instantly, because it's already in the right place, similar to the issue that the skipping animation docs mention.

This doesn't seem to happen without this change though in the boilerplate if the item is already in exactly the right position, which is surprising. Any idea why? Is there some code somewhere that handles that case?

Hey guys！I found a solution, and it seems to work! @pimterry @alexreardon 

    onTouchEnd={() => {
        this.onTouchEnd(
           provided.draggableProps.onTransitionEnd
        );
    }}

    onTouchEnd(onTransitionEnd) {
        const { toggleIsDragging } = this.props;
        if (onTransitionEnd) {
            setTimeout(() => {
                onTransitionEnd({
                    propertyName: 'transform',
                });
            }, 330);
        } else {
            toggleIsDragging(false);
        }
    }

It looks ugly, but it solves my problem.

feychenie commented 4 years ago

Huge thanks @longhaha1998, solved also my issue!

I noticed that the onTransitionEnd prop is a function only on drag release and is null otherwise, so we can just test the prop type and do the call as soon as it is a function

  <Draggable /* ... usual draggable props here */>
    {(draggableProvided, snapshot) => {
      if (
        typeof (
          draggableProvided.draggableProps.onTransitionEnd
        ) === 'function'
      ) {
        window?.requestAnimationFrame(() =>
          draggableProvided.draggableProps.onTransitionEnd({
            propertyName: 'transform',
          })
        );
      }
      return (
        <Card ref={draggableProvided.innerRef} {...draggableProvided.draggableProps}>
         // ... etc
n8sabes commented 4 years ago

@feychenie thanks, that mostly works for me. However, I am getting console errors on drop: Invariant failed: Cannot finish a drop animating when no drop is occurring. Are you seeing this and if so, any ideas how to fix it?

While it works much better, the whole can get stuck after few rapid aborted drags that stay within the item's original slot. After trying other items a few times, the list unsticks itself.

SSOURABH58 commented 3 years ago

snapshot

Thanks, it really solves the issue for me, but the only problem now is I am receiving an error massage in the console

issue rbd
jayankmayukh commented 3 years ago

@SSOURABH58 @n8sabes I was able to avoid that error(Invariant failed) by wrapping the call to onTransitionEnd in queueMicrotask instead of requestAnimationFrame.

if (typeof provided.draggableProps.onTransitionEnd === 'function') {
    queueMicrotask(() =>
        provided.draggableProps.onTransitionEnd?.({
            propertyName: 'transform',
        })
    );
}
barbalex commented 7 months ago

I am having the same issue. Unfortunately onTouchEnd makes no sense to me: It does not seem to be available on DragDropContext. I am trying to solve the issue by reacting to onDragUpate, using a debounced callback that runs on the tail end of the funtion repeatedly calling.

Edit: I got it now. The problem was that I had not added provided.draggableProps.style to the Draggable element.