nuclearpasta / react-native-drax

A drag-and-drop system for React Native
MIT License
554 stars 69 forks source link

Force re-measurement of absolutely positioned DraxViews on demand #112

Open GregoryWullimann opened 3 years ago

GregoryWullimann commented 3 years ago

First of all, great library, really easy to use!

But I encountered a small issue and I can't fix it. I have some DragView positioned absolutely in my app, and with a parameter you can change the position of these elements.

When the DragView are rendered the first time, the Drag and Drop works perfectly, but if the DragViews are reordered (i.e their absolute positions change), then the DragViews when dragging have some offset, like they are in the previous positions.

Is there a way to tell the DraxProvider to recalculate the elements positions? (I imagine this is the problem)

This seems to happen only on Android (Tried physical OnePlus 5T and emulated Pixel 2), while on an emulated Iphone 11 Pro it works fine!.

Might be related: https://github.com/nuclearpasta/react-native-drax/issues/78 https://github.com/nuclearpasta/react-native-drax/issues/58

GregoryWullimann commented 3 years ago

Temporary solution I'm using, which is not optimal:

Have a state that I toggle when the DragView change their positions, then set a key to the DragViews I want to rerender. So whenever the changePosition function is called, it will completely rerender the DragView, in this way it will calculate the new positions. Not optimal since the rerendering can be heavy, but good enough for now.

const [rerender, setRerender] = useState(false);

const changePosition = () =>{
    //do stuff

    // Since it's seems to be only on Android, avoid doing it on iOS to improve performance
    if (Platform.OS !== "ios") {
      setRerender(!setRerender);
    }
}

<DragView key={rerender + "" + index} />
lafiosca commented 3 years ago

I am glad you have found a workaround, and indeed I think while the other issues touch on related topics, what you’re describing about using absolute positions and having on-demand re-layouts is significantly different enough to leave this issue open as a feature request. I cannot say when it will be worked on, unfortunately.

AdamJB commented 3 years ago

This would be a useful thing to fix. I had similar issues where i had collapsable/expandable rows that needed dragging, and the offset drove me crazy. Thanks for the workaround tip! Using a set Key for the DraxView helped.

xdarkleonx commented 3 years ago

Temporary solution I'm using, which is not optimal:

Have a state that I toggle when the DragView change their positions, then set a key to the DragViews I want to rerender. So whenever the changePosition function is called, it will completely rerender the DragView, in this way it will calculate the new positions. Not optimal since the rerendering can be heavy, but good enough for now.

const [rerender, setRerender] = useState(false);

const changePosition = () =>{
    //do stuff

    // Since it's seems to be only on Android, avoid doing it on iOS to improve performance
    if (Platform.OS !== "ios") {
      setRerender(!setRerender);
    }
}

<DragView key={rerender + "" + index} />

What about DraxList? This trick is not working in DraxList. Any ideas?

lafiosca commented 3 years ago

@xdarkleonx Could you give some more detail on what you mean? Are you absolutely positioning the entire list or the items? Using a unique key on the list component to force re-render didn’t work?

xdarkleonx commented 3 years ago

I am using DraxList

 <DraxProvider>
      <View style={styles.container}>
        <DraxList
          data={alphaData}
          renderItemContent={({ item }) => renderItem(item)}
          onItemReorder={({ fromIndex, toIndex }) => reorderItems(fromIndex, toIndex)}
          keyExtractor={(item) => item}
        />
      </View>
  </DraxProvider>

and it works fine. But when orientation was changed, draggable item jumping ups outside of screen. (tested on android emulator API 29).

lafiosca commented 3 years ago

I'm sorry to keep asking questions here @xdarkleonx, but this looks like a generic DraxList example to me. Are you doing anything unusual with your list related to absolute positioning, or is your comment suggesting that orientation changes are re-measurements in general are broken (for Android API 29 emulator)? If the latter, this sounds like it should be a separate issue.

xdarkleonx commented 3 years ago

You are right. It different issue and should be separate. I will create a new issue.

ethanhusband commented 1 year ago

Been playing around with this all day, and it seems tantalizingly close to working, but even updating the key of the affected components instead of the provider (which causes too much of a delay in my use case) still doesn't seem to recalculate properly.

That said, on the initial render everything works perfectly. I wonder if it's possible to expose a method that passively refreshes/recalculates the entire Drax registry? Not sure if that's tenable, but happy to make the PR if I can be pointed in the right direction in terms of logistics required @lafiosca

lafiosca commented 1 year ago

@ethanhusband If updating the keys of the dragged views is not working, it makes me wonder if there is a more direct bug here that can be fixed rather than forcing re-render. Regardless of either solution, no one is actively maintaining the library right now. I am able to look over PRs and make a best effort guess by relying on the developers to test and explain what they did, but I am not set up for testing myself, and the library is really behind the state of the ecosystem at this point and ultimately needs to be refreshed or abandoned.

LunatiqueCoder commented 1 year ago

In our case, we found out that the onLayout event was not called on the DraxView for some reason if the DraxView position would change. Remounting (not rerendering!) the DraxView would trigger onLayout (where the measurement happens) so we went along with this workaround for some time.

Source code: https://github.com/nuclearpasta/react-native-drax/blob/main/src/DraxView.tsx#L442-L449

However, after further investigation, we also found out that having a parent <View /> wrapping a <DraxView /> would cause the onLayout event to be called only in the parent <View />, but not in the child <DraxView />.

Removing any wrapping/parent <View /> would make the onLayout event actually trigger in the <DraxView />. We also removed the remount workaround. :) Hope this helps! 🍻

Result:

image