[Proposal] Ability to set max depth

[johnny]

[Proposal] Ability to set max depth

[danielboendergaard]

I'm using IsValidTarget now and it works great, but I just thought that it might be something that could be simplified by adding max depth as an option.

In my application I have a max depth of 2. I have sublists on all my list items, because I want to be able to take a sublist item and put it in the root list and then put items into it, even if it had no subitems before.

There is the option to not include the sublist as you point out but I want the user to be able to put a list item in the sublist, just not a list item that has its own subitems.

Essentially if i specify a max depth of 2, I don't want the user to be able to do this:

While writing this i thought that it would make more sense to just rewrite my isValidTarget function to be generic, maybe someting like this can be added to the plugin or maybe just as an example.

isValidTarget: function ($item, container) {
    var depth = 1, // Start with a depth of one (the element itself)
        maxDepth = 2,
        children = $item.find('ol').first().find('li');

    // Add the amount of parents to the depth
    depth += container.el.parents('ol').length;

    // Increment the depth for each time a child
    while (children.length) {
        depth++;
        children = children.find('ol').first().find('li');
    }

    return depth <= maxDepth;
}

[johnny]

Thanks for pointing this out. Personally, I have not used it and the plugin is designed in such a way, that I do not think this is necessary:

1. Sublists are not generated automatically
2. There is the option isValidTarget where you can easily check, if the nesting is valid.

But I do think that this might be used often enough to warrant hacking up an example and adding it to the docs.

What do you think?

[johnny]

Your example looks good. I will soon add a new example using this.

The reason for not adding this to the plugin itself is, that a fully general maxDepth function would need to load all subcontainers of the dragged item.

Thanks for your feedback.

[danielboendergaard]

No problem :)

[johnny]

I will leave it open as a reminder.

[marcandre]

We need this exact same feature (also with maxDepth = 2).
I coded it up, but this would be a nice feature to have.

The reason for not adding this to the plugin itself is, that a fully general maxDepth function would need to load all subcontainers of the dragged item.

Actually, I think this is a great reason why it should be included. This is not totally trivial to do. Moreover, the current API makes it not quite obvious to do this, as the item depth should be calculated once at onDragStart, stored and accessed for all later calls to isValidTarget, even though one could be tempted to calculate it in isValidTarget. Also, the lib doesn't currently store the container's depth, so we must loop, but this would probably be trivial to add at the lib level.

Note that the code @danielboendergaard gave is misleading as it won't quite work properly for maxDepth > 2.

tl;dnr: I'm 👍 to add this as a feature. I'd be glad to provide a PR if need be.

[marcandre]

PS: This would also make the lib more robust. Currently, it's possible to drag a nested item from a group with nested: true to a group with nested: false. To me, nested: false implies maxDepth: 0 and the move should not be allowed.

[Arthedian]

I have big problem with @danielboendergaard code, because its not working well. Problem is when I have

  <ol>
   <li>First
        <ol>
              <li>Second</li>
              <li>Third</li>
          </ol>
   </li>
  <li>Fourth</li>
 <ol>

and when i want to switch "Second" a "third" it doesnt do anything. Can i do anything with that?

[danielboendergaard]

I fully support adding support for this in the core, which is why I made this post in the first place :)

@marcandre I'm curious why you say that my solution doesn't work for maxdepth > 2 since I'm using it in a project with maxdepth = 3 without issues.

However, I still don't think that my solution is optimal since it uses a lot of DOM traversing and I would rather see an implemetation in the core instead.

@Arthedian I don't really have a good answer for you since it is working for me, try to make a jsbin and share it here.

[Arthedian]

here is the link http://jsbin.com/ruvuqogo/1/edit i dont know why but its not working well on jsbin.

[danielboendergaard]

@Arthedian I just looked at your code and it works on jsbin if you correct the js errors it points out.

I think you should try to use the css from the docs and modify it to your needs, I find that the lists needs to have certain styling for the plugin to work properly.

[Arthedian]

css from docs, and still not working: http://jsbin.com/ruvuqogo/6/edit

[marcandre]

@danielboendergaard the code doesn't always count the depth of $item correctly, in particular the following:

ol
  li ol
  li ol
    li

Here's the code to show that: http://jsfiddle.net/4ULSV/

[johnny]

@marcandre
If you could provide a PR for this feature, I will gladly merge it.

You are right, that this will make the lib more robust. You can replace the nested option with a maxDepth option.

[moschel26]

@johnny
What you are making jquery sortable?
I need your help
http://stackoverflow.com/questions/34837423/failed-get-json-in-jquery-sortable
I've been trying and ask in the forums, but unresolved

Any suggestion to solve my problem?

Thank you

[michaelsoriano]

+1 for maxdepth in nesting