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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
LeadDreamer opened this issue Mar 10, 2024 · 0 comments

Comments

@LeadDreamer
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant