Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DnD for both node reordering and drop as child #181

Open
rmapes opened this issue Sep 11, 2023 · 6 comments
Open

DnD for both node reordering and drop as child #181

rmapes opened this issue Sep 11, 2023 · 6 comments

Comments

@rmapes
Copy link

rmapes commented Sep 11, 2023

I would like to render the tree where each node can be at an arbitrary place in the hierarchy and dropping a node on another node makes it a child of that node.
I would also like to be able to reorder children by dragging within the child list.

At the moment, if I make all of the nodes internal nodes (i.e. they all have a child list which may be empty), then dropping a node on another node works fine (calls onMove with the target node as parent and an index of 0). However, dropping a node after another node also returns that node as a parent with an index of 0, unless the node both has children and is closed. This means that I can't implement the move behaviour consistently.

On the other hand, if I make all nodes with no children leaf nodes (by setting children to null if it is an empty array), I get the correct move behaviour (parent is parent of group and index is order within children) but I can no longer drop on the leaf nodes to create a child.

  1. Am I doing something wrong?
  2. If not, what's the best way to override the drop behaviour so that dropping on a leaf node will call onMove with that node as a parent.
@rmapes
Copy link
Author

rmapes commented Sep 14, 2023

[UPDATE]: Looks like this may be a duplicate of #140 and #144 . Will leave this open to capture some of the specific details

@jameskerr
Copy link
Member

Thank you. You are correct in that this isn't possible with the code today. This was the most useful write up of the problem though, thank you.

@GermanJablo
Copy link

if I make all of the nodes internal nodes (i.e. they all have a child list which may be empty), then dropping a node on another node works fine (calls onMove with the target node as parent and an index of 0)

I don't know if I would describe this as "works fine". Index 0 is opinionated. Maybe I want to add it as the last child.
It would be best to differentiate between drop "inside", "before" and "after".

@jameskerr
Copy link
Member

Can you try this again with the lastest version? We make some changes in #202 that made you able to drop as a sibling of a "open" node with no children.

If that addresses this, let's close this and those other issues. Thanks @rmapes

@rmapes
Copy link
Author

rmapes commented Feb 11, 2024

It doesn't seem to have changed anything. With all of the nodes having an empty child list, I can still drop as a sibling if the node is closed, but if the node is open it will try and add it as a child. Dropping on the node will also add as a child, as expected.

@StephenPCG
Copy link

@rmapes I met the same problem with you. After some digging, I realize that #202 indeed solves the problem. Watch the demo video closely:
image
image
When cursor is below an open empty node, move your mouse left and right, you will see the cursor length changes, indicate whether the drop will be a sibling or child.

However, this is really bad experience for users. I've figured a better approach: make node with empty children closed by default, then if the drop cursor is below the node, the drop will always be sibling, and if the drop cursor is above the node, the drop will be a child. This should be a intuitive way for users.

My pesdo code:

const Node = ({node}) => {
    useMount(() => {
        if (node.children.length === 0) {
            node.close()
        } else {
            node.open()
        }
    })

    return <div className="node">...</div>
}

const App = () => {
    return <Tree ...>{Node}</Tree>
}

useMount is the same as useEffect but only run for the first time component is rendered.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants