-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[TreeView] Support item reordering using drag and drop #12213
base: master
Are you sure you want to change the base?
[TreeView] Support item reordering using drag and drop #12213
Conversation
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have implemented drag and drop for the pivoting panel in #9877 – there's no touchscreen support yet, but maybe you'll find it useful.
Preview: https://deploy-preview-9877--material-ui-x.netlify.app/x/react-data-grid/pivoting/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
What was the reason why you went for the touchXXX
event instead of dragXXX
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did I? I don't recall using touch events
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
mouseXXX
sorry, which will force you to use touchXXX
to support mobile from what I understand
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember exactly, but it probably didn't work as expected.
I will double check this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I used mouse events only for panel resizing. But after reading Matheus' comment in #9330 (comment) I switched to pointer events – see https://github.com/cherniavskii/mui-x/blob/pivoting/packages/x-data-grid-premium/src/hooks/utils/useResize.ts
For drag & drop, I used the draggable
attribute and drag events. I've extracted the useResize
hook to a separate file to avoid confusion: https://github.com/cherniavskii/mui-x/blob/pivoting/packages/x-data-grid-premium/src/components/GridPivotPanel.tsx
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
f953fb6
to
88a8b4d
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
packages/x-tree-view/src/TreeItem2DragAndDropOverlay/TreeItem2DragAndDropOverlay.tsx
Outdated
Show resolved
Hide resolved
1587447
to
f48f825
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
@@ -4,3 +4,9 @@ export type TreeViewItemId = string; | |||
export type TreeViewBaseItem<R extends {} = { id: string; label: string }> = R & { | |||
children?: TreeViewBaseItem<R>[]; | |||
}; | |||
|
|||
export type TreeViewItemsReorderingAction = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This type need to be on the community package because it is used in TreeItem2DragAndDropOverlay
which is on the community package (to avoid creating a different Tree Item component on the pro package)
], | ||
})); | ||
|
||
function TreeItem2DragAndDropOverlay(props: TreeItem2DragAndDropOverlayProps) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is on the community package because it allows to have a single Tree Item component shared across all plans.
For now I did not try to add any shiny abstraction to have it in the pro package, I think having a dummy component here is the best trade-off
If we want to go a little further, we can see how to inject the style from the pro plan since it's almost all the this component currently have, but I don't think it's worth the effort for now
@@ -122,6 +124,9 @@ export const useTreeView = <Plugins extends readonly TreeViewPlugin<TreeViewAnyP | |||
contextValue.runItemPlugins = (itemPluginProps) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I'll move all the runItemPlugin
calls in a standalone hook to be able to document it more thoroughly, but it would make the review harder so it stays here for now.
The idea of these new enhancers is to have each plugin responsible for generating its props passed to the various slots.
It allows to have TreeItem2DragAndDropOverlay
being super dumb.
If we scale this to more plugins, we might have to have a smarter merging process for callbacks and refs, but I don't think it's a top priority.
}; | ||
|
||
return ( | ||
<TreeItem2Provider itemId={itemId}> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think we can skip re-creating the whole Tree Item here in order to get the content props and re-dispatch them to the drag handle.
One way of doing it would be to add handleDragStart
and handleDragStop
on useTreeItem2Utils
.
That way, this demo could just override its content
slot to render the default TreeItem2Content
with the children
it receives plus the added icon (and pass the handlers to it from useTreeItem2Utils
).
It could then use slotProps
and defaultMuiPrevented
to remove the behaviors from the root.
Basically it would be closer to this demo, which would be super nice because if in the future we add editing, or anything else, it would just work out of the box.
But adding it to useTreeItem2Utils
is not trivial because it means this hook must be able to be enriched from the plugins somehow, and right now it does not.
That will be a nice follow up.
export interface UseTreeItem2DragAndDropOverlaySlotPropsFromItemsReordering | ||
extends TreeItem2DragAndDropOverlayProps {} | ||
|
||
declare module '@mui/x-tree-view/useTreeItem2' { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One limitation of using declare
here is that if you use one RichTreeView
and one RichTreeViewPro
, and if this RichTreeView
use a custom Tree Item, then this custom Tree Item will have a drag & drop props on the value returned by getRootProps
and the other functions.
I think it's fine
Opened for review |
|
||
{{"demo": "ApiMethodGetItemTree.js", "defaultCodeOpen": false}} | ||
|
||
:::info |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We will be able to create a way better demo once we have the label editing
aa4dc51
to
461c01a
Compare
461c01a
to
5fbedd1
Compare
This pull request has conflicts, please resolve those before we can evaluate the pull request. |
Doc preview
Closes #9686
Requirements for this PR
RichTreeViewPro
component #12610Basic example
indentationAtItemLevel
experimental feature (see [TreeView] Allow to define indentation at the item level #13126)itemsReordering
propCurrent status
TreeItem
TreeItem2