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

Remove prop type from Draggable #188

Closed
alexreardon opened this issue Nov 21, 2017 · 4 comments
Closed

Remove prop type from Draggable #188

alexreardon opened this issue Nov 21, 2017 · 4 comments

Comments

@alexreardon
Copy link
Collaborator

Given that a Draggable must have the same type as it's parent Droppable - there is no point it requiring a Draggable to declare its type. We would need the prop if we allowed floating draggables #29. I suppose the answer to this question is whether we want to support floating draggables

@alexreardon
Copy link
Collaborator Author

Related: #25

Right now I am thinking of removing type from Draggable and using the type from the parent Droppable. This would remove the need for #25 and would also make #29 redundant.

If you really did need a 'floating' Draggable you could create a small Droppable that has isDropDisabled set to true to avoid any dropping on it. Given we are optimising for lists I think this is the best course of action.

Thoughts @jaredcrowe ?

@alexreardon
Copy link
Collaborator Author

I had a crack at this - but I fell down in the selector function. We use the type in the selector to avoid needing to do any work on draggables that are not impacted by a drag. Selectors cannot read from context. More information: reduxjs/react-redux#289

Given this limitation I think the simplest path is to stick with the type prop for now. However, we can do #25 to make the developer experience nicer.

I am thinking of a way around this - but it feels gross. The idea would be we wrap what is the Draggable today which reads the type from the context and then passes that on as a prop. This would not impact the consumption story at all but introduces a level of indirection that could get confusing

@alexreardon
Copy link
Collaborator Author

The type prop may no longer be needed as apart of #86. Hopefully as a part of that work we can drop the type from Draggable altogether 👍

@alexreardon
Copy link
Collaborator Author

I can confirm that the type prop is being removed from Draggables as a part of #86

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

No branches or pull requests

1 participant