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

Track drag source though unmount #64

Closed

Conversation

nelix
Copy link
Collaborator

@nelix nelix commented Feb 11, 2015

No description provided.

@gaearon
Copy link
Member

gaearon commented Feb 12, 2015

I think it's better to do this together with the resurfacing after #69.

@nelix
Copy link
Collaborator Author

nelix commented Feb 12, 2015

Mind if I leave the PR here to remind me?

@gaearon
Copy link
Member

gaearon commented Feb 12, 2015

Sure

@gaearon
Copy link
Member

gaearon commented Feb 26, 2015

TODO as I see it:

  • Merge master into 1.0 (basically means reapplying changes from Remove support for deprecated API #67 after all the refactoring, SORRY for that happening again lol). Ideally 1.0 should be overwritten as a single commit over current master, that commit being similar to 91fd2c5.
  • Rewrite this branch to be against 1.0 branch
  • Figure out (see below) if we want to pass “resurfaced” component to endDrag or not
  • Update Kanban example to use this branch

The upside of passing “resurfaced” component is that you can do something like component.props.onEndDrag without worrying that component may have resurfaced. For example, as Card is dropped into a different Kanban list, we might want to trigger some animation on that list.

The downside of doing that is sloppy behavior if there are two components with the same key on the screen at the same time. Which one would we pass? Does the most recently mounted one win? Why? Would we log a warning? Is there a legitimate use case for two components representing the same drag source? Should each of them receive endDrag? If one of them is “primary” and one of them “secondary”, which one would receive endDrag? Would both? What if there are zero components? (Source was removed and never added.) Should we pass null? Unmounted component?

Finally, what if resurfaced component has a different type? Think Card that has a different type WonderfulCard in some other List in Kanban. We wouldn't want one component's endDrag to handle the other component, would we? But WonderfulCard might want to do something when the dragging ends.

Should there be separate methods like joinDrag/leaveDrag for resurfaced drag sources instead of passing something else into endDrag? We'd still fire beginDrag/endDrag on the original drag source but allow other sources with same key to do something else. This API would also allow arbitrary number of drag sources to join and leave the drag midway.

@gaearon
Copy link
Member

gaearon commented Feb 26, 2015

So here's an alternative suggestion I just thought can be better than getKey stuff.

  1. When dragging ends, we call endDrag on source component with the original instance as component, no matter what
  2. There is no getKey
  3. However there are three new lifecycle methods: canJoinDrag(component, item), joinDrag(component, item), leaveDrag(component, item, effect)
  4. (We should probably make item a second parameter in endDrag for consistency—a breaking change that should probably go with Deprecate e parameter #93.)

Now let's define a compatible drag source as a component that has (or will) register a drag source with the same type as the source component.

When something is dragged, here's what happens:

  1. Source component's beginDrag is called
  2. For each compatible drag source currently mounted, call canJoinDrag(component, item) where component is that component (not the source component!)
  3. For each compatible drag source that returned true from canJoinDrag, call joinDrag(component, item) on them and consider it a mirror drag source

Then, until the dragging ends:

  1. Whenever a compatible drag source is mounted, call canJoinDrag on it. If it's true, consider it another mirror drag source
  2. Whenever a mirror drag source is unmounted, call leaveDrag(component, item) on it
  3. Mirror drag sources have their this.getDragState() “just like the source component” (with isDragging = true for current type)
  4. When dragging ends, source component receives endDrag
  5. All mirror drag sources receive leaveDrag(component, item, effect) with the result drop effect

How does this API changes things? Instead of

        dragSource: {
          getKey(component) {
            return DropTypes.COLUMN + component.props.id;
          },

you'd have

        dragSource: {
          canJoinDrag(component, item) {
            return item.id === component.props.id;
          },

Let's do that?

Note: I use mirror instead of resurfaced to clarify this may happen without source component unmounting.

@gaearon gaearon added this to the 0.12 milestone Mar 2, 2015
@gaearon gaearon mentioned this pull request Mar 4, 2015
@gaearon
Copy link
Member

gaearon commented Mar 4, 2015

There's a good stress test case from #108: sortable example without keys. isDragging should work correctly even if components don't have keys and “receive” new items in componentWillReceiveProps.

@gaearon
Copy link
Member

gaearon commented Mar 7, 2015

canJoinDrag-like API is now implemented in dnd-core, so work there supersedes this PR.
react-dnd/dnd-core@62e7cd2

@gaearon gaearon closed this Mar 7, 2015
@gaearon gaearon modified the milestones: v1.0, 0.12 Apr 20, 2015
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

Successfully merging this pull request may close these issues.

None yet

2 participants