cormacrelf / angular-skyhook

An implementation of react-dnd for Angular.
https://cormacrelf.github.io/angular-skyhook/
Other
164 stars 42 forks source link

Can skyhook support nested sortable lists #469

Open directcuteo opened 5 years ago

directcuteo commented 5 years ago

Hi! To be more precised about what I would like to accomplish here is an example made with dragula: https://codepen.io/anon/pen/RgBNje

I would want to have in my app a main list which will contain items. These items will be different angular components generated in a dynamic way based on the same js object (which is actually the object that will be parsed in the operations - add, remove, copy etc).

An important thing is that a component may have inside it more lists (2,3, or even 4 lists horizontally) of these dynamic components, like a button or an input field (a component with a list of components can't have another component with lists in it).

I would like to be able to drag items from any of the existing lists to any of the existing lists. (same rule applies: a component with a list in it, can't be dragged in a sub list of an another component, it can be just sorted within the big list where it belongs to). Also I will need to be able to drag new items or delete others.

I've read this thread which seems to match somehow mine but I would like an opinion for my particular case. https://github.com/cormacrelf/angular-skyhook/issues/327

I have this working with ng2-dragula but I want to implement it with the skyhook because, as the docs say, makes almost no limiting choices and I would like to be able to extend more afterwards if it will be the case.

Is it possible with skyhook? Which of the possible solutions is the one I should go for? (NgRx, SimpleKanban(Sortable), basic-sortable(core) )

Thank you!

cormacrelf commented 5 years ago

Yes, it’s possible, as you have correctly noted the kanban examples basically do this without the recursion, but there is no reason why you couldn’t add recursion.

The closest to the ng2-Dragula version is: construct a big tree. Structure your tree such that the leaves are different from regular nodes, like { kind: "leaf", objects: [...] }which should render with a different skyhook type (eg LEAF) to satisfy your non-mixing condition. For leaves, emulate SimpleKanban’s lowest level, but replace this.leaf.objects with a new list (made with immer) on successful drop. For nodes, emulate the mid level, with the same replacing of this.node.children. Have an ngIf that just picks which one to render for a given list element (either node or leaf), and recurse into that.

I can’t advise you on whether that’s good enough for your codebase. It might be slow as the tree gets very large. I can’t say how large this will be, it depends entirely on what your components do beyond being sortable. If you aren’t sure, just make a mutable version as above and if it’s slow you can write functions to immutably manipulate the tree from the top using multi level indexes, turn them into ngrx actions, and disable change detection. You can see why I’m not going to specifically recommend the additional work that involves. You will have to decide for yourself how much time to spend optimising.

directcuteo commented 5 years ago

Firstly, thank you for the explanation, I was able to understand the idea. One blocker that I still have on the SimpleKanban implementation is how can I make a copy operation? I will have an external non-editable and non-sortable list which will hold new items to be dropped. I see the example on NgRx with copy, great (still want to avoid it's complexity), but on SimpleKanban there is none. Should I use that function createData?(): Data ? How is that function affecting the indexes and where should I pay extra attention?

directcuteo commented 5 years ago

Indeed, it happens as you said: performance is affected, it becomes very laggy on drag (I guess because of the change detection). Also there is a problem with the type, it behaves like you said: it mixes if I let the same type. So I use then 2 types - NODE, LEAF - and on the main list I have type: ItemTypes.NODE, accepts: ItemTypes.LEAF but, as in the SortableSpec interface is writen, now I can drag a LEAF into the nodeList but I can't sort within the list (and this kinda blocks me). I saw that you added this function later (v1.2.0) on some request. I guess it is not implemented now to support the entire complexity I would want? Also, then what about accomplish it in NgRx where the NgRxSortableConfiguration interface doesn't have an accepts function at all? Thank you for your answers!

cormacrelf commented 5 years ago

accepts: [NODE, LEAF] — just use an array of types.

You are correct that NgrxSortableConfiguration can’t do this, I will push a fix asap.

cormacrelf commented 5 years ago

Released ~1.3.0~ 1.3.1.

I also noticed that you'll probably want to check the DropTargetMonitor for info about whether you are hovering directly or through a bunch of child lists, and whether a drop has been handled already by a child list. So I have added a monitor argument everywhere it makes sense on SortableSpec. Note that the NgRxSortable abstraction is breaking down a little bit here, but you can do (not tested):

import { DropTargetMonitor } from '@angular-skyhook/core';
import { DraggedItem } from '@angular-skyhook/sortable';
export class NestedNgRxSortable<D> extends NgRxSortable<D> {
    drop = (item: DraggedItem<D>, monitor: DropTargetMonitor<DraggedItem<D>>): void => {
        if (!monitor.didDrop()) {
            super.drop(item, monitor);
        }
    }
}

const _spec = new NestedNgRxSortable(/* store, actionType */, {
    type: "NESTED",
    trackBy: /* ... */,
    getList: /* ... */,
    canDrop: (_item, monitor) => {
        return monitor.isOver({ shallow: true });
    },
});
directcuteo commented 5 years ago

The problem I am facing after playing with canDrop is that if I make the example like you suggested the HOVER action on the main list is not triggered (therefore I can't see the placeholder while dragging through the list). I mention that I have now the accepts: [NODE, LEAF] and it accepts them both fine.

The same behaviour is if I don't use NestedNgRxSortable and on main list's spec I put canDrop: (_item, monitor) => {return monitor.isOver();} (it can have shallow param to any value, effect doesn't change) and on the child list's spec I use canDrop: (_item, monitor) => {return true;} .

The only way it seems to call the HOVER action on main list is if on main list's spec I put canDrop: {return true;} (not good cuz I will have double drop, obviously).

I'm expecting (like on my dragula implementation does) to have a placeholder according to the list I am hovering.

cormacrelf commented 5 years ago

As I recall hover is only called for SortableSpec if canDrop is true. Doesn’t all this suggest a better Nested implementation that checks shallow isOver instead of, or in addition to didDrop? That might solve “double drop”. You would then scrap the canDrop implementation as the default is to return true.