Opened 7 years ago

Closed 7 years ago

#9036 closed bug (notabug)

Sortable: $.contains() give the wrong arguments.

Reported by: nyanhan Owned by: nyanhan
Priority: minor Milestone: none
Component: ui.sortable Version: 1.10.0
Keywords: haspatch Cc:
Blocked by: Blocking:

Description

fix a bug. $.contains() give the wrong arguments, on link 798 this bug case, connect parent error, and can't sort well in one container.

Change History (6)

comment:1 Changed 7 years ago by tj.vantoll

Owner: set to nyanhan
Status: newpending

Hi nyanhan, thanks for taking the time to contribute to the jQuery UI project. The arguments do appear to be backwards on that line (https://github.com/jquery/jquery-ui/blob/master/ui/jquery.ui.sortable.js#L797).

But I'm unable to recreate the problem you're describing as nested connected sortables do appear to be ignored correctly - http://jsfiddle.net/tj_vantoll/gXNxq/.

Can you alter my test case to show the problem you're experiencing?

Thanks.

comment:2 in reply to:  1 Changed 7 years ago by nyanhan

Status: pendingnew

Replying to tj.vantoll:

Hi nyanhan, thanks for taking the time to contribute to the jQuery UI project. The arguments do appear to be backwards on that line (https://github.com/jquery/jquery-ui/blob/master/ui/jquery.ui.sortable.js#L797).

But I'm unable to recreate the problem you're describing as nested connected sortables do appear to be ignored correctly - http://jsfiddle.net/tj_vantoll/gXNxq/.

Can you alter my test case to show the problem you're experiencing?

Thanks.


It will appear in this case. http://jsfiddle.net/nmnJC/ Thank you for your help.

comment:3 Changed 7 years ago by tj.vantoll

Status: newopen
Summary: $.contains() give the wrong arguments.Sortable: $.contains() give the wrong arguments.

Thanks I see the issue now. If connectWith is removed in your test case the problem goes away.

comment:4 Changed 7 years ago by tj.vantoll

Keywords: haspatch added

Pending pull request https://github.com/jquery/jquery-ui/pull/904. This also might be addressed by the fix for #8792 (https://github.com/jquery/jquery-ui/pull/916).

comment:5 Changed 7 years ago by mikesherov

http://jsfiddle.net/nmnJC/1/ show that the connectWith issue is fixed by https://github.com/jquery/jquery-ui/pull/916

Please submit a test case showing how $.contains argument order causes a problem. Thanks!

comment:6 Changed 7 years ago by mikesherov

Resolution: notabug
Status: openclosed

If you read the comment on https://github.com/jquery/jquery-ui/blob/83cbf979788f22ba3bd1668507623c0dd6b57041/ui/jquery.ui.sortable.js#L796, you'll notice that the contains statement is there to prevent a nonsensical edge case: the item contains its container.

For now, I'm going to close this as not a bug. If you have a test case that shows otherwise, I'll reopen. Thanks again!

Note: See TracTickets for help on using tickets.