inovua / reactdatagrid

Empower Your Data with the best React Data Grid there is
https://reactdatagrid.io
Other
3.45k stars 57 forks source link

🐛 Bug: drag & drop callbacks return incorrect data objects (with undocumented work-arounds) #412

Open LeadDreamer opened 3 months ago

LeadDreamer commented 3 months ago

enterprise edition, ver "5.10.2"

Relevant code or config

EnterpriseColumnLayout.js, line 815:


    dragEndCallbacks = (props, dropIndex) => {
        if (dropIndex) {
            const data = props.data[dropIndex];
            props.onRowReorderEnd && props.onRowReorderEnd({ data, dropIndex });
        }
    };

    When this occurs, the drag data has ALREADY BEEN REMOVED from the array, so dropIndex CANNOT point to the data

What you did:

While implementing row reorder, numerous bugs and/or missing data were noted.
For example, onRowReorder is only called (callback) when there is no grouping, but documentation indicates that it should be possible even when grouped.
After tracing back from a test onRowReorder without grouping, This lead to an (undocumented) callback "onGroupRowReorder", which did not return relevant data - the above code snippet makes it clear why - the row has been removed from the internal dataSource cache (reasonably) at the start of the drag. Unfortunately, apparently nobody told the person responsible for the callback code.

Reproduction repository: https://github.com/SaltSweetSpirits/reactdatagrid_bug_example

Reproduction CodeSandBox: https://codesandbox.io/p/github/SaltSweetSpirits/reactdatagrid_bug_example/main

Problem description: onRowReorder is only called (callback) when there is no grouping, but documentation indicates that it should be possible even when grouped.
After tracing back from a test onRowReorder without grouping, This lead to an (undocumented) callback "onGroupRowReorder", which did not return relevant data - the above code snippet makes it clear why - the row has been removed from the internal dataSource cache (reasonably) at the start of the drag. Unfortunately, apparently nobody told the person responsible for the callback code.

Suggested solution: There are more callbacks available than are in the published documentation. Specifically, all of the following can be relevant: onRowReorder onRowReorderStart onRowReorderEnd onGroupRowReorder onGroupRowReorderStart onGroupRowReorderEnd

onRowReorder and onGroupRowReorder are not useful because of the bug, above.

onRowReorderStart and onGroupRowReorderStart are useful because they provide a copy of the data in the row being drag/drop.

onRowReorderEnd and onGroupRowReorderEnd are PARTIALLY useful, in that they signal the completion of the process, and in the case of Grouped data return the new GROUP status values (unfortunately as a "/" separated string, rather than an array of strings).

A functional sequence is to use onGroupRowReorderStart to capture the data Row at the start of the operation, and stash a copy. Then use onGroupRowReorderEnd to capture the event, update the grouped values from the dropGroup string and update the row in the original dataSource BY ID.

Note there is a subtlety here: while the drag'n'drop operation is occurring, the parent React state machine is NOT running - so cannot use useState cache the original row value - I used a useRef.current to stash the value. This also causes issues when attempting to clear the cache.

  const dragRow = useRef(null);
  ...
  const onGroupRowReorderStart = useCallback(({ data, dragGroup }) => {
    dragRow.current = data;
    setDragGroup(dragGroup);
  });
...
  const onGroupRowReorderEnd = useCallback(({ data, dropGroup }) => {
    const dropGroups = dropGroup.split("/");
    setDataSource((prev) => {
      const newData = [...prev];
      const matchIndex = newData.findIndex(
        (row) => row.id === dragRow.current.id,
      );
      groupBy.forEach((group, index) => {
        newData[matchIndex][group] = dropGroups[index];
      });
      return newData;
    });
    setDragGroup(null);
  }, []);

EXPECTED CHANGES: => Reconcile the dataRow extracted from the internal dataSource cache - either by NOT removing it, or caching it to return at the end of the operation. => Document the onGroupRowReorder callbacks and functionality - I shouldn't have to use developer tools, breakpoints and callstacks to discover these.