software-mansion / react-native-gesture-handler

Declarative API exposing platform native touch and gesture system to React Native.
https://docs.swmansion.com/react-native-gesture-handler/
MIT License
5.85k stars 954 forks source link

ScrollView breaks RefreshControl on Android #1067

Closed janicduplessis closed 1 year ago

janicduplessis commented 4 years ago

When using ScrollView from rngh RefreshControl no longer works. This happens because on Android RefreshControl works by wrapping the ScrollView with a SwipeRefreshLayout component. This component should interact with the rngh gesture system since it needs to be able to interrupt and recognize simultaneously the ScrollView it wraps.

This can be kind of accomplished by wrapping RefreshControl with createNativeWrapper on Android. This is the first part of the hack patch I have to fix this:

GestureComponents.js

     });
   },
+  get RefreshControl() {
+    if (Platform.OS === 'android') {
+      return memoizeWrap(ReactNative.RefreshControl, {
+        disallowInterruption: true,
+        shouldCancelWhenOutside: false,
+      });
+    } else {
+      return ReactNative.RefreshControl;
+    }
+  },
   get Switch() {
     return memoizeWrap(ReactNative.Switch, {

The problem then is that ScrollView sets disallowInterruption to true, which means it cannot get interrupted or recognize with another gesture handler. Setting disallowInterruption to false makes RefreshControl work but causes other issues like nested ScrollView will both scroll at the same time.

I tried playing with adding simultaneousHandlers or waitFor to the RefreshControl associated with the ScrollView but wasn't able to get it working. Seems like disallowInterruption takes priority over that.

So at this point I was mostly looking for a hack to get it working so I came up with:

NativeViewGestureHandler.java

 import android.view.ViewGroup;

+import androidx.swiperefreshlayout.widget.SwipeRefreshLayout;
+
 public class NativeViewGestureHandler extends GestureHandler<NativeViewGestureHandler> {

   private boolean mShouldActivateOnStart;
@@ -48,7 +50,7 @@ public class NativeViewGestureHandler extends GestureHandler<NativeViewGestureHa
       }
     }

-    boolean canBeInterrupted = !mDisallowInterruption;
+    boolean canBeInterrupted = !shouldDisallowInterruptionBy(handler);
     int state = getState();
     int otherState = handler.getState();

@@ -62,9 +64,19 @@ public class NativeViewGestureHandler extends GestureHandler<NativeViewGestureHa
     return state == STATE_ACTIVE && canBeInterrupted;
   }

+  private boolean shouldDisallowInterruptionBy(GestureHandler handler) {
+    if (handler.getView() instanceof SwipeRefreshLayout) {
+      return false;
+    }
+    return mDisallowInterruption;
+  }
+
   @Override
   public boolean shouldBeCancelledBy(GestureHandler handler) {
-    return !mDisallowInterruption;
+    if (handler.getView() instanceof SwipeRefreshLayout) {
+      return true;
+    }
+    return !shouldDisallowInterruptionBy(handler);
   }

   @Override

Basically it just special cases when the other handler is SwipeRefreshLayout so that even if mDisallowInterruption is true it will treat it like it was false.

Not sure how this could be properly fix, will keep using this hack for now.

WadhahEssam commented 3 years ago

@janicduplessis

I got it working by putting a ScrollView inside a ScrollView so the code looks like this:

<PanGestureHandler>
 <ScrollView>
  <ScrollView refreshControl={<RefreshControl />}>
   // rest
  </ScrollView>
 </ScrollView>
</PanGestureHandler>

What side effects should I expect ?

kmagiera commented 3 years ago

@jakub-gonet do you maybe have time to prioritize this one?

jakub-gonet commented 3 years ago

@janicduplessis, could please describe what is broken here (expected/current behavior)? I used this code as a repro example and it seems alright to me:

import React, { Component, useCallback } from 'react';
import { SafeAreaView, View, Text, RefreshControl } from 'react-native';

import { ScrollView } from 'react-native-gesture-handler';

const wait = timeout => {
  return new Promise(resolve => {
    setTimeout(resolve, timeout);
  });
};
export default function App() {
  const [refreshing, setRefreshing] = React.useState(false);

  const onRefresh = useCallback(() => {
    setRefreshing(true);

    wait(1500).then(() => setRefreshing(false));
  }, []);

  return (
    <SafeAreaView style={styles.container}>
      <ScrollView
        contentContainerStyle={styles.scrollView}
        refreshControl={
          <RefreshControl refreshing={refreshing} onRefresh={onRefresh} />
        }>
        <Text>Pull down to see RefreshControl indicator</Text>
      </ScrollView>
    </SafeAreaView>
  );
}

const styles = {
  container: {
    flex: 1,
  },
  scrollView: {
    flex: 1,
    backgroundColor: 'pink',
    alignItems: 'center',
    justifyContent: 'center',
  },
};

android-ios

EDIT: I was running this example on fresh RN 0.63.2 installation

janicduplessis commented 3 years ago

Hmm strange, I can repro using the same code in my app by replacing everything in App.js, but can't repro in snack. Issue might be related to the react-native version I'm using in my app, will investigate more.

janicduplessis commented 3 years ago

@jakub-gonet I was able to reproduce with https://github.com/janicduplessis/rnghtest

I had to make the scrollview scrollable to reproduce. It is just a simple react-native init project and added rngh. Still doesn't repro in snack so it might only happen on more recent RN versions.

Tested on Pixel 3 API 29 emulator

import React, {useCallback} from 'react';
import {View, Text, RefreshControl} from 'react-native';

import {ScrollView} from 'react-native-gesture-handler';

const wait = (timeout) => {
  return new Promise((resolve) => {
    setTimeout(resolve, timeout);
  });
};
export default function App() {
  const [refreshing, setRefreshing] = React.useState(false);

  const onRefresh = useCallback(() => {
    setRefreshing(true);

    wait(1500).then(() => setRefreshing(false));
  }, []);

  return (
    <ScrollView
      style={{flex: 1}}
      refreshControl={
        <RefreshControl refreshing={refreshing} onRefresh={onRefresh} />
      }>
      <View
        style={{
          height: 2000,
          paddingTop: 64,
          backgroundColor: 'pink',
          alignItems: 'center',
        }}>
        <Text>Pull down to see RefreshControl indicator</Text>
      </View>
    </ScrollView>
  );
}
pke commented 3 years ago

On 0.62.2 setting the the refresh control on Android the content of the default RN ScrollView is no longer rendered. Maybe that's related?

jakub-gonet commented 3 years ago

@janicduplessis, I tried to reproduce it and seems like it's not working. After replacing ScrollView with RN one it still doesn't work. It's a problem with RNGH though, I'll try to deep deeper into that.

vivekjm commented 3 years ago

I had the same issue with the scrollview from react-native-gesture-handler tried so many methods finally i just switched to scrollview from react-native to solve this issue

Guuz commented 3 years ago

I ran into this issue today as well. A RNGH scrollview with content that is large enough to scroll. Adding a RefreshControl. And it doesn't work. If the content is small enough that no scrolling is needed it does work. Im 100% sure this worked a few months ago when i build these screens.

Any idea on how to fix this properly? And what are the downsides of using the RN scrollview? this was never 100% clear to me (why the RNGH scrollview is better).

edit: @jakub-gonet can you comment on this?

anantadwi13 commented 3 years ago

I'm still getting this issue in RNGH v1.9.0 and RN v0.63.3. Testing the above code from @janicduplessis take me to this issue on Android, but it doesn't happen on iOS.

lfaz commented 3 years ago

facing same issue, swipe up on android emulator causing refresh controll trigger, works all good on ios

xmflsct commented 3 years ago

Same here. I tried multiple times, and few of the times it worked. Is it the same with you that randomly it works?

tdekoning commented 3 years ago

@jakub-gonet Any update on this?

FRizzonelli commented 3 years ago

I ran into this issue today as well. A RNGH scrollview with content that is large enough to scroll. Adding a RefreshControl. And it doesn't work. If the content is small enough that no scrolling is needed it does work. Im 100% sure this worked a few months ago when i build these screens.

Any idea on how to fix this properly? And what are the downsides of using the RN scrollview? this was never 100% clear to me (why the RNGH scrollview is better).

edit: @jakub-gonet can you comment on this?

This is exactly the same problem I have. @Guuz Did you find any solution? I'm creating an animated flat list, if I import the FlatList from react-native it does work, from rngh doesn't :(

Guuz commented 3 years ago

@FRizzonelli no i have not :(

my-name-is-nheo commented 3 years ago

I had bounce set to false. But removing that, solved the issue for me. Baffling.

evrimfeyyaz commented 3 years ago

@jakub-gonet Is there an update on this? Is there anything we can do to help?

isabellaskc commented 3 years ago

I had this issue as well, switched the import of ScrollView from 'react-native-gesture-handler' to 'react-native' , to get it to work for now

sherif95 commented 2 years ago

I had the same issue as well , imported the ScrollView from 'react-native' and it's working fine now without any problems

jakub-gonet commented 2 years ago

I debugged this for a bit and looks like refresh control is getting interrupted by scroll view (that has disallowInterruption: true). I'm considering some solutions to this problem but I want to avoid adding a special case for ScrollView.

As a workaround for now, you can wrap Refresh Control in NativeView Gesture handler and use waitFor with it:

export const RefreshControl = createNativeWrapper(RNRefreshControl, {
  disallowInterruption: true,
  shouldCancelWhenOutside: false,
});

const refreshRef = useRef(null);

<ScrollView waitFor={refreshRef} refreshControl={<RefreshControl ref={refreshRef} />}>
    {/* ... */}
</ScrollView>
Full example ```ts import React, { useCallback, useRef } from 'react'; import { View, Text, RefreshControl as RNRefreshControl } from 'react-native'; import { createNativeWrapper, ScrollView } from 'react-native-gesture-handler'; const RefreshControl = createNativeWrapper(RNRefreshControl, { disallowInterruption: true, shouldCancelWhenOutside: false, }); const wait = (timeout: number) => { return new Promise((resolve) => { setTimeout(resolve, timeout); }); }; export default function App() { const refreshRef = useRef(null); const [refreshing, setRefreshing] = React.useState(false); console.log(refreshing); const onRefresh = useCallback(() => { setRefreshing(true); void wait(1000).then(() => setRefreshing(false)); }, []); return ( }> Pull down to see RefreshControl indicator ); } ```
KenKluskens commented 2 years ago

@jakub-gonet Is there a way to use your workaround with a FlatList? or is the only option for a FlatList to switch back to the react-native version ?

jakub-gonet commented 2 years ago

You can use renderScrollComponent prop to pass ScrollView with those props set. Or you can patch-package RNGH to apply this patch from master and use waitFor with ref directly on FlatList.

anabeatrizzz commented 2 years ago

@vivekjm Error is gone after following what you wrote, but content inside ScrollView is not shown as @pke mentioned.

EDIT: I noticed that when I import a component that returns RefreshControl, the content of ScrollView is not shown, but when I use RefreshControl directly with ScrollView it works as expected (Android physical device).

1Jesper1 commented 2 years ago

The fix for me was to use the react-native ScrollView.

AlexSirenko commented 2 years ago

@jakub-gonet any updates on this? I tried to use onScoll from reanimated it doesn't work with refresh control.

kimkong88 commented 2 years ago

I still need update on this :(. so far I just made wrapper component

{Platform.OS === "android" ? ( <RNFlatList {...this.props} /> ) : ( <FlatList {...this.props} /> )} to support refresh on both ios and android.

a-eid commented 2 years ago

just wrapping RefreshControl with createNativeWrapper fixes the issue,

const RefreshControl = createNativeWrapper(RNRefreshControl);

<ScrollView refreshControl={<RefreshControl />}>
    {/* ... */}
</ScrollView>

-- embedding RefreshControl inside a component doesn't work thou. sadly..

const RefreshControl = createNativeWrapper(RNRefreshControl);

function CustomRefreshControl(){
  return <RefreshControl />
}

<ScrollView refreshControl={<CustomRefreshControl />}>
    {/* ... */}
</ScrollView>

I'm using version 2.1.0

tyrauber commented 2 years ago

This issue also exists when importing FlatList from react-native-gesture-handler and wrapping it in react-native-reanimated Animated.createAnimatedComponent. Wrapping RefreshControl in createNativeWrapper does not resolve the issue. The pull to refresh event fails to fire on Android as demonstrated in this Snack.

A work around is to import FlatList directly from react-native, not wrap the component in reanimated and set the Animated value manually on the FlatList onScroll event, but this is far less performant than using Animated.Event and ends up blocking the UI thread on slow devices.

AntonioLi1 commented 1 year ago

@janicduplessis, could please describe what is broken here (expected/current behavior)? I used this code as a repro example and it seems alright to me:

import React, { Component, useCallback } from 'react';
import { SafeAreaView, View, Text, RefreshControl } from 'react-native';

import { ScrollView } from 'react-native-gesture-handler';

const wait = timeout => {
  return new Promise(resolve => {
    setTimeout(resolve, timeout);
  });
};
export default function App() {
  const [refreshing, setRefreshing] = React.useState(false);

  const onRefresh = useCallback(() => {
    setRefreshing(true);

    wait(1500).then(() => setRefreshing(false));
  }, []);

  return (
    <SafeAreaView style={styles.container}>
      <ScrollView
        contentContainerStyle={styles.scrollView}
        refreshControl={
          <RefreshControl refreshing={refreshing} onRefresh={onRefresh} />
        }>
        <Text>Pull down to see RefreshControl indicator</Text>
      </ScrollView>
    </SafeAreaView>
  );
}

const styles = {
  container: {
    flex: 1,
  },
  scrollView: {
    flex: 1,
    backgroundColor: 'pink',
    alignItems: 'center',
    justifyContent: 'center',
  },
};

android-ios android-ios

EDIT: I was running this example on fresh RN 0.63.2 installation

I am running into the same issue with android but I used flatlist instead of scrollview. I also didn't import anything from RNGH. Did you manage to find a fix? If so, what did you change?

tsalama commented 1 year ago

@j-piasecki This issue seems to have caused a regression -- scrolling ScrollViews and FlatLists on Android now triggers nested TouchableOpacity onPresses whenever this new RNGH RefreshControl (or @jakub-gonet 's patch) is used

rajatrawataku1 commented 1 year ago

In my case the content is not being rendered. I am using a VirtualList and ScrollView in some places to implement PullToRefresh functionality. I am using memoized JSX and passing it to the refreshControl prop of ScrollView/VirtualList.

 const PullToRefreshJSx = useMemo(
    () => (
      <PullToRefresh
        refetchData={async () => {
          await refetchData(false);
        }}
      />
    ),
    [refetchData]
  );

// PullToRefresh Component //

export function PullToRefresh({
  refetchData,
}: {
  refetchData: () => Promise<void>;
}) {
  const Snackbar = useSnackBar();
  const [isRefreshing, setIsRefreshing] = useState(false);

  const onRefresh = useCallback(async () => {
    setIsRefreshing(true);

    try {
      await refetchData();
    } catch (err) {
      alertFromBeError(err, Snackbar);
    } finally {
      setIsRefreshing(false);
    }
  }, [refetchData, Snackbar]);

  return <RefreshControl refreshing={isRefreshing} onRefresh={onRefresh} />;
}

However when I am directly using the component like this, its working fine for me in Android Platform.

image

react-native-version:0.68.2

Antho2407 commented 1 year ago

I am experiencing the exact same issue as above. Having a wrapper that encapsulates the states and custom styles is preventing the rendering of children, only in Android. Trying to switch ScrollView / RefreshControl for the react-native ones did not help I am using RN 0.70.3 and react-native-gesture-handler 2.8.0

Antho2407 commented 1 year ago

In case this can help anybody using a custom wrapper, this fixed the issue for me :

return (
        <RefreshControlRN
            refreshing={isRefreshing}
            tintColor={tokensColors.warmWhite}
            // See https://github.com/facebook/react-native/issues/32144
            {...props}
            onRefresh={onRefresh}
        />
    );