tajo / react-movable

🔀 Drag and drop for your React lists and tables. Accessible. Tiny.
https://react-movable.pages.dev
MIT License
1.55k stars 51 forks source link

Improve documentation for cases when React.forwardRef is needed? #27

Closed dumbmatter closed 5 years ago

dumbmatter commented 5 years ago

I just finished porting a project from react-sortable-hoc to react-movable, and I gotta say, I'm really impressed. At least for my use case, I found react-movable to have a more intuitive API, and then the bundle size is smaller too! Great work.

The one thing that confused me for a bit was returning the correct value in renderItem.

Here's your example from the README - simple enough.

Here's a minor change to have renderItem return a custom React component, like:

      renderItem={({ value, props }) => (
        <Item value={value} {...props}>
          {value}
        </Item>
      )}

That fails, with an error message about the ref. Well, that's the error in CodeSandbox... in my app when I hit it, the error message was much less clear. So it took me a bit to figure out it can be fixed easily by using React.forwardRef.

Looking again at your README, I see "items need to be direct children of the DOM element that's being set with this ref", which I guess is what should have alerted me to this issue. But that's in the renderList section, maybe it would be better in the renderItem section too? And it might be good to be more explicit about what it means and how React.forwardRef might be needed, since a lot of people are not familiar with it. An example in the storybook would be nice too.

tajo commented 5 years ago

Yea, fair enough. There could be a note that one of the props passed through is a ref, so you might need to apply forwardRef if your are spreading it over your own component. Do you want to create a PR?

(btw the ref is passed only to the dragged item, all other items are accessed through the parent aka renderList ref... but that still means your <Item /> needs to forwardRef)

dumbmatter commented 5 years ago

Sure, I'd be happy to give it a shot :)

vtarelkin commented 2 years ago

@tajo Still was unclear to me why use ref and took me some time to add it.