Skip to content

feat: onDragMove prop #324

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

Merged
merged 2 commits into from
Mar 28, 2025
Merged

Conversation

lorenzogonnelli
Copy link
Contributor

@lorenzogonnelli lorenzogonnelli commented Mar 27, 2025

This pull request introduces the onDragMove callback to the sortable components, allowing for more granular control and feedback during drag operations. The key changes include adding the onDragMove callback to various components, updating type definitions, and documenting the new callback.

Callback Integration:

Type Definitions:

Component Updates:

Copy link

vercel bot commented Mar 27, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
react-native-sortables-docs ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 28, 2025 10:20am

@lorenzogonnelli
Copy link
Contributor Author

Hi @MatiPl01, sorry i wouldn't want to make you crazy hehe, i need this prop because i have a trash element with position absolute on the bottom of my page and i would like for the item to be deleted as soon as the user snap it on top of it. Currently there are no way to detect the coordinates of the dragged element. WIth this prop, you will be able to.

@MatiPl01
Copy link
Owner

Hey @lorenzogonnelli

I was wondering what is the use case of this change so thanks for explaining that. I think that this is an useful change and I will review this PR later on today.

After a quick look on the code, I have a small concern related to the callback implementation. You use this helper hook that I also used for other callbacks, like onDragStart but it uses runOnJS to call the callback from the JS thread (it makes things easier for the library users as they don't have to pass a reanimated worklet function). It is good as long as it is not called too often.

Since your callback is called on every little movement of the item, I would prefer not to use runOnJS and require passing a worklet function instead. I can give you a code snippet with a suggestion how to change this it you wish when I will be reviewing your PR later on today.

Thanks again for the contribution!

@lorenzogonnelli
Copy link
Contributor Author

Hey @lorenzogonnelli

I was wondering what is the use case of this change so thanks for explaining that. I think that this is an useful change and I will review this PR later on today.

After a quick look on the code, I have a small concern related to the callback implementation. You use this helper hook that I also used for other callbacks, like onDragStart but it uses runOnJS to call the callback from the JS thread (it makes things easier for the library users as they don't have to pass a reanimated worklet function). It is good as long as it is not called too often.

Since your callback is called on every little movement of the item, I would prefer not to use runOnJS and require passing a worklet function instead. I can give you a code snippet with a suggestion how to change this it you wish when I will be reviewing your PR later on today.

Thanks again for the contribution!

Sure! Thanks for pointing this out. I understand the concern, and that makes sense. A snippet with a suggested change would be really helpful—I’ll wait for your review and update the PR accordingly.

Copy link
Owner

@MatiPl01 MatiPl01 left a comment

Choose a reason for hiding this comment

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

Very good quality and clean PR. Thanks a lot!

I left just 2 comments suggesting to add docs changes in a separate PR.

I know that I asked you in a comment left before to change the onDragMove callback to not to use runOnJS (used by useJSStableCallback callback wrapper) but I decided that I will refactor callbacks usage later on to determine in the library itself if the callback provided by the user is a worklet function or not, so you don't have to make any changes now. I will change callbacks to automatically detect if the function is a worklet by using the reanimated's isWorkletFunction function.

@lorenzogonnelli
Copy link
Contributor Author

Very good quality and clean PR. Thanks a lot!

I left just 2 comments suggesting to add docs changes in a separate PR.

I know that I asked you in a comment left before to change the onDragMove callback to not to use runOnJS (used by useJSStableCallback callback wrapper) but I decided that I will refactor callbacks usage later on to determine in the library itself if the callback provided by the user is a worklet function or not, so you don't have to make any changes now. I will change callbacks to automatically detect if the function is a worklet by using the reanimated's isWorkletFunction function.

Thanks! You've spared me some time ahah, anyways I've proceeded to do everything you asked, the docs change are in a separate PR i just opened :)
Thank you

@MatiPl01 MatiPl01 merged commit 98a9e9d into MatiPl01:main Mar 28, 2025
6 checks passed
@MatiPl01
Copy link
Owner

@lorenzogonnelli
FYI: I implemented smarter callbacks handling in the #326 PR. Now, when you add a 'worklet' string at the beginning of the function passed as a callback, it will run on the UI thread without needing to jump from the UI to the JS thread as it was doing before, hence it should be more performant.

MatiPl01 pushed a commit that referenced this pull request Mar 31, 2025
# [1.5.0](v1.4.0...v1.5.0) (2025-03-31)

### Bug Fixes

* Active portal not working on Android ([#330](#330)) ([0edd622](0edd622))
* Data change example animated height conflicting with layout transition ([#333](#333)) ([d8f95bd](d8f95bd))
* Drag end callback invalid data array ([#332](#332)) ([e5fe29b](e5fe29b))
* fade drop indicator on snap ([#323](#323)) ([3178583](3178583))

### Features

* Add a possibility to enable or disable the portal provider ([#331](#331)) ([5526e29](5526e29))
* Delayed absolute layout initialization before sorting is enabled for the first time ([#329](#329)) ([57668f7](57668f7))
* onDragMove prop ([#324](#324)) ([98a9e9d](98a9e9d)), closes [/#diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR14](https://github.com///issues/diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR14) [/#diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR14](https://github.com///issues/diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR14) [/#diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR42-R48](https://github.com///issues/diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR42-R48) [/#diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR62](https://github.com///issues/diff-909e9ac37b95e3b1e1de1bbac752161e284101aa59ff16a835ec2ff60370c24dR62) [/#diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R13](https://github.com///issues/diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R13) [/#diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R13](https://github.com///issues/diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R13) [/#diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R42-R48](https://github.com///issues/diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R42-R48) [/#diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R70](https://github.com///issues/diff-e33ed4a677abd501d9340a3814a5001ab07d7b208f855caa1cd3760af9a4fc88R70) [/#diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R8](https://github.com///issues/diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R8) [/#diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R8](https://github.com///issues/diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R8) [/#diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R136-R145](https://github.com///issues/diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R136-R145) [/#diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R180-R185](https://github.com///issues/diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R180-R185) [/#diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R204-R208](https://github.com///issues/diff-544f2eb90180c2dd21d666753ae5105128f3b522247fc9868e93bf516c72f1b8R204-R208) [/#diff-09a936c481316c018f0c31756ee59f904d5906ac2013ed97131ca1926b7699ccR64](https://github.com///issues/diff-09a936c481316c018f0c31756ee59f904d5906ac2013ed97131ca1926b7699ccR64) [/#diff-b91ba790038255caebcc4ae026433dab840df2ed341d09bb3f58b75eff1416faR60](https://github.com///issues/diff-b91ba790038255caebcc4ae026433dab840df2ed341d09bb3f58b75eff1416faR60) [/#diff-b91ba790038255caebcc4ae026433dab840df2ed341d09bb3f58b75eff1416faR60](https://github.com///issues/diff-b91ba790038255caebcc4ae026433dab840df2ed341d09bb3f58b75eff1416faR60) [/#diff-b91ba790038255caebcc4ae026433dab840df2ed341d09bb3f58b75eff1416faR104](https://github.com///issues/diff-b91ba790038255caebcc4ae026433dab840df2ed341d09bb3f58b75eff1416faR104) [/#diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fL48-R48](https://github.com///issues/diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fL48-R48) [/#diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fL48-R48](https://github.com///issues/diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fL48-R48) [/#diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fR105](https://github.com///issues/diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fR105) [/#diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fR389-R396](https://github.com///issues/diff-c764e053814b2d438bc15e419a151ba1aac64acbac7bcdeb8646047a6f8b845fR389-R396)
* Smart drag callbacks handling on UI or JS thread ([#326](#326)) ([f53f5d7](f53f5d7))
@MatiPl01
Copy link
Owner

🎉 This issue has been resolved in version 1.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

2 participants