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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Version 3.0.2 breaks compatability with refreshControl and large titles (from react-native-navigation) #337

Closed
haveneersrobin opened this issue Nov 23, 2021 · 7 comments

Comments

@haveneersrobin
Copy link

haveneersrobin commented Nov 23, 2021

Describe the bug

We have recently updated our react-native-draggable-flatlist version from 2.6.2 to 3.0.2. Since then, multiple issues have arisen with regards to compatibility with react-native-navigtation. Important to note

  1. These issue were not present with version 2.6.0
  2. These issue do not occur when using FlatList from either react-native-gesture-handler or react-native. I have included examples below.

The issues

  • When scrolling up from the list, the title should become small for as long as you hold the 'scrollable' view.
  • The refresh control is hidden behind the navigation bar. It should appear on top of the navigation bar instead

➡   Video showcasing react-native-draggable-flatlist

draggable.mp4

Expected outcome

It should behave exactly as when you would use FlatList from either react-native-gesture-handler or react-native.

➡   Video showcasing react-native-gesture-handler's FlatList (This is the expected outcome)

rngh.mp4

To Reproduce

A snack is not possible with react-native-navigation as it uses native libraries, but I have created a small repo with a minimally reproducible example which you can use to investigate the issue. You can find it here.

Platform & Dependencies

  • react-native-draggable-flatlist version: 3.0.2
  • Platform: iOS
  • React Native or Expo version: 0.66.3 (but also happening on 0.66.1)
  • react-native-reanimated: version: 2.2.3
  • react-native-gesture-handler: version: 1.10.3

Additional context

Large titles is a design spec by Apple, see this link. You can enable this by setting the following options on your component:

topBar: {
    largeTitle: {
      visible: true,
    },
}

Please let me know if I can provide any more information!

@haveneersrobin
Copy link
Author

Additionally, this issue is still relevant in our production code as well. We have patched the source code to use the react-native's FlatList. I believe the library would still benefit from a prop with which users can pass their own AnimatedFlatListComponent.

The patch looks like this (compiled files omitted for brevity):

diff --git a/node_modules/react-native-draggable-flatlist/src/components/DraggableFlatList.tsx b/node_modules/react-native-draggable-flatlist/src/components/DraggableFlatList.tsx
index 9d9fcb1..ab3dc2a 100644
--- a/node_modules/react-native-draggable-flatlist/src/components/DraggableFlatList.tsx
+++ b/node_modules/react-native-draggable-flatlist/src/components/DraggableFlatList.tsx
@@ -8,6 +8,7 @@ import React, {
 import {
   ListRenderItem,
   FlatListProps,
+  FlatList as RNFlatList,
   NativeScrollEvent,
   NativeSyntheticEvent,
   LayoutChangeEvent,
@@ -94,6 +95,10 @@ function DraggableFlatListInner<T>(props: DraggableFlatListProps<T>) {
     activationDistance: activationDistanceProp = DEFAULT_PROPS.activationDistance,
   } = props;
 
+  //@ts-ignore
+  const RNAnimatedFlatList = Animated.createAnimatedComponent(RNFlatList) as typeof AnimatedFlatList;
+  const List = props.useReactNativeFlatList ? RNAnimatedFlatList : AnimatedFlatList;
+
   const [activeKey, setActiveKey] = useState<string | null>(null);
 
   const keyExtractor = useCallback(
@@ -398,7 +403,7 @@ function DraggableFlatListInner<T>(props: DraggableFlatListProps<T>) {
             }}
           />
           <PlaceholderItem renderPlaceholder={props.renderPlaceholder} />
-          <AnimatedFlatList
+          <List
             {...props}
             CellRendererComponent={CellRendererComponent}
             ref={flatListRef}
diff --git a/node_modules/react-native-draggable-flatlist/src/types.ts b/node_modules/react-native-draggable-flatlist/src/types.ts
index 039c5ea..54aa01a 100644
--- a/node_modules/react-native-draggable-flatlist/src/types.ts
+++ b/node_modules/react-native-draggable-flatlist/src/types.ts
@@ -33,6 +33,7 @@ export type DraggableFlatListProps<T> = Modify<
     renderItem: RenderItem<T>;
     renderPlaceholder?: RenderPlaceholder<T>;
     simultaneousHandlers?: React.Ref<any> | React.Ref<any>[];
+    useReactNativeFlatList?: boolean;
   } & Partial<DefaultProps>
 >;

@computerjazz
Copy link
Owner

Thanks for the excellent example! I've never used react-native-navigation and has no idea what it's doing under the hood to hook into the underlying scroll component, but I tracked your issue down to the PlaceholderItem component: https://github.com/computerjazz/react-native-draggable-flatlist/blob/main/src/components/DraggableFlatList.tsx#L401. Removing the placeholder item makes the header animation and refresh control work again. Maybe react-native-navigation is walking the tree and looking for the first child, and if it's a scrollable component, it hooks into it somehow? I just released a patch version that conditionally renders the placeholder, try version 3.0.3.

rndfl-rnn-fix

I'll Just a warning though, it seems like react-native-navigation has some brittle expectations around the children it renders, and I can't promise that a future update won't break this functionality again. And if you did want to render a placeholder item, looks like you'd be out of luck.

@haveneersrobin
Copy link
Author

Thanks for the fix @computerjazz ! That does improve things.

I have found another issue, I think. Normally, when the list is longer than your screen and you scroll up, the large title becomes small. It stays small unless you are at the top of the list. However, when you start dragging an item when the title is small, the tile becomes large again (which should not happen) and everything jumps down, which makes dragging the item feel clunky. It is not consistently broken, but most of the time it is.

You can test this yourself by adding items to flatListTestData in DraggableFlatListExample/FlatListConfig.ts in the repo I provided.

See video below:
https://user-images.githubusercontent.com/7559898/143426085-218f493b-050b-4b4f-9666-19cf59d4a412.mp4

@computerjazz
Copy link
Owner

computerjazz commented Dec 2, 2021

Are you sure this wasn't present in 2.6.2? The reason I ask is that I traced the issue to the scrollEnabled prop on the FlatList, which was the same in 2.6.2— if you set scrollEnabled={false} while the list is scrolled, the header becomes large again. This is not just an issue with react native draggable flatlist, I have reproduced it with the RNGH FlatList in your example as well.

Here's what it looks like when i set scrollEnabled false 1 second after beginning the drag, using the RNGH flatlist:
rndfl-rnn-scrollenabled

react-native-draggable-flatlist needs to be able to set scrollEnabled to false during a drag, or else dragging the item up and down scrolls the list instead of moves the item.

@haveneersrobin
Copy link
Author

Hmm, seems like you are right. The issue might have been present in earlier versions, apologies. Nonetheless, is there any way to overcome this, you think?

@computerjazz
Copy link
Owner

I'm not sure, and I don't see any documentation about how ScrollViews are supposed to interact with RNN (do you know where that's documented?). I'd open an issue on react-native-navigation with the scrollEnabled: false bug above, maybe they can help.

@haveneersrobin
Copy link
Author

I have (finally) found the time to report this bug to react-native-navigation seen as though this can also be reproduced with react-native's own FlatList. See: wix/react-native-navigation#7537

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

2 participants