mui / mui-x

MUI X: Build complex and data-rich applications using a growing list of advanced React components, like the Data Grid, Date and Time Pickers, Charts, and more!
https://mui.com/x/
4.06k stars 1.25k forks source link

[tree view] Add drag & drop support #9686

Closed oliviertassinari closed 1 month ago

oliviertassinari commented 4 years ago

An initial issue to keep track of our progress on the drag & drop feature. Note that we won't make it MIT.

Benchmarks

felipezacani commented 4 years ago

Hey @oliviertassinari thanks for the link! However, even though I may use this in the future, what I wanted now was a very simple behavior, like the native html drag n drop. I just wanted to add the draggable=true attribute to a TreeItem.

oliviertassinari commented 4 years ago

@felipezacani What stops you from achieving it?

felipezacani commented 4 years ago

@felipezacani What stops you from achieving it?

Not really sure if there is any other way of passing the attribute to the correct native item. I tried the same as I did successfuly with the Paper component, adding a "draggable={true}" to its tag, it doesn't work and its not passed down to any native component (checked on FirefoxDev inspector). I also noticed userSelect="none" has no effect on the TreeItems.

felipezacani commented 4 years ago

@oliviertassinari problem was on my side, I had another component (mounted) using react-dnd. I'm still not sure why exactly, but if a react-dnd component is mounted, all other native draggable attributes are disabled (unless you use react-dnd in everything, of course). Thanks anyway!

felipezacani commented 4 years ago

@oliviertassinari problem was on my side, I had another component (mounted) using react-dnd. I'm still not sure why exactly, but if a react-dnd component is mounted, all other native draggable attributes are disabled (unless you use react-dnd in everything, of course). Thanks anyway!

joziahg commented 2 years ago

Any new thoughts or progress on this front?

JPNZ4 commented 2 years ago

Perhaps if we could use the prop for onFocus in TreeItem, we could trade off the keyboard navigation for dragging. My use case I need the drag and don't need the keyboard navigation.

https://github.com/mui-org/material-ui/blob/3f58dd9208c5aa8daed74c74d97cd63c2c35a785/packages/mui-lab/src/TreeItem/TreeItem.js#L393-L397

The TreeItem uses the below function for onFocus, which breaks the drag with the prevent scroll. I have tried to turn this off using the TreeView prop onNodeFocus but that ends up in a loop...

https://github.com/mui-org/material-ui/blob/3f58dd9208c5aa8daed74c74d97cd63c2c35a785/packages/mui-lab/src/TreeItem/TreeItem.js#L264-L274)

joziahg commented 2 years ago

Here is how I'm using react-dnd with TreeView: https://codesandbox.io/s/dawn-resonance-pgefk?file=/src/Demo.js

Could be useful to some

JPNZ4 commented 2 years ago

Here is how I'm using react-dnd with TreeView: https://codesandbox.io/s/dawn-resonance-pgefk?file=/src/Demo.js

Could be useful to some

This is not working in any chromium browsers for me (95.0.4638.69). Only Firefox. Is it working for you @joziahg ?

joziahg commented 2 years ago

Here is how I'm using react-dnd with TreeView: https://codesandbox.io/s/dawn-resonance-pgefk?file=/src/Demo.js Could be useful to some

This is not working in any chromium browsers for me (95.0.4638.69). Only Firefox. Is it working for you @joziahg ?

This is a known issue, you can follow it here mui/material-ui#29518

aludin commented 2 years ago

@oliviertassinari Currently evaluating mui pro for a client project. Being able to perform dnd with the TreeView is a big part of the workflow. Tried the following:

<TreeItem
   draggable
   label="APP"
  nodeId="1"
   onDragStart={() => {
        console.log("ondragStart");
   }}
  onDrag={() => {
       console.log("onDrag");
  }}
/>

The drag events are triggered with FireFox but not with Chrome/Edge. Is there a workaround to make this work?

mattsrobot commented 2 years ago

Just wondering but TreeView is advertised in the MUI-X product, but I can't personally see any noticeable difference between the standard MUI and MUIX versions? Is this correct?

dd-ssc commented 2 years ago

I would like to share a few learnings from the last couple of days of working on drag'n'drop and the MUI tree view:

I have created a sandbox based on this repo to illustrate my points.

The upper tree view in the sandbox uses a DraggableTreeItem that takes care of drag'n'drop (useDrag and what not) and wraps a MUI TreeItem. As far as I can tell, this would be a fairly standard way to implement dragging a tree item out of a tree view; if I'm wrong there, I'd appreciate if someone could correct me.

The bottom tree view uses a DraggableCustomItem that is identical to DraggableTreeItem, but wraps the CustomItem also used everywhere else in the app and is attached to the actual TreeItem as its label prop.

The two drop targets to the right are identical; a few sample CustomItems were already dropped in both.

The app in the sandbox suffers from issue mui/material-ui#29518: In Firefox, drag'n'drop works for both tree views; for Chrome or Safari, I am using the stopImmediatePropagation() workaround from that issue to make the upper tree view work; I could not get the bottom one to work outside of Firefox so far.

A few things to note about the upper tree view:

  1. When dnd'ing an expanded item, the item is shown while dragging including all its children. Depending on the use case, this might make sense; IMO, it looks somewhat unusual. (related SO question)
  2. What worse IMO is that the tree item is shown while dragging - in its full (tree view) width, possibly including an icon to the left and some border to the right. There might be some way to mitigate that (CSS or so), but I don't have the required skills.
  3. My original intention was to make the tree view items look like those boxes in the drop targets (as is now done in the lower tree view) for some visual coherence. I tried real hard for quite a while to use the ContentComponent prop because (judging from its prominence in the docs) that seemed the way to go at first, but then I realized that approach requires me to do all the node expansion / collapsing / selection handling myself; I eventually gave up when it became just hopeless; related SO question.

My personal impression so far is that it might not be the best approach to try and make the TreeItem itself dnd'able.

About the bottom tree:

  1. No matter if the item is expanded or collapsed or if it has any children, only the item itself is dnd'd.
  2. The item being dragged only has the size of itself, not full (tree view) width, no icons to the left, no border to the right.
  3. Using a custom component as tree item was a piece of cake - once I had realized I can use not only a string, but an entire component as TreeItem label prop. Technically, the API docs do state node as type (not string or so) for that prop, but label as a prop name IMO really leads a naive first time user (such as myself) to believe otherwise - and the description (The tree node label.) doesn't really help, either.

The drawback: As stated, the bottom tree currently only works in Firefox. Obviously unacceptable for a production app.

Also, I'm not much of a UI/UX guy, but IMHO, as a user, I would expect to drag only the item itself around, not an item of a tree view. Look at a tree view in macOS Finder or Windows Explorer: The "production" item object thingie I am working with when using an app that I - as a user - associate with for example a file or a folder doesn't have tree view icons to the left or some border to the right.

Finally, I have a few related questions that are probably too specific for StackOverflow, so asking here:

  1. What is the point of the ContentComponent prop in the first place when I can achieve the same result, to override the content component, much simpler using the label prop ? Is it just to override the expansion/collapsing/selection behavior as in the example ? (BTW: It took me quite a while actually understand what that example actually does...)
  2. Is there any way to get this stopImmediatePropagation() workaround to work for CustomItems ? I only have a fuzzy understanding of what it does and I have no clue why it works, but as it did the trick for Chrome with TreeItems, I tried to use it for CustomItems, but it seems I don't have the TypeScript skills to understand what's wrong and how to fix it, running into all sorts of typing issues and compiler errors. Is there at least a theoretical chance to make this work (e.g. by enabling the custom item to receive a ref of the corresponding type) ?

Could anyone kindly share some thoughts and possibly lend a hand in getting drag'n'drop to work ? It's not a major problem for my use case if the focus breaks or whatnot, but getting this to work in all major browsers would be mean huge progress for my current project.

Just my 2 cents. Thank you very much for your attention.

radu-nicoara-bayer commented 1 year ago

Having an easy way of using drag and drop in TreeView would also mean a lot for our business.

mattsrobot commented 1 year ago

What about using arborist and using MUI for the themed elements?

CHE1RON commented 1 year ago

Bumping this, also using arborist under the hood would be great 🤩

github-actions[bot] commented 1 month ago

:warning: This issue has been closed. If you have a similar problem but not exactly the same, please open a new issue. Now, if you have additional information related to this issue or things that could help future readers, feel free to leave a comment.

@oliviertassinari: How did we do? Your experience with our support team matters to us. If you have a moment, please share your thoughts in this short Support Satisfaction survey.

oliviertassinari commented 1 month ago

What about using arborist and using MUI for the themed elements?

@mattsrobot We could have done this, a simple UI layer, pretty much what Shadcn does with most of its components. Now, our aim is to build a tree view that we can then expose unstyled, and used with, e.g. Mantine. We want to be able to go more in-depth in the execution than, e.g. arborist.