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

feat: extended SortalbeJS events with data attribute #139

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

Conversation

NicolaiHorlacher
Copy link
Contributor

Maybe this PR ist out of scope for vue-draggable-plus but I would like to see this or a similar feature in this library.

It's a hassle to implement custom logic on Sortable Events, because SortableJS only works on the DOM level.
This Library works on a data level and delegates the DOM manipulation to Vue.js. It would be nice to get more data access in the emitted events on the component and in the callbacks in the composable.

vue.draggable.next implements this using a custom change event with references the underlying data.

vue-draggable-plus already uses a workaround to connect data to SortableEvents. Data is saved to the HTMLElement which is dragged. This is currently only used to insert data from draggable list to another.

This PR uses this method to expose the data to every SortableJs event.
To make this work for all events the data needs to be attached in the onChoose event, because it's triggered before onStart.

During mergeOptions the passed options are extended with an data attribute that is read from the already extended HTMLElement.

With these changes it is much easier to implement side effects on SortableEvents.
For example showing a push notification when an element is added to a list would be possible with:

@add="(e) => showNotification(e.itemData.name + ' was added')"

I`m open to suggestions on how this feature could be implemented in an more elegant way. This is the best I could come up with.

Copy link

netlify bot commented May 20, 2024

Deploy Preview for vue-draggable-plus ready!

Name Link
🔨 Latest commit 2fb5e2f
🔍 Latest deploy log https://app.netlify.com/sites/vue-draggable-plus/deploys/664b98a91e83250008ac7c87
😎 Deploy Preview https://deploy-preview-139--vue-draggable-plus.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Alfred-Skyblue
Copy link
Owner

Thank you so much for your suggestion! I've been pondering whether to expose a getCurrentData function. I'm considering saving the currently dragged data externally during onStart, making it accessible within getCurrentData, and then destroying it in onEnd. Since we can only drag one item at a time, getCurrentData would always return the currently dragged item. Do you think this would be a better approach?

@Alfred-Skyblue
Copy link
Owner

Is the implementation of this feature in 68c549f meeting your requirements?

@NicolaiHorlacher
Copy link
Contributor Author

Sorry, for the late answer. I completely forgot this PR. Thanks for your feedback and alternative implementation.

Yes the implementation in your commit looks promising.
I was able to think about it a little more and I noticed another limitation of this approach:

It is not possible to limit the pull and put group behavior based on the data of the dragged element.
It would be possible to extend the pull and put function with an additional data parameter. But I think at this point the approach becomes messy and diverts to much from Sortable.js.

I like your idea and reasoning with the getCurrentData function (here: #153). getCurrentData can be used to access the data in pull and put also. So think this is the preferable solution.

I tried out getCurrentData and at it seems to work fine and meets all requirements that I tried to implement with this PR.
The only thing is missing is an export statement in front of the getCurrentData function.

@vprint
Copy link

vprint commented Dec 3, 2024

Any news on this PR ? How can i acces data using SortableEvent type ?

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