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

[v4] When BottomSheetFlatList is scrolling, and BottomSheet is panning down, then BottomSheetFlatList scrolls to top #1939

Open
theobouwman opened this issue Sep 16, 2024 · 10 comments
Assignees
Labels
bug Something isn't working v5

Comments

@theobouwman
Copy link

Bug

When BottomSheetFlatList is scrolling, and BottomSheet is panning down, then BottomSheetFlatList scrolls to top.
Here a screen recording:

RPReplay_Final1726515224.MP4

Environment info

Library Version
@gorhom/bottom-sheet ^4.5.1
react-native 0.70.14
react-native-reanimated ^2.10.0
react-native-gesture-handler ~2.5.0

Steps To Reproduce

  1. Have a BottomSheetFlatList with enough items so you can scroll.
  2. Fling the list so it scrolls to the bottom.
  3. Then drag the sheet down.
  4. You now see that the BottomSheetFlatList has scrolled al the way to the top.

Describe what you expected to happen:

  1. We use this setup for an infinite feed and expect it just stay where you left it after scrolling.

Reproducible sample code

https://snack.expo.dev/bbA39-KvJDVDDnjqWk1. with example code.
When we use FlashList (https://shopify.github.io/flash-list/docs/) we don't have this jump-to-top bug. But we lose a lot of integration with BottomSheet. And scrolling does not work with FlashList in BottomSheet on Android. So not a good solution.

@theobouwman theobouwman added the bug Something isn't working label Sep 16, 2024
@gorhom gorhom self-assigned this Oct 2, 2024
@gorhom gorhom added the v5 label Oct 2, 2024
@gorhom
Copy link
Owner

gorhom commented Oct 2, 2024

thanks @theobouwman for reporting this issue, it should be fixed in the next release for v5

@gorhom
Copy link
Owner

gorhom commented Oct 2, 2024

Simulator.Screen.Recording.-.iPhone.15.Pro.Max.-.2024-10-02.at.22.40.04.mp4

@oe-bayram
Copy link

The fix only addresses vertical lists and doesn’t resolve the issue for horizontal lists.

@hirbod
Copy link
Contributor

hirbod commented Oct 26, 2024

@oe-bayram should be fairly easy to update the locking function for horizontal support.

@hirbod
Copy link
Contributor

hirbod commented Oct 26, 2024

@theobouwman FYI, v5 exports BottomSheetFlashList, which works very good, so you should be good to migrate.

@oe-bayram
Copy link

@hirbod Thanks for the suggestion! Just to clarify, would you recommend using a custom scrollEventsHandlersHook to implement horizontal locking support? I'm not entirely sure I understood your hint correctly.

@Aerilym
Copy link

Aerilym commented Oct 30, 2024

This seems to be the same issue I'm having with a horizontal BottomSheetFlashList component inside a BottomSheet, I've played around with a load of different props with no luck. from what @hirbod said it sounds like the solution is to do the same thing as the vertical fix but for horizontal. Is that something anyone is looking at doing or should I have a go?

Here's the issue:

Screen_Recording_20241030_212743_Expo_Go.mp4

This is the code at that state (tried with directionalLockEnabled enabled and disabled:

      <BottomSheet
        ref={bottomSheetRef}
        snapPoints={[130, '60%']}
        enableDynamicSizing={false}
        index={0}
        style={{
          paddingStart: 8,
        }}
      >
        {placeBasic ?
          <BottomSheetView
            style={{width: '100%', flexDirection: 'row', justifyContent: 'space-between', alignItems: 'center'}}>
            <Text style={{ fontSize: 22 }}>{placeBasic.name.replaceAll('\n', ' ')}</Text>
          </BottomSheetView> : null}
        {placeBasic ? <BottomSheetFlashList
          data={["Save", "Share", "Events", "Edit", "Directions", "More"]}
          horizontal
          keyExtractor={(item) => item}
          renderItem={(data) => <View style={{marginVertical: 4, marginHorizontal: 2}}><CustomButton title={data.item}/></View>}
          estimatedItemSize={24}
        /> : null}
        {placeBasic ? placeComplexLoading ? <TextLoading/> : placeComplex?.photos?.length ?
          <BottomSheetFlashList
            data={data}
            horizontal
            keyExtractor={(data) => data.name}
            renderItem={(data) => {
              const size = 240
              return <View style={{height: size, width: size, marginHorizontal: 4}}>
                <Image source={{
                  uri: `https://places.googleapis.com/v1/${data.item.name}/media?maxHeightPx=${size}&maxWidthPx=${size}&key=${apiKey}`,
                }} style={{height: size, width: size, borderRadius: 10}} width={size} height={size}
                       resizeMode="cover"/>
              </View>
            }}
            estimatedItemSize={240}
          />
          : null : null}
        {placeBasic ? placeComplexLoading? <TextLoading/> : placeComplex?.formattedAddress ?
          <BottomSheetView
            style={{flexDirection: 'row', justifyContent: 'space-between', alignItems: 'center', paddingEnd:8 }}>
            <Text style={{fontSize: 16, flexWrap:"wrap"}}>{placeComplex.formattedAddress}</Text>
            <CustomButton title=">"/>
          </BottomSheetView>
          : null:null}
      </BottomSheet>

@hirbod
Copy link
Contributor

hirbod commented Oct 30, 2024

Yes, the code I've seen only locks and take care for the Y axis and does nothing for X.
https://github.com/gorhom/react-native-bottom-sheet/blob/master/src/hooks/useScrollEventsHandlersDefault.ts

I think adding the same for X will fix it.

@mrmuke
Copy link

mrmuke commented Nov 24, 2024

Oh great, so is this fixed in an upcoming update? I also need horizontal scrolling to work.

@YasinAkimura
Copy link

YasinAkimura commented Dec 1, 2024

Yes, the code I've seen only locks and take care for the Y axis and does nothing for X. https://github.com/gorhom/react-native-bottom-sheet/blob/master/src/hooks/useScrollEventsHandlersDefault.ts

I think adding the same for X will fix it.

possible fix

ScrollView from react-native-gesture-handler supports disallowInterruption which solves the issue of fighting handlers for a proper solution we can wrap any List component by createNativeWrapper of the react-native-gesture-handler legacy api and use the horizontal and disallowInterruption props

I was able to use it to fix that issue on android as in ios this was never an issue because the first handler always wins.

  • FlatList and ScrollView supported do not use BottomSheet components for horizontal scrolling lists as these should not care about X axis for our smooth experience

const MyFlatList = createNativeWrapper(FlatList);
<MyFlatList horizontal disallowInterruption {...props} />

  • TypeSafe
    const FlatList = createNativeWrapper(NFlatList) as unknown as <TItem>( props: FlatListProps<TItem> & Readonly<NativeViewGestureHandlerProps>, ) => React.ReactElement;

gotcha

this somehow does not work with FlashList as the component above doesn't even allow it to initiate scrolling

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v5
Projects
None yet
Development

No branches or pull requests

7 participants