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

monitor.isDragging() sometimes returns false when it should return true #227

Closed
amoenk opened this issue Jul 9, 2015 · 12 comments
Closed

Comments

@amoenk
Copy link

amoenk commented Jul 9, 2015

When using two or more interoperable sortable lists, monitor.isDragging() returns false sometimes when it should return true when dragging an item from one container to the other.

Setup: starting from the simple sortable demo, add another container and set of sortable objects. Cards should be draggable from one container to the other, sortable within their own container, and sortable as they are dragged from one container to the other.

The issue occurs when moving a card from one container to the other. The card component depends upon the isDragging property (injected by the DragSource decorator) to determine opacity, however, when the item is dragged from one container to the other, the global monitor changes the sourceId which no longer matches the dragged component and ends up returning false for calls to isDragging when it should return true.

Since the dragsource already requires a consistent globally unique key per draggable object (other stuff really misbehaves if your draggable component's keys change at any time), can the code use that key when designating the sourceId so that dragging from one container works correctly?

Let me know if you need to see the example code. My code demonstrating this issue is based off the simple sortable demo.

@vkbansal
Copy link
Contributor

@amoenk Yes, we will need example code to help you.

@amoenk
Copy link
Author

amoenk commented Jul 10, 2015

Thanks for your help! Here's the working example: http://notlouieagain.com/~amoenk/dnd/

And this link to browse the unbundled code: http://notlouieagain.com/~amoenk/dnd/js/

Oh, I included the original simple sortable demo code in there too while I was debugging, but the important components are Ingredient and IngredientGroup.

@vkbansal
Copy link
Contributor

I had a look at your code and I can't seem to find DnDStudy component. Can you show me your complete code and only yours. You can remove the demo code.

@amoenk
Copy link
Author

amoenk commented Jul 11, 2015

Demo code removed, DnDStudy component is a page component here: http://notlouieagain.com/~amoenk/dnd/js/pages/DnDStudy.react.js

@vkbansal
Copy link
Contributor

Ok I had a look and I believe that it may be a problem due to keys of IngredientGroup inside DnDStudy. Instead of relying on the index of the array, try using some unique id similar to that of Ingredient inside IngredientGroup.

@amoenk
Copy link
Author

amoenk commented Jul 11, 2015

Code updated, but still same problem.

@vkbansal
Copy link
Contributor

Ok. I was wrong then.

@gaearon can you help?

@gaearon
Copy link
Member

gaearon commented Jul 11, 2015

For isDragging to work across the component tree, please manually implement isDragging on your drag source.

const ingredientSource = {
    beginDrag(props) {
        return { ingredient: props.ingredient };
    },
    isDragging(props, monitor) {
        return props.ingredient === monitor.getItem().ingredient;
    }
};

See DragSource docs:

isDragging(props, monitor): Optional. By default, only the drag source that initiated the drag operation is considered to be dragging. You can override this behavior by defining a custom isDragging method. It might return something like props.id === monitor.getItem().id. Do this if the original component may be unmounted during the dragging and later “resurrected” with a different parent. For example, when moving a card across the lists in a Kanban board, you want it to retain the dragged appearance—even though technically, the component gets unmounted and a different one gets mounted every time you move it to another list. Note: You may not call monitor.isDragging() inside this method.

Does this help?

@amoenk
Copy link
Author

amoenk commented Jul 11, 2015

Works perfectly! Must have missed that in the docs.

Thanks for all your help! I'll close the ticket.

@amoenk amoenk closed this as completed Jul 11, 2015
@gaearon
Copy link
Member

gaearon commented Jul 12, 2015

No problem. Happy to help.

tmattio added a commit to tmattio/react-krdz that referenced this issue Dec 12, 2016
A card can now navigate through several lists as well as empty list.
A noticable thing is that I have to implement the isDragging function, it is not being injected properly if the component is outside of the current DOM tree.
See: react-dnd/react-dnd#227
@maasha
Copy link

maasha commented Jan 13, 2018

meh! Links to http://notlouieagain.com/ are not responding ...

@shahzore-qureshi
Copy link

I found the root cause of my issue. I had to stop using the DragDropContextProvider to wrap my DropTargets and DragSources. See the example below.
http://react-dnd.github.io/react-dnd/docs-drag-drop-context-provider.html

export default class YourApp {
    render() {
        return (
            <DragDropContextProvider backend={HTML5Backend}>
                /* ... */
            </DragDropContextProvider>
        )
    }
}

Instead, I had to use the DragDropContext to wrap my entire class like below.
http://react-dnd.github.io/react-dnd/docs-drag-drop-context.html

class YourApp {
    /* ... */
}
export default DragDropContext(HTML5Backend)(YourApp)

This immediately fixed the problem for me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants