Ticket #4703 (closed bug: worksforme)

Opened 4 years ago

Last modified 3 months ago

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:
Blocking: Blocked by:

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

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

Change History

comment:1 Changed 4 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 4 years ago by kae

bug-fix

Changed 4 years ago by kae

much better patch - use this one instead

comment:2 Changed 4 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 4 years ago by paul

  • Status changed from new to closed
  • Resolution set to fixed

Fixed in r2999, thanks!

comment:4 Changed 4 years ago by mvug

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

comment:5 Changed 3 years ago by bhellman1

  • Status changed from closed to reopened
  • Resolution fixed deleted

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 3 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 3 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 3 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 2 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 2 years ago by eis-os (previous) (diff)

comment:10 Changed 8 months ago by scott.gonzalez

  • Milestone changed from TBD to 2.0.0

comment:11 Changed 8 months ago by mikesherov

  • Status changed from reopened to open

comment:12 Changed 8 months ago by mikesherov

  • Keywords haspatch added
  • Summary changed from nested sortables sometimes allow recursive drops to Sortable: 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 8 months ago by mikesherov

#4729 is a duplicate of this ticket.

comment:14 Changed 4 months ago by tj.vantoll

  • Owner set to kae
  • Status changed from open to pending

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 3 months 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 3 months ago by kae

  • Status changed from pending to new

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 3 months ago by mikesherov

  • Status changed from new to closed
  • Resolution set to worksforme
Note: See TracTickets for help on using tickets.