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.52k stars 1.31k forks source link

[data grid] `onRowOrderChange` is not called sometimes when dragging causes scrolling in datagrid. #6165

Closed twhiteheadzm closed 1 year ago

twhiteheadzm commented 2 years ago

Duplicates

Latest version

Current behavior 😯

When dragging a row up or down and it causes scrolling of the table, it sometimes does not call onRowOrderChange. Seems to happen most when scrolling a long way. The row is moved correctly in the grid and no errors shown in console. Dragging it again a short distance does trigger onRowOrderChange.

Expected behavior 🤔

onRowOrderChange should be called whenever a row is dragged.

Steps to reproduce 🕹

Link to live example:

Steps: 1. 2. 3. 4.

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```

Order ID 💳 (optional)

No response

cherniavskii commented 2 years ago

Thanks @twhiteheadzm I can reproduce it with this demo: https://codesandbox.io/s/sparkling-architecture-gp9lmp?file=/demo.tsx

https://user-images.githubusercontent.com/13808724/191265334-a8b8e2cb-4fe9-4459-a5e4-4867f90b328b.mov

cherniavskii commented 2 years ago

With virtualization enabled, the GridRowReorderCell is being removed from the DOM at some point during scroll, so no dragEng event is fired on this element. We will have to rethink our implementation of reordering to make it work with virtualization. Here's some context from react-beautiful-dnd: https://github.com/atlassian/react-beautiful-dnd/blob/master/docs/patterns/virtual-lists.md#use-the-droppable---renderclone-api

As a workaround, you can disable virtualization with disableVirtualization prop.

cc @DanailH

cherniavskii commented 2 years ago

Also, I expect we will face the same issue with column reordering (after https://github.com/mui/mui-x/issues/6236 is fixed)

yaredtsy commented 2 years ago

hey @cherniavskii what if we use addEventListener for listening for dragend event? The reason we have to manually remove the listener when the component dismounts is, the listeners persist even when the parent component is unmounted. right?

--- a/packages/grid/x-data-grid-pro/src/components/GridRowReorderCell.tsx
+++ b/packages/grid/x-data-grid-pro/src/components/GridRowReorderCell.tsx
@@ -31,6 +31,7 @@ const useUtilityClasses = (ownerState: OwnerState) => {

 function GridRowReorderCell(params: GridRenderCellParams) {
   const apiRef = useGridApiContext();
+  const ref = React.useRef<HTMLDivElement>(null);
   const rootProps = useGridRootProps();
   const sortModel = useGridSelector(apiRef, gridSortModelSelector);
   const treeDepth = useGridSelector(apiRef, gridRowMaximumTreeDepthSelector);
@@ -81,9 +82,19 @@ function GridRowReorderCell(params: GridRenderCellParams) {
     [apiRef, params.id],
   );

+  const dragEnd = (event: React.MouseEvent<HTMLDivElement> | any) => {
+    event.currentTarget.removeEventListener('dragend', dragEnd);
+    publish('rowDragEnd')(event);
+  };
+
   const draggableEventHandlers = isDraggable
     ? {
-        onDragStart: publish('rowDragStart'),
+        onDragStart: (event: React.MouseEvent<HTMLDivElement>) => {
+          // IF virtualization is enabled
+
+          ref.current?.addEventListener('dragend', dragEnd);
+          publish('rowDragStart')(event);
+        },
         onDragOver: publish('rowDragOver'),
         onDragEnd: publish('rowDragEnd'),
       }
@@ -94,7 +105,7 @@ function GridRowReorderCell(params: GridRenderCellParams) {
   }

   return (
-    <div className={classes.root} draggable={isDraggable} {...draggableEventHandlers}>
+    <div ref={ref} className={classes.root} draggable={isDraggable} {...draggableEventHandlers}>
       <rootProps.components.RowReorderIcon />
       <div className={classes.placeholder}>{cellValue}</div>
     </div>

Demo: https://codesandbox.io/s/awesome-field-rh2gs8?file=/demo.tsx

cherniavskii commented 2 years ago

@yaredtsy Nice, I like the idea! Wanna submit a PR?

yaredtsy commented 2 years ago

i have submitted a PR.