Opened 8 years ago

Closed 5 years ago

#4703 closed bug (worksforme)

Sortable: nested sortables sometimes allow recursive drops

Reported by: kae Owned by: kae
Priority: minor Milestone: 2.0.0
Component: ui.sortable Version: 1.7.2
Keywords: haspatch Cc:
Blocked by: Blocking:

Description

http://verens.com/demos/php_and_jquery/8-2-sorting-trees/sorting-trees.php

it is sometimes possible to drag an element into one of its own contained lists.

to replicate this in my link above, just drag the Home item slowly downwards. periodically, you will see the drop marker (the red line) flash inside the Home item itself. if you drop at that point, then the element tries to move into itself and vanishes from view, leaving a browser error.

Attachments (2)

ui.sortable.patch (435 bytes) - added by kae 8 years ago.
bug-fix
ui.sortable.js.diff (4.7 KB) - added by kae 8 years ago.
much better patch - use this one instead

Download all attachments as: .zip

Change History (19)

comment:1 Changed 8 years ago by kae

can be solved by adding this to position 644 in ui.sortable.js if($.ui.contains(this.currentItem[0],this.containers[i].element[0]))continue;

Changed 8 years ago by kae

Attachment: ui.sortable.patch added

bug-fix

Changed 8 years ago by kae

Attachment: ui.sortable.js.diff added

much better patch - use this one instead

comment:2 Changed 8 years ago by kae

example of the fixed script in action http://verens.com/demos/nested-sortables/nested-sortables.html

compare with demo in main post

comment:3 Changed 8 years ago by paul

Resolution: fixed
Status: newclosed

Fixed in r2999, thanks!

comment:4 Changed 8 years ago by mvug

doesn't seem to work quite well in IE8...

comment:5 Changed 7 years ago by bhellman1

Resolution: fixed
Status: closedreopened

Hello, I'm interested in trying this patch to allow for nested sortables but the links above are dead. Where can the patch be found / demo?

thxs

comment:6 Changed 7 years ago by kae

try this instead: http://demo.verens.com/nested-sortables/nested-sortables.html

action is a bit jerky. I remember solving that once, but reverted for some reason that escapes me.

probably easier to use a different plugin for this (jstree for example) until someone with better insight than me figures it out.

comment:7 Changed 7 years ago by bhellman1

@Kae, very nice it seems like you got it closer. Maybe CSS is the reason for the jumpyness? I'd love to use JSTREE but sadly it doesn't support moving handlers, which prevents me from putting userinteractable items in the list item like checkboxes comments etc...

Any interest in moving your solution to the next level? Tons of people are looking for this type of solution that allows nested sortables, that can searalize and is built on top of JQUERY UI.

comment:8 Changed 7 years ago by kae

I don't have the time at the moment (busy with a 9-5 job, and a book that I'm nearly 2 weeks behind on), but may come back to it in a few weeks.

hmmm... having said that, here's a more stable one: http://demo.verens.com/nested-sortables.2/nested-sortables.html

just added back in an old line at L1037:

&& itemElement.parentNode == this.placeholder[0].parentNode only rearrange items within the same container

I can't spare more time on this at the moment.

comment:9 Changed 7 years ago by eis-os

Would it be possible to enable the following (mentioned) code again?

&& itemElement.parentNode == this.placeholder[0].parentNode"

This prevents flickering and jumping problems moving nested sortable items into a children that got sortable too. Or at least an option to enable this?

Last edited 7 years ago by eis-os (previous) (diff)

comment:10 Changed 5 years ago by Scott González

Milestone: TBD2.0.0

comment:11 Changed 5 years ago by mikesherov

Status: reopenedopen

comment:12 Changed 5 years ago by mikesherov

Keywords: haspatch added
Summary: nested sortables sometimes allow recursive dropsSortable: nested sortables sometimes allow recursive drops

Please note that the fixes presented here don't work in IE8 and they'd need to for a patch to be accepted.

comment:13 Changed 5 years ago by mikesherov

#4729 is a duplicate of this ticket.

comment:14 Changed 5 years ago by tj.vantoll

Owner: set to kae
Status: openpending

I ported the only functioning test case listed here over to jsFiddle and I'm not seeing any issues against master - http://jsfiddle.net/tj_vantoll/FRy2k/. I tested in Firefox, Chrome, and IE8 since it was explicitly listed.

Can anybody else recreate this?

comment:15 Changed 5 years ago by mikesherov

I'm fairly certain this is now fixed, years ago. Leave it as pending in case OP miracously has a new test case.

comment:16 Changed 5 years ago by kae

Status: pendingnew

the jsFiddle example appears to be working fine, so consider the problem fixed. I don't have rights to modify the ticket to mark it as done.

comment:17 Changed 5 years ago by mikesherov

Resolution: worksforme
Status: newclosed
Note: See TracTickets for help on using tickets.