search

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

Expose more events #26

Closed EternallLight closed 4 years ago

EternallLight commented 5 years ago

Hi @tajo,

Here the commit with changes from issue #20.

I called the new callback afterDrag as you suggested so that it matches to the existing beforeDrag. It is called right before onChange and contains the new index of the dragged element.

Both beforeDrag and afterDrag callbacks are now the ClientRect object. Since, we can't access the ghostRef when beforeDrag is called, I'm returning the ClientRect of the original DOM element.

Also, added onMove callback. I haven't implemented its triggered when list elements are moved with keyboard shortcuts as it contains the mouse coordinates and they are not quite relevant when moving the items with keyboard.

Regarding the tests, I haven't found any of them for beforeDrag in the existing codebase, so I haven't written tests for new callbacks. Let me know if you find them necessary.

Thanks, Andrey

tajo commented 5 years ago

Looks great!

Regarding the tests, I haven't found any of them for beforeDrag in the existing codebase, so I haven't written tests for new callbacks. Let me know if you find them necessary.

That would nice. There should be tests for all public APIs. For beforeDrag there is at least an example (dynamic table), but yea, if you could add some e2e tests for beforeDrag, afterDrag and onMove that would appreciated!

EternallLight commented 5 years ago

if you could add some e2e tests for beforeDrag, afterDrag and onMove that would appreciated!

Okay, will take a look at this next week. Thanks for the review

tajo commented 4 years ago

Closing for inactivity. Feel free to re-open.