feat(@clayui/core): implement move functionality for tree #4255

julien · 2021-09-07T14:10:54Z

@matuzalemsteles this is "yet another follow up" on top of
#4254. Since I don't have permissions to push to your
fork (and that you don't have permissions to push to mine),
I thought pushing directly to upstream would be easier,
that way we can both iterate on the same branch.

julien · 2021-09-08T10:59:13Z

@matuzalemsteles I made some small changes (mostly) clean up in e2e85cc

julien · 2021-09-08T11:08:56Z

@matuzalemsteles I left everything as is so that's it's easier to review what's been done, but I'll probably need to squash and clean those previous commits: I added a dependency by mistake in @clayui/core

julien · 2021-09-08T12:13:39Z

Just one more thought @matuzalemsteles, we've named this function createImmutableTree, but we're actually mutating it (even if we do that just in that function, and even though I tried returning a "new copy") what would you think about renaming that?

matuzalemsteles · 2021-09-08T16:18:06Z

Since I don't have permissions to push to your fork (and that you don't have permissions to push to mine), I thought pushing directly to upstream would be easier, that way we can both iterate on the same branch.

Oh true, perfect I had forgotten about that.

I added a dependency by mistake in @clayui/core

Yeah I saw it, I think I just need to update yarn.lock to remove the dependencies.

Just one more thought @matuzalemsteles, we've named this function createImmutableTree, but we're actually mutating it (even if we do that just in that function, and even though I tried returning a "new copy") what would you think about renaming that?

Yes, I think if we can't come up with the idea of returning a copy of the structure even in a way that doesn't affect the original structure we can rename it to something different since it doesn't deal with Immutable anymore. Anyway, I'll see if I can look at this again today.

matuzalemsteles · 2021-09-08T16:48:57Z

packages/clay-core/src/tree-view/useItem.tsx

+			canDrop: monitor.canDrop(),
+			overTarget: monitor.isOver({shallow: true}),
+		}),


I added this to get the properties to add the hover classes when one element is over another.

Yes good idea. I avoided adding those until they were needed, but I also knew that at one point we would need them.

matuzalemsteles · 2021-09-08T16:50:56Z

packages/clay-core/src/tree-view/useItem.tsx

+		key: keyRef.current,
+	};
+
+	const [, drag] = useDrag({


We still have to modify the preview to look like the Lexicon look.

Yes good point, I'll have a look into it.

Hey @matuzalemsteles about the preview (sorry I'm not able to paste the image from the Google Document here), what's your idea? I was thinking I'd create an image representing the "node being dragged" from a canvas element and using canvas.toBlob or similar and pass it to the preview component. Does that make sense to you?

I think we can just create an HTML markup 😅, for example we can render in DnD preview an empty image and use useDragLayer to get the position and render the markup.

We added this to useItem because of the useDrag which returns the preview.

useEffect(() => { preview(getEmptyImage(), {captureDraggingState: true}); }, [preview]);

And we created a DragLayer component that would be the preview, something like this:

const layerStyles = { cursor: 'grabbing', height: '100%', left: 0, pointerEvents: 'none', position: 'fixed', top: 0, width: '100%', zIndex: 150, }; function getItemStyles(initialOffset, currentOffset) { if (!initialOffset || !currentOffset) { return { display: 'none', }; } const {x, y} = currentOffset; const transform = `translate(${x}px, ${y}px)`; return { WebkitTransform: transform, transform, }; } const DragLayer = () => { const {currentOffset, initialOffset, isDragging, item} = useDragLayer( (monitor) => ({ currentOffset: monitor.getSourceClientOffset(), initialOffset: monitor.getInitialSourceClientOffset(), isDragging: monitor.isDragging(), item: monitor.getItem(), }) ); if (!isDragging) { return null; } return ( <div style={layerStyles}> <div className="some-class-to-style" style={getItemStyles(initialOffset, currentOffset, isDragging)} > {item.name} </div> </div> ); };

We can call this component in TreeView. This is an example I made in DXP a while ago with some modifications.

Thanks @matuzalemsteles I'll test that.

Added a "custom" drag layer based on this in 2c1b354 we'll just need to add that some-class-to-style to make it look a bit nicer.

matuzalemsteles · 2021-09-08T16:54:33Z

packages/clay-core/src/tree-view/useItem.tsx

+				return;
+			}
+
+			reorder((dragItem as Value).indexes, item.indexes);


Although practically this implementation is ready now, we still have to play with the indexes a bit and maybe change something in the reorder, because for DnD from what I see in the specification we will have to deal with adding the element above or below the element with hover or within it. Actually, I don't know we should do this or just add inside, it's not clear in the documentation 😅

Yes, that's a good idea. I personally think that if we're able to determine at which position the item should be added it would be ideal, but the basic idea was that you could reorder nodes in the tree, I'll also check how far we can improve this.

Hey @matuzalemsteles, I've added eacefd2 which makes it possible to "reorder" child nodes in the same "root/parent" node. Might need some cleaning up but I still wanted you to take a quick look

Oh perfect, I'm going to take a look at this commit.

matuzalemsteles · 2021-09-08T17:01:38Z

packages/clay-core/src/tree-view/useTree.ts

+	function nodeByPath(path: Array<number>): any {
+		const queue = [...path];
+		const rootIndex: number = queue.shift() as number;
+		let item = tree[rootIndex];
+		let parent = tree;
+		let index = rootIndex;
+
+		while (queue.length) {
+			index = queue.shift() as number;
+
+			if (item.children) {
+				parent = item.children;
+				item = item.children[index];
+			}
+		}
+
+		return {
+			index,
+			item,
+			parent,
+		};
+	}


Cool, great work

Thanks, I thought that seeing a while loop would make you sad 😄

haha in this case the number of interactions is smaller, so it's ok 😅. It's hard to run away from it.

matuzalemsteles · 2021-09-08T17:18:20Z

packages/clay-core/src/tree-view/useTree.ts

+					const nodeToRemove = nodeByPath(from);
+					const parentNode = nodeToRemove.parent.filter(
+						(item: any, index: number) =>
+							index !== nodeToRemove.index
+					);
+
+					tree[from[0]] = {
+						...tree[from[0]],
+						children: parentNode,
+					};
+
+					const {item} = nodeByPath(path);
+
+					tree[path[0]] = {
+						...item,
+						children: [...item.children, nodeToRemove.item],
+					};


It was very simple, good work here too. I will try to further explore the idea of making a copy of the structure so as not to manipulate the original structure. I'll explore some ideas like not needing to create a copy of each element but only the path that is being modified, kind of a partial copy. I'll try something like this.

Yes, very simple and that's what I liked about it. I'll also have a thought about what you're just said (partial copy) sounds interesting.

Yeah, I was researching more about this, instead of making a complete copy of the structure, we would keep the root part of the tree a copy and the nested items that correspond to the path would be a copy too but the rest would still be a reference to the objects of the original tree, so in a way we're not modifying the original structure.

An interesting thing with this is that maybe we can use the same flow to get the paths (nodeByPath here) to do that, so maybe we don't need to make another iterable. This is more in theory but I can be totally wrong 😅.

I think if we get that, it's going to be awesome, because we deal with it immutable, we don't use a lot of memory to make a copy every time we modify the tree. I'm going to explore that today, I'll bring you more information whatever.

That's an interesting idea, but when you mentioned the word "reference", I still see it as "mutating" something, in the sense that you can it's value (at least that's how I see it), but I'm not saying I don't like it. Now if we'd want nodeByPath to return only the stuff we need to "update/mutate", I think it's totally possible.

That's an interesting idea, but when you mentioned the word "reference", I still see it as "mutating" something

Yeah, in a way. Because we still have a reference to the path but we haven't modified it we keep it intact, it means we have the base of the tree on the first level being a copy and the nested ones are references but the path to be modified will be a copy, so even if you're making a change this is not reflected in the original tree, this would be a semi persistence of the structure perhaps.

We are still dealing with immutability the original structure is immutable but the new structure is created with the modifications has memory-level references to the original object paths and only a part of this new structure that is new.

This is similar to this image that can give a view about it, the blue corresponds to the path being modified, ie in this case adding the e and creating the new structure based on the original with the name ys. In the path in the path are copies and the rest are just references to the original tree.

Well, I spent the afternoon looking at it but I didn't make much progress, I found some documents and articles on semi-persistent structures but nothing to help with that yet. I think I'll have to explore more coding to test how it would be done and refine the code. I'm also thinking that to do this I would need some auxiliary structure to help mount the tree.

I'm also thinking that to do this I would need some auxiliary structure to help mount the tree.

Yeah that makes sense to me

Concerning the references, I understood what you meant but I was thinking of "references" (and pointers), like in C, i.e.

int x = 5; int *px = &x; *px = 666;

anyway let's forget my original comment

Yes, perfect, they are pointers.

julien · 2021-09-09T06:31:54Z

Since I don't have permissions to push to your fork (and that you don't have permissions to push to mine), I thought pushing directly to upstream would be easier, that way we can both iterate on the same branch.

Oh true, perfect I had forgotten about that.

What do you think about using this "workflow" when we need to collaborate on the same component?
If we know we're going to need help from someone else working in the squad, we simply push our code to "upstream" (i.e. here) and it makes life easier for everyone.

About the dependency that was added by mistake, I just wanted to mention it so we don't forget.

Concerning createImmutableTree, I'll also have another look to see how far we can go.

julien · 2021-09-09T16:44:14Z

packages/clay-core/src/tree-view/useTree.ts

+						children: parentNode,
+					};
+
+					const {item} = nodeByPath(path);


@matuzalemsteles I should rename this: it's an item (or node) in the tree, but it's also the "thing" where we going to move the dropped node (maybe we could use sourceNode and destinationNode). Naming is hard 😄. Also the bracket notation ...tree[from[0]] looks a bit ugly. I'll review this part just to make it more readable (if you want to feel free).

Quite honestly I liked nodeByPath 😂

Thanks 😄 , yeah nodeByPath is fine, but it's more about that variable const {item} that I was talking about. I didn't take time to clean this up today, so I'll continue on monday

matuzalemsteles · 2021-09-09T18:00:41Z

What do you think about using this "workflow" when we need to collaborate on the same component? If we know we're going to need help from someone else working in the squad, we simply push our code to "upstream" (i.e. here) and it makes life easier for everyone.

Yes, it's a good flow to follow for these cases.

matuzalemsteles · 2021-09-09T23:53:06Z

packages/clay-core/src/tree-view/useTree.ts

+					if (from[0] === path[0]) {
+						const nodeToMove = nodeByPath(from);
+						const {children} = tree[from[0]];
+						children.splice(nodeToMove.index, 1);
+						children.splice(path[1], 0, nodeToMove.item);
+					} else {
+						const nodeToRemove = nodeByPath(from);
+						const parentNode = nodeToRemove.parent.filter(
+							(item: any, index: number) =>
+								index !== nodeToRemove.index
+						);
+
+						tree[from[0]] = {
+							...tree[from[0]],
+							children: parentNode,
+						};
+
+						const {item} = nodeByPath(path);
+
+						tree[path[0]] = {
+							...item,
+							children: [...item.children, nodeToRemove.item],
+						};
+					}


We need to clean up a little more here to make this work for nested elements, otherwise, it will only work on the first level of the tree.

Yes, that said before this change I tested in the "nested" story and it was working fine (the body of the else block). My reasoning was the following: if you have from that looks like this: [0][1], and you have path that looks like this [0][2], then you know that the items are being moved in the same "root" node, but you are right; it does need some cleaning/improvement because you could also have from that looks like this [0][1][0] and path that looks like this [0][2][1] - I'll look into that.

I see, to identify when we should move into children or move up or down from the item, we can do a comparison of path and from, compare the size of both Arrays if they are equal, and check if the indexes are equal, so we know that it is only to reorder at the same level but we still need to identify the position of the element, it is also worth checking if the reordering in the same level is something valid to be implemented for a Tree.

matuzalemsteles · 2021-09-13T20:37:05Z

I just rebase with the master to remove the commits from the other PRs, I had a lot of conflicts so I had to revisit the commits to make sure I didn't miss anything, so we still keep the history here safe.

matuzalemsteles · 2021-09-14T01:00:50Z

I made some progress today, added an implementation that replicates the idea of partial tree copying so as not to modify the original tree, added a test to cover that too, made some small fixes in useInternalState and other unrelated fixes.

The implementation of partial copy was very interesting, it was simple and follows the idea of just copying only the path that will be modified, I also use the same iterable flow that accesses the path to create the copy of the objects and keep the "pointer" of the item in the structure as we navigate.

This change does not yet handle moving the element above or below an item.

julien · 2021-09-14T10:03:28Z

packages/clay-core/src/tree-view/useTree.ts

+								index !== nodeToRemove.index
+						);
+					} else {
+						immutableTree = immutableTree.filter(


Hey @matuzalemsteles good job on the tests, however I'm seeing a "little" problem.
This is visible with the "dynamic" story, you'll see that when dragging a child node from the 3rd element (i.e. Documents and Media) to the 2nd item (i.e. Repositories) what happens is that the tree loses its first node (i.e. Liferay Drive). I added a test, but it passes, which either makes me think I'm testing the wrong thing, or using a totally different structure, so I'll review that part.

I also think we should always have a "parent", (by default it's the tree itself), because it's actually this branch of the if/else statement that is causing the bug. Anyway I think that with we're reaching our goal which means we're on the good way.

@matuzalemsteles, the "dynamic" story is missing a nestedKey prop which is what was causing the issue.

I've added it (locally for the moment), but maybe we should add a default value ("children" by default) if the user doesn't pass one, or always assume that children is going to be the nested key what do you think?
This would be one step forward to make the API a bit stricter and "defined".

@matuzalemsteles one last thing I've noticed is that with these changes, our "page elements" story is now "broken". I'm going to have a look at that now.

@matuzalemsteles I think it might be because of this, and I also think that we could rename [left, right] because it's not very clear. What do you think?

I've added it (locally for the moment), but maybe we should add a default value ("children" by default) if the user doesn't pass one, or always assume that children is going to be the nested key what do you think?

It makes sense to keep the default value for children.

one last thing I've noticed is that with these changes, our "page elements" story is now "broken". I'm going to have a look at that now.

Looks like the dynamic examples broke, let me see what change broke that.

I think it might be because of this, and I also think that we could rename [left, right] because it's not very clear. What do you think?

I'm not sure if that's what broke because this was already implemented well before making these changes to DnD, it was in the first PR of the sketch.

Well, the idea of [left, right] is that inside a TreeViewItem we can have two cases, the Stack (left) and the Group (right) but we also support the case of just having Stack, but we can change the name anyway, but nothing came. in my mind right now.

<TreeView.Item> <TreeView.ItemStack> {...} </TreeView.ItemStack> <TreeView.Group> {...} </TreeView.Group> </TreeView.Item> // or just stack <TreeView.Item> {...} </TreeView.Item>

I fix the bug, actually, it was my recent change, I had forgotten to pass the nestedKey to the context. 8cfd75c

julien · 2021-09-14T10:04:30Z

packages/clay-core/src/tree-view/useTree.ts

+	let immutableTree = [...initialTree] as T;
+
+	function pointer(tree: T, index: number, item: any) {
+		return [...tree.slice(0, index), item, ...tree.slice(index + 1)] as T;


I now understand what you meant with partial copy.

julien · 2021-09-14T13:32:52Z

@matuzalemsteles, it possibly can be improved, but in 52eaca2 I added logic to reorder the nodes inside a parent node, if you check the updated stories and try to reorder items in the "dynamic" story, you'll see what I mean (for example, drop "Google Drive" on "PDF"). Now what we need is some nice styles to make it more "visible".

matuzalemsteles

Just added some comments on the recent change, I haven't explored much today, but we're almost there. I can explore a solution tomorrow to move the item inside an item and move it up and down if you like.

matuzalemsteles · 2021-09-15T21:39:29Z

packages/clay-core/src/tree-view/__tests__/useTree.ts

+	it('can move items from the root of the tree', () => {
+		const tree = [
+			{
+				children: [{name: 'Blogs'}, {name: 'Documents and Media'}],
+				name: 'Liferay Drive',
+			},
+			{
+				children: [{name: 'Blogs'}, {name: 'Documents and Media'}],
+				name: 'Repositories',
+			},
+			{
+				children: [{name: 'PDF'}, {name: 'Word'}],
+				name: 'Documents and Media',
+			},
+		];


This case was probably always passed because it only cover one level of nesting, maybe a test with a deeper nesting might be better.

Sure I'll add a test for that too.

My point was that before my changes, in the stories this was working differently than in the tests, which is what I found strange.

I added a test for that in a118a2a, (I also removed one of my previous changes too), seems to work like it's supposed to.

matuzalemsteles · 2021-09-15T21:53:34Z

packages/clay-core/src/tree-view/useTree.ts

+					if (pathToAdd.parent) {
+						if (!Array.isArray(pathToAdd.parent[nestedKey])) {
+							pathToAdd.parent[nestedKey] = [];
+						}
+
+						pathToAdd.parent[nestedKey].splice(pathToAdd.index, 1);
+						pathToAdd.parent[nestedKey].splice(
+							pathToAdd.index,
+							0,
+							nodeToRemove.item
+						);
+						pathToAdd.parent[nestedKey].splice(
+							pathToAdd.index + 1,
+							0,
+							pathToAdd.item
+						);
+					} else {
+						if (!Array.isArray(pathToAdd.item[nestedKey])) {
+							pathToAdd.item[nestedKey] = [];
+						}
+
+						pathToAdd.item[nestedKey] = [
+							...pathToAdd.item[nestedKey],
+							nodeToRemove.item,
+						];
+					}


I've tested this use case a bit and it looks good but it's still a little weird when you try to move an item inside an item, it will add a sibling to the item instead of adding inside, I think we'll have to do something similar that we did in the Page Editor, I had made a calculation that 20% of the item's edge, will move up when it's outside of that 20%, it will move inward, the same is done for the item's bottom edge, this can be done before calling the reorder function.

But I think we would still have to use a different behavior and not use parent, maybe we have to determine to use the parent just in case when we move up and down and use the item item[nestedKey] to add the item inside, the problem is that we will almost always have parent, maybe comparing the size of the from and path along with the index values, we can know if the user wants to move inside the item.

I've tested this use case a bit and it looks good but it's still a little weird when you try to move an item inside an item, it will add a sibling to the item instead of adding inside, I think we'll have to do something similar that we did in the Page Editor, I had made a calculation that 20% of the item's edge, will move up when it's outside of that 20%, it will move inward, the same is done for the item's bottom edge, this can be done before calling the reorder function.

Yes, I agree and I'll have a look into that.

But I think we would still have to use a different behavior and not use parent

Yes, I actually removed that in a118a2a, Now we also need to add the proper styles, looking at the scss files, I think we're going to need @pat270 's help for that.

julien · 2021-09-17T09:27:21Z

hey @matuzalemsteles in 98bb47e I've added what you mentioned here. Please take a look when you can.

matuzalemsteles · 2021-09-17T15:06:22Z

@julien I still think we have to do some reorder tweaks to identify when to add above or below an item, we'll always add it as the last item, I'll try to explore that further and I'll add dragging styles in Clay CSS today as well.

matuzalemsteles · 2021-09-17T21:17:47Z

I sent a few more changes but related to the visual part of Drag and Drop, I've added new classes to the TreeView CSS to handle the drag preview visual and the dropping state hover, I haven't explored the possibility of adding the dropping state when it's moving the item above or below an item.

I also noticed some weird behavior when I move an item inside another item that has children, it creates an empty item between the items and adds it inside, I'll analyze this more deeply on Monday.

We also need to limit some cases that can break the drag and drop, for example trying to move an item that has children into itself, visually I disable this but we just need to ignore it in the drop callback.

matuzalemsteles · 2021-09-17T21:20:37Z

Hey @pat270 can you review the CSS I just added if it breaks the patterns? See the commits 0d1e582 and 4fd4ef9

matuzalemsteles · 2021-09-17T21:21:47Z

packages/clay-css/src/scss/cadmin/components/_treeview.scss

+	&.treeview-dropping {
+		$dropping: setter(map-get($link, dropping), ());
+
+		&-bottom {
+			$dropping-bottom: setter(map-get($dropping, bottom), ());
+
+			@include clay-css($dropping-bottom);
+		}
+
+		&-inside {
+			$dropping-inside: setter(map-get($dropping, inside), ());
+
+			@include clay-css($dropping-inside);
+		}
+
+		&-top {
+			$dropping-top: setter(map-get($dropping, top), ());
+
+			@include clay-css($dropping-top);
+		}
+	}


Well, I didn't find another name for this but I'm open to suggestions 😁.

julien · 2021-09-20T10:11:14Z

Hey @matuzalemsteles I think there's an issue with the isDragging variable, because when the "drag and drop" finishes, it's still set to true, and since we use that to add the disabled class in some conditions it gives "bad results". Apart from that I added logic to prevent dropping an item onto itself, and made some changes to the reorder function to allow reordering items in the same root node. Let me know what you think when you can.

matuzalemsteles · 2021-09-20T14:21:55Z

@julien thanks, I'll take a look.

matuzalemsteles · 2021-09-22T01:24:35Z

@julien I spent time today investigating to add the behaviors and classes for styling to demonstrate whether when moving an item into an item or up or down, I also brought in checking whether it's moving an item into itself to avoid problems, this checks for nested ones as well.

This is very close to finishing now, I just have to check a case that is more related to the use of index, when you move an item to another item that is at the same level in the hierarchy but inside it, in the current behavior, we will always remove the item and then add but that makes it change the length of the array and our index becomes stale and that adds some behavior like adding a new empty object to the structure, this happens also for the case of moving the item below the last item, I spent some time trying to investigate this but I couldn't find a solution to fix all the use cases, to tell you the truth I found a simple one but I have to test it to see if it will fix all cases, I'll check it out tomorrow,

matuzalemsteles · 2021-09-24T23:06:30Z

Well, I sent more changes correcting the problem of adding an empty item when navigating through paths, I did some tests and it looks pretty stable and the experience is very interesting, I added more test cases too and an improvement when dragging an item always opening an item while hovering and that's pretty bad because it increases the size of the TreeView and changes the location of the item you want to move, instead to open the item you have to hover for a few ms on the item, this demonstrates the intention to open.

I think we've reached the end of the DnD implementation I don't know what the use cases are anymore. Maybe make the DnD more configurable to avoid dropping an item on some other item, disable the DnD for a specific item and etc... this seems to be some cases I've seen in Page Elements from Page Editor.

julien · 2021-09-27T08:04:32Z

@matuzalemsteles I've reviewed the changes and they look good to me. I think we could probably rebase with master and squash a few commits before doing the final merge.

matuzalemsteles · 2021-09-27T14:53:25Z

@julien sure, makes sense to me.

…e item in drop action This is just a draft for an attempt to implement RFC 6902 (JSON Patch), it also won't cover all the cases that the RFC covers but uses the standards to implement the `move` operation which is what interests us most for perform the `drop` operation, also somewhat based on the Immer API which is used for immutability. It still has to add the implementation of applying patches and dealing with immutability which is being very difficult to do. In section 4.4 the `path` and `from` are a string but we are using an array of indices to simplify the case here. I'm not considering adding the index to tree structure to `items` because that would be an operation we need to call again when implementing "load more" and it's a little expensive depending on the depth and size, just testing other alternatives.

As we fixed the behavior of `useInternalState` to follow the behavior of if it doesn't have any of the properties it will use the internal state, we also need to support the initial state, if the initial state is not passed but there is a value in `value` prop, we should use it as state initial.

…view-dropping-inside` and `treeview-dropping-top`

…moving

…eview-dropping-middle`

…ldren

…nd its children

… the object The behavior is now different, if the indexes are the same size, it means you must move the item to the same level of the hierarchy but if you want to move the item inside an item add a new index 0 to path.

…elow an item This case is very simple and doesn't cover all the use cases of moving the item up and down over an item, add in the next interactions: - Move item above or below a nested item - Move the item below the last item in the hierarchy - Move the item above the first item in the hierarchy

…ture when navigating indexes This fixes two main issues when using indexes to move items: - Array can change the position of items when deleting first the item - Move an item below the last item

This covers most use cases when moving an item, moving a nested item, persisting the items, moving to the last item in the list.

…t for a few ms When moving an item that has a lot of nesting, opening as it moves the item to the desired place causes a bad experience, especially if the items have many items, use the strategy of opening only if it stays still on the item for a few ms that shows the intention to open the item instead of opening just swiping when you don't have an intention to open.

Some things got lost during the PR rebase with the master.

matuzalemsteles · 2021-09-27T16:43:09Z

I sent another PR #4307 with some squashed commits to avoid losing the commit history.

julien · 2021-09-28T09:53:32Z

Thanks a lot for putting that submitting that "new" pull request, which I'm going to review now (well, re-review). If I have additional comments, I'll comment there.

julien mentioned this pull request Sep 7, 2021

chore(@clayui/core): uses RFC 6902 (JSON Patch) implementation to move item in drop action #4254

Closed

matuzalemsteles reviewed Sep 8, 2021

View reviewed changes

julien mentioned this pull request Sep 9, 2021

feat(@clayui/core): adds functionality for expandable nodes and nested nodes #4222

Merged

julien commented Sep 9, 2021

View reviewed changes

matuzalemsteles reviewed Sep 9, 2021

View reviewed changes

matuzalemsteles force-pushed the issue/4207.1 branch from 64e44db to c4ffea7 Compare September 13, 2021 20:35

julien commented Sep 14, 2021

View reviewed changes

matuzalemsteles reviewed Sep 15, 2021

View reviewed changes

matuzalemsteles reviewed Sep 17, 2021

View reviewed changes

Julien Castelain and others added 2 commits September 27, 2021 12:19

feat(@clayui/core): add initial dnd implementation for TreeView

fe1e286

matuzalemsteles and others added 24 commits September 27, 2021 12:24

chore(@clayui/core): add test for move functionality

fb2080a

chore(@clayui/core): add missing nestedKey prop

376ef3b

chore(@clayui/core): make it possible to reorder child nodes

4ba0452

fix(@clayui/chore): fix error when not passing nestedKey to context

b6b8cc1

chore(@clayui/core): add test for nested items

d00d337

chore(@clayui/core): pass overTarget prop to TreeViewItem

74a2b71

chore(@clayui/core): drop either in a parent node or child node

e7eee0d

chore(@clayui/core): fix tsc errors

0564ea3

chore(@clayui/css): Add new treeview-dragging class

c78083f

fix(@clayui/core): fix drag layer position

cb5a3c4

feat(@clayui/css): adds new classes treeview-dropping-bottom, `tree…

6d36569

…view-dropping-inside` and `treeview-dropping-top`

chore(@clayui/core): add dropping-inside state and disable item when …

4b0902f

…moving

chore(@clayui/core): allow reordering items in same root node

14a76d1

chore(@clayui/core): fix TypeScript error

cc18fe3

chore(@clayui/core): replace treeview-dropping-inside class by `tre…

96e070b

…eview-dropping-middle`

fix(@clayui/core): fix error when moving item up, down and add to chi…

0d9edd2

…ldren

fix(@clayui/core): fixes error when moving the item inside the item a…

ddc8979

…nd its children

fix(@clayui/core): fixes error when adding an empty item in the struc…

9c8ee8a

…ture when navigating indexes This fixes two main issues when using indexes to move items: - Array can change the position of items when deleting first the item - Move an item below the last item

chore(@clayui/core): adds more test cases by moving nested items

0d28210

This covers most use cases when moving an item, moving a nested item, persisting the items, moving to the last item in the list.

chore: regen yarn.lock

3cddb9b

Some things got lost during the PR rebase with the master.

matuzalemsteles force-pushed the issue/4207.1 branch from 886fabb to 3cddb9b Compare September 27, 2021 15:30

matuzalemsteles mentioned this pull request Sep 27, 2021

feat(@clayui/core): implements DnD in TreeView #4307

Merged

matuzalemsteles closed this Sep 27, 2021

julien deleted the issue/4207.1 branch January 12, 2022 08:26

feat(@clayui/core): implement move functionality for tree #4255

feat(@clayui/core): implement move functionality for tree #4255

Conversation

julien commented Sep 7, 2021 • edited Loading

julien commented Sep 8, 2021

julien commented Sep 8, 2021

julien commented Sep 8, 2021

matuzalemsteles commented Sep 8, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julien Sep 9, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julien commented Sep 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matuzalemsteles commented Sep 9, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

matuzalemsteles commented Sep 13, 2021

matuzalemsteles commented Sep 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julien commented Sep 14, 2021 • edited Loading

matuzalemsteles left a comment

Choose a reason for hiding this comment

matuzalemsteles Sep 15, 2021 • edited Loading

Choose a reason for hiding this comment

julien Sep 16, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

julien commented Sep 17, 2021

matuzalemsteles commented Sep 17, 2021

matuzalemsteles commented Sep 17, 2021

matuzalemsteles commented Sep 17, 2021

Choose a reason for hiding this comment

julien commented Sep 20, 2021

matuzalemsteles commented Sep 20, 2021

matuzalemsteles commented Sep 22, 2021

matuzalemsteles commented Sep 24, 2021

julien commented Sep 27, 2021

matuzalemsteles commented Sep 27, 2021

matuzalemsteles commented Sep 27, 2021

julien commented Sep 28, 2021

julien commented Sep 7, 2021 •

edited

Loading

julien Sep 9, 2021 •

edited

Loading

julien commented Sep 14, 2021 •

edited

Loading

matuzalemsteles Sep 15, 2021 •

edited

Loading

julien Sep 16, 2021 •

edited

Loading