Opened 9 years ago
Closed 9 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 follow-up: 2 Changed 9 years ago by
| Owner: | set to nyanhan |
|---|---|
| Status: | new → pending |
comment:2 Changed 9 years ago by
| Status: | pending → new |
|---|
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 9 years ago by
| Status: | new → open |
|---|---|
| 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 9 years ago by
| 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 9 years ago by
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 9 years ago by
| Resolution: | → notabug |
|---|---|
| Status: | open → closed |
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!
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.