nuclearpasta / react-native-drax

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

Check for an individual item 'draggable' prop #89

Closed vineyardbovines closed 3 years ago

vineyardbovines commented 3 years ago

This PR adds a check in DraxList for an individual list item key draggable. This would allow individual list items to be prevented from dragging, instead of all or none (itemsDraggable). If the draggable key doesn't exist, it will fall back to itemsDraggable.

lafiosca commented 3 years ago

This is an interesting idea, but the code won't build as is.

image

info is of type ListRenderItemInfo<T>, meaning info.item is of type T which comes from the DraxList item type and can be anything (extends unknown). This means info.item.draggable is not guaranteed to exist, and in fact info.item may not be an object with accessible properties. I don't expect people to use the library that way very often, but that's how it is to align with FlatList's typings.

The way I see it, we have at least a few options here on how to implement this:

  1. The "breaking" approach: We would update the type definitions so the T in DraxList<T> must extend an object with an optional draggable property. This would be a "breaking-ts" change but probably would not have a massive impact on anyone. It would also bring the DraxList out of parity with FlatList.
  2. The hackish approach: We would do it just like you're doing it here, but we would just add an extra little check on the shape of info.item and only check for the draggable property if the shape allows that. Then we're basically saying, "If you want to use this feature, your list items must fit this shape."
  3. The over-engineered approach: We would add a new prop to DraxList called viewPropsExtractor. It would be a function that takes an item of type T and returns a partial DraxViewProps object. If this function is provided, the DraxList will call it for each item in the list and add those resulting props to the DraxView for that item. In this case, your code could look something like:
<DraxList
  ...
  viewPropsExtractor={(item) => ({ draggable: item.draggable })}
/>

I do not like approach 1 very much. My preference is approach 3 because it has the bonus of providing additional functionality for potential future requests. I could also be convinced that approach 2 is fine for simplicity of use, or even possibly doing both 2 and 3.

lafiosca commented 3 years ago

I implemented approach 3 in #93 which will be included in 0.8.0

vineyardbovines commented 3 years ago

This is great, thank you so much for the thorough explanations here @lafiosca and the implementation. This will really help a use case for my project.

Do you have a patreon or something? I'd like to throw something your way.

lafiosca commented 3 years ago

@gretzky I can't convey how much I appreciate that offer, with everything that's been going on in my life. I do not currently have a patreon, but I was intending to read up on GitHub sponsorships to see if it would make sense to enable them on this project.

lafiosca commented 3 years ago

@gretzky Thanks for inspiring me to finally set up a GitHub Sponsors account. 😸 If you're still interested, you can find the Sponsor button on this repo or on my profile!