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

Pass event down to onChange #20

Merged
merged 1 commit into from
Jul 18, 2016
Merged

Pass event down to onChange #20

merged 1 commit into from
Jul 18, 2016

Conversation

megawac
Copy link
Contributor

@megawac megawac commented Jul 18, 2016

Main reason I want to do this is to render a set of custom html elements rather than simple strings. Passing evt up lets me avoid a bunch of string parsing and instead just process the html element (just found out about data-id :)

Useful for desynchronized groups where you can add items to one group but cannot add items to the other; adding to the other removes from the second list

/cc @cheton

@cheton cheton merged commit 432308e into SortableJS:master Jul 18, 2016
@cheton
Copy link
Collaborator

cheton commented Jul 18, 2016

Thank you for your pull request. It looks good to me. I'll publish a release soon.

However, there are too many parameters in the onChange method, I'd try to refactor this part in the next major release.

@megawac
Copy link
Contributor Author

megawac commented Jul 18, 2016

Sounds good, I'm not sure if passing the sortable instance is really
necessary?

On Mon, Jul 18, 2016 at 11:25 AM, Cheton Wu notifications@github.com
wrote:

Thank you for your pull request. It looks good to me. I'll publish a
release soon.

However, there are too many parameters in the onChange method, I'd try to
refactor this part in the next major release.


You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#20 (comment),
or mute the thread
https://github.com/notifications/unsubscribe-auth/ADUIENZ6xgJ_Hi9n4O38nj2Wg5Pv0A6tks5qW5rvgaJpZM4JOweM
.

@cheton
Copy link
Collaborator

cheton commented Jul 18, 2016

It's not necessary when using ref to store the sortable instance. I will plan to remove it as well.

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.

2 participants