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

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

Closed
2 tasks done
twhiteheadzm opened this issue Sep 15, 2022 · 6 comments · Fixed by #7357
Closed
2 tasks done
Assignees
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Reordering Related to the data grid Reordering feature

Comments

@twhiteheadzm
Copy link

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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

@twhiteheadzm twhiteheadzm added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Sep 15, 2022
@cherniavskii
Copy link
Member

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

Screen.Recording.2022-09-20.at.15.03.38.mov

@cherniavskii cherniavskii added bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Reordering Related to the data grid Reordering feature and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 20, 2022
@cherniavskii cherniavskii moved this to 🆕 Needs refinement in MUI X Data Grid Sep 20, 2022
@cherniavskii
Copy link
Member

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
Copy link
Member

Also, I expect we will face the same issue with column reordering (after #6236 is fixed)

@flaviendelangle flaviendelangle changed the title onRowOrderChange not being called sometimes when dragging causes scrolling in datagrid. [data grid] onRowOrderChange not being called sometimes when dragging causes scrolling in datagrid. Sep 29, 2022
@flaviendelangle flaviendelangle changed the title [data grid] onRowOrderChange not being called sometimes when dragging causes scrolling in datagrid. [data grid] onRowOrderChange is not called sometimes when dragging causes scrolling in datagrid. Sep 29, 2022
@yaredtsy
Copy link
Contributor

yaredtsy commented Nov 5, 2022

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
Copy link
Member

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

@yaredtsy
Copy link
Contributor

yaredtsy commented Nov 7, 2022

i have submitted a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work component: data grid This is the name of the generic UI component, not the React module! feature: Reordering Related to the data grid Reordering feature
Projects
Status: Done
4 participants