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

Allow group options to have a function for pull #526

Closed
wants to merge 1 commit into from

Conversation

rockhymas
Copy link

Fixes #504

@David-Desmaisons I saw your comments in the related issues. I believe this deals with them properly, but if not, I'm happy to keep working on this until it's correct. Just point me in the right direction.

@rockhymas
Copy link
Author

Ping

@David-Desmaisons
Copy link
Member

Thanks for the PR.
I am still not convinced of the interest of using a function for pull since the option prop is reactive. Moreover passing DOM element to the function does not sound to me like a gooe pattern.
That said, the cloning mode had to be cached on the instance as you are doing.
I should provide this feature in the short term.

@JivanRoquet
Copy link

JivanRoquet commented Mar 16, 2019

Hi there,

Just my two cents concerning the interesting of using a function for pull.

@David-Desmaisons is there any other way you would think of, when trying to make the pull behaviour reactive to the ctrl/cmd buttons, so that:

  • dragging with Ctrl/Cmd pressed => duplicate the item (leave it in place in original position, and create a new one)
  • dragging without Ctrl/Cmd pressed => move the item as usual

I've been trying to do that with pull as a function, but no luck so far. Any other way to get to the same behaviour would be massively welcome.

If using pull as a function is the only way to get to that, then I think that this would be indeed a very useful thing to do, given that being able to alter the move/copy behaviour with Ctrl/Cmd keys while dragging is quite a standard and very widespread thing.

@David-Desmaisons
Copy link
Member

@JivanRoquet I understand and I agree with your arguments.
Pull as function will be provided.
I made a PR to Sortable.js to get all the information to handle it correctly in vue.draggable.

@rockhymas
Copy link
Author

Just wanted to chime in that I've been following the work and am glad it's going forward, thanks @David-Desmaisons!

@David-Desmaisons
Copy link
Member

Fixed by PR #584

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