camohub / jquery-sortable-lists

jQuery plugin to sorting lists also the tree structures.
MIT License
209 stars 60 forks source link

Items added after initializing list with maxLevels cannot be dragged #68

Open lucaslm opened 3 years ago

lucaslm commented 3 years ago

Greetings again @camohub!

I've noticed an issue that appeared in this new version 2.2.0. Whenever we have a sortable list initialized with some value for maxLevels, any new item that is later appended to the list cannot be dragged into a new position. I've set an example on my forking so you can see for yourself:

https://lucaslm.github.io/jquery-sortable-lists/example3

The reason for this error is because you changed how to compute the level an item would have when dragged to a new position. Specifically, upon initializing the list you add some data to every item, namely 'inside-levels' and 'upper-levels'.

https://github.com/camohub/jquery-sortable-lists/blob/aba9cb1c95b9ae4e88c844c93d25e40f96e4c75c/jquery-sortable-lists-mobile.js#L181-L192

Later on the function checkMaxLevels, you get 'upper-levels' from the new parent and 'inside-levels' from the item being dragged. You then add those to see if it exceeds maxLevels. Since items created later do not have 'inside-levels', the sum evaluates to NaN and the expression to false, even when it shouldn't.

https://github.com/camohub/jquery-sortable-lists/blob/aba9cb1c95b9ae4e88c844c93d25e40f96e4c75c/jquery-sortable-lists-mobile.js#L989-L995

This can be avoided if you do not insert new items, but it wasn't an issue when insideLevs was computed at startDrag for the item being dragged, and upperLevs at its dropping, though it could be less performatic.

I suppose there are two approaches for this. We can create a new option to control how maxLevels is computed, to let the user choose whether to compute the levels on the fly ineficiently (but allowing later item insertions) or to store the levels on initialization. The other approach is to use a MutationObserver to store an item's levels each time it is included on the list's DOM after initialization.

Let me know which approach you think is best, and I can create a pull request for this issue.