Skip to content

Conversation

voithos
Copy link
Contributor

@voithos voithos commented Feb 24, 2013

...es directly. Fixed #9116 - droppable: drop event can cause droppables to remain active if any droppable is created/destroyed in the event handler

@scottgonzalez
Copy link
Member

Thanks. Can't this be done by just copying the original array using .slice() instead of using the UUIDs?

@voithos
Copy link
Contributor Author

voithos commented Feb 25, 2013

Ah, yes, it looks like it can. I just tested using .slice(), and it works perfectly.

I guess I was somewhat confused as to how exactly the droppable was being removed when there was still a reference to it in $.ui.ddmanager.droppables (which is why my first instinct was to compare the UUIDs). Looking further into it, I see that $(...).remove() ends up calling jQuery UI's $.cleanData which removes the droppable from the ddmanager. That clears things up for me.

Should I make a different branch for this fix, using the .slice() method?

@scottgonzalez
Copy link
Member

You can create a new branch or you can just --amend your commit with the new implementation.

…the droppables directly. Fixed #9116 - droppable: drop event can cause droppables to remain active if any droppable is created/destroyed in the event handler
@voithos
Copy link
Contributor Author

voithos commented Feb 26, 2013

Commit has been amended.

@mikesherov
Copy link
Member

Can you add an explanatory comment to the commit as well. A maintainer won't easily understand why you're doing slice there. Thanks again!

@scottgonzalez
Copy link
Member

I'll add the comment when I clean this up to meet our coding standards. I'm going to land this now.

@scottgonzalez
Copy link
Member

Landed in 1c80735. Thanks.

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

Successfully merging this pull request may close these issues.

3 participants