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

Convert to Hooks #8

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Convert to Hooks #8

wants to merge 1 commit into from

Conversation

max-sym
Copy link

@max-sym max-sym commented May 7, 2020

Changes:

  • Converted the logic to hooks.

Packages:

  • Added eslint + prettier (for code formatting);
  • Updated react and react-dom;
  • Removed stylus (was throwing an error in the current version) and redux;

The packages were changed because of personal preferences so it can be discussed, for sure :)

@max-sym max-sym mentioned this pull request May 7, 2020
@tamagokun
Copy link
Owner

this is great! I'm glad you simplified the dependencies as it makes the example easier to understand. I think at the time I had just copied over some boilerplate App code that I was using.

I'm going to pull this down and give the code a look over. I think it's definitely worth merging this in and doing away with the old stuff :)

@max-sym
Copy link
Author

max-sym commented May 7, 2020

Nice!
I think that it's not 100% clean yet, but that's also just an example.

},
})

const [{}, dragRef] = useDrag({
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pull out a preview ref here so we can set the previewing component: const [{}, dragRef, previewRef]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting, so there exists the ability to show custom preview components? Nice!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, maybe we can remove the {} here, because eslint is complaining on those lines.
So it will become const [, dragRef, previewRef] = ...

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes, that would be great!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The official react-dnd docs didn't seem to have any info about previewRef thing in the useDrag...
But yeah, that's awesome that we can specify that!

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🤷 yeah, i'm not sure why the docs are lacking this. I found a codesandbox example using it.


return (
<div ref={dropRef}>
<div
Copy link
Owner

@tamagokun tamagokun May 11, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then use that previewRef here:

<div ref={dropRef}>
  <div ref={previewRef}>
    <div ref={dragRef}>..</div>
    <Tree ... />
  </div>
</div>

This makes it so the Tree attached to the node you are dragging is a part of the drag preview

@tamagokun tamagokun mentioned this pull request May 11, 2020
@wiktoriavh
Copy link

This seems to be older, but I do wanna say that this worked great when I was trying to build the nested list for react-dnd. This is my code:
--> https://codesandbox.io/s/react-dnd-nested-list-example-fehzw

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.

3 participants