fivecar / react-native-draglist

FlatList that can be reordered by dragging its items
MIT License
118 stars 19 forks source link

onReordered function didn't work me! Here's a fix :) #2

Closed vaibhavverma9 closed 1 year ago

vaibhavverma9 commented 1 year ago

In your README, you have the following onReordered function:

async function onReordered(fromIndex: number, toIndex: number) {
    // Since we remove the element first, account for its index shift
    const finalIndex = fromIndex < toIndex ? toIndex - 1 : toIndex;
    const copy = [...data];     // Don't modify react data in-place
    const removed = copy.splice(fromIndex, 1);

    copy.splice(finalIndex, 0, removed[0]); // Now insert at the new pos
    setData(copy);
  }

This gave me many issue. I think the state is getting reset for some reason.

Here's an onReordered function that works for me:

async function onReordered(fromIndex: number, toIndex: number) {
    let newData = data;
    const reorderedItem = data[fromIndex];
    newData.splice(fromIndex, 1);
    newData.splice(toIndex, 0, reorderedItem);
    setData(newData);
  }

I also created an Expo snack with both functions: https://snack.expo.dev/@vaibhavverma9/testing-react-native-draglist

The original onReordered function is commented out, so you can test both! Let me know if you see the same issue on the snack

fivecar commented 1 year ago

@vaibhavverma9 : thank you for reporting this! It's a bug in the DragList implementation. I aim to have a fix out for it tomorrow.

In the meantime, may I make one suggestion to your code?

  let newData = [...data];

This way, you don't modify data (because as it stands, the assignment of let newData = data simply has newData referencing the exact same array as data).

I'll ping back as soon as a fix is launched. Thank you for taking time to report the issue!

fivecar commented 1 year ago

@vaibhavverma9 : thanks for spotting a key bug. We now use useRef to track the onReordered function so that the right version of the function is always used.

Note that I've simplified the sample code now to also simply do:

  async function onReordered(fromIndex: number, toIndex: number) {
    const copy = [...data]; // Don't modify react data in-place
    const removed = copy.splice(fromIndex, 1);

    copy.splice(toIndex, 0, removed[0]); // Now insert at the new pos
    setData(copy);
  }

Thank you for reporting this issue, which helped us make an improvement in v2.0, just launched!