Droppable: Fixes: Ticket #8060. Over event is not fired correctly in a droppable. #583

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
2 participants
@nhatcher

jquiery.ui.droppable

this.over should change in a drop event even if the draggable is not accepted.

Droppable: this.over is changed even for non acceptable draggables. F…
…ixes Ticket #8060. Over was not fired correctly.
@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Nov 5, 2012

Member

Hi @nhatcher, thanks again for contributing this patch. We recently re-enabled the test suite for droppable. In order for us to land this patch, we'd need a few tests added to the test suite proving this doesn't break existing functionality and also that it fixes the bug as described. Can you add some tests here please?

Member

mikesherov commented Nov 5, 2012

Hi @nhatcher, thanks again for contributing this patch. We recently re-enabled the test suite for droppable. In order for us to land this patch, we'd need a few tests added to the test suite proving this doesn't break existing functionality and also that it fixes the bug as described. Can you add some tests here please?

@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Nov 17, 2012

Member

@nhatcher, have you had a chance to look into adding tests for this pull request?

Member

mikesherov commented Nov 17, 2012

@nhatcher, have you had a chance to look into adding tests for this pull request?

@mikesherov

This comment has been minimized.

Show comment
Hide comment
@mikesherov

mikesherov Nov 20, 2012

Member

Thanks again for contributing! Actually, this works as expected per the documentation. From http://api.jqueryui.com/droppable/#event-over: "Triggered when an accepted draggable is dragged over the droppable (based on the tolerance option)." For that reason, I'm going to close this.

Just because this didn't get merged, that doesn't mean you shouldn't try contributing another patch! There's tons of open bugs to work on over at: http://bugs.jqueryui.com/query?status=open&type=bug&group=component&col=id&col=summary&col=status&col=type&col=priority&col=milestone&col=component&order=priority&report=31

Thanks again!

Member

mikesherov commented Nov 20, 2012

Thanks again for contributing! Actually, this works as expected per the documentation. From http://api.jqueryui.com/droppable/#event-over: "Triggered when an accepted draggable is dragged over the droppable (based on the tolerance option)." For that reason, I'm going to close this.

Just because this didn't get merged, that doesn't mean you shouldn't try contributing another patch! There's tons of open bugs to work on over at: http://bugs.jqueryui.com/query?status=open&type=bug&group=component&col=id&col=summary&col=status&col=type&col=priority&col=milestone&col=component&order=priority&report=31

Thanks again!

@mikesherov mikesherov closed this Nov 20, 2012

@nhatcher

This comment has been minimized.

Show comment
Hide comment
@nhatcher

nhatcher Nov 20, 2012

Hi @mikesherov, thank you very much for taking your time in reviewing the pull request and the encouragement. Unfortunately, I will not have a chance until the end of the year to have a look at this issue again.

Hi @mikesherov, thank you very much for taking your time in reviewing the pull request and the encouragement. Unfortunately, I will not have a chance until the end of the year to have a look at this issue again.

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