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] | Footer height not included in dynamic size with scroll view #1725

Closed
smikheiev opened this issue Feb 2, 2024 · 10 comments
Closed

[v4] | Footer height not included in dynamic size with scroll view #1725

smikheiev opened this issue Feb 2, 2024 · 10 comments
Labels
bug Something isn't working

Comments

@smikheiev
Copy link

smikheiev commented Feb 2, 2024

Bug

I have a bottom sheet with a scroll view inside, and with the enableDynamicSizing prop set. Dynamic size calculation seems to work fine - the bottom sheet height is adjusted to the scroll view content height.
But when I add a footer with footerComponent prop, the bottom sheet height is not changed. Footer adds a bottom margin so the content can be scrolled above it, but doesn't change the bottom sheet height.
I'm not sure if this is an expected behavior or not. If it is - is there a way to include footer height in the bottom sheet dynamic height calculation?

Environment info

Library Version
@gorhom/bottom-sheet 4.6.0
react-native 0.71.15
react-native-reanimated 3.6.1
react-native-gesture-handler 2.9.0

Steps To Reproduce

  1. Try to run the code below with footerComponent={Footer} and without it (remove or comment it out)
  2. See that the bottom sheet height doesn't change
with footer without footer
with-footer.mp4
without-footer.mp4

Reproducible sample code

const App = () => {
  const sheetRef = useRef<BottomSheet>(null)

  const data = useMemo(
    () =>
      Array(15)
        .fill(0)
        .map((_, index) => `index-${index}`),
    []
  )

  const handleOpenPress = useCallback(() => {
    sheetRef.current?.expand()
  }, [])
  const handleClosePress = useCallback(() => {
    sheetRef.current?.close()
  }, [])

  const renderItem = useCallback(
    (item) => (
      <View key={item} style={styles.itemContainer}>
        <Text>{item}</Text>
      </View>
    ),
    []
  )
  return (
    <View style={styles.container}>
      <Button title="Open" onPress={handleOpenPress} />
      <Button title="Close" onPress={handleClosePress} />
      <BottomSheet
        //
        enableDynamicSizing
        enablePanDownToClose
        footerComponent={Footer}
        ref={sheetRef}
      >
        <BottomSheetScrollView enableFooterMarginAdjustment>{data.map(renderItem)}</BottomSheetScrollView>
      </BottomSheet>
    </View>
  )
}

const Footer = ({ animatedFooterPosition }: BottomSheetFooterProps) => {
  return (
    <BottomSheetFooter animatedFooterPosition={animatedFooterPosition}>
      <View style={styles.footer}>
        <Text>Some footer</Text>
      </View>
    </BottomSheetFooter>
  )
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
  itemContainer: {
    padding: 6,
    margin: 6,
    backgroundColor: '#eee',
  },
  footer: {
    backgroundColor: 'pink',
    height: 60,
    justifyContent: 'center',
    alignItems: 'center',
  },
})
@smikheiev smikheiev added the bug Something isn't working label Feb 2, 2024
Copy link

github-actions bot commented Feb 2, 2024

@smikheiev: hello! 👋

This issue is being automatically closed because it does not follow the issue template.

@darshan09200
Copy link

I have a temporary solution

Steps to fix the issue

  1. Create a patches folder if you don't have one already.
  2. Create a file @gorhom+bottom-sheet+5.0.0-alpha.6.patch

I am using @gorhom+bottom-sheet@5.0.0-alpha.6 for my project, if you are using some different version update the files accordingly and generate a new patch using npx patch-package @gorhom/bottom-sheet

  1. Paste the following
diff --git a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetScrollable/createBottomSheetScrollableComponent.tsx b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetScrollable/createBottomSheetScrollableComponent.tsx
index 9162b66..52bd563 100644
--- a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetScrollable/createBottomSheetScrollableComponent.tsx
+++ b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetScrollable/createBottomSheetScrollableComponent.tsx
@@ -4,7 +4,7 @@ import React, {
   useImperativeHandle,
   useMemo,
 } from 'react';
-import { useAnimatedProps, useAnimatedStyle } from 'react-native-reanimated';
+import { useAnimatedProps, useAnimatedReaction, useAnimatedStyle } from 'react-native-reanimated';
 import { Gesture } from 'react-native-gesture-handler';
 import { BottomSheetDraggableContext } from '../../contexts/gesture';
 import {
@@ -87,11 +87,17 @@ export function createBottomSheetScrollableComponent<T, P>(
     );
     //#endregion

+    useAnimatedReaction(() => {
+      return animatedFooterHeight.value
+    }, (curr, prev) => {
+      animatedContentHeight.value += curr - (prev || 0);
+    })
+
     //#region callbacks
     const handleContentSizeChange = useStableCallback(
       (contentWidth: number, contentHeight: number) => {
         if (enableDynamicSizing) {
-          animatedContentHeight.value = contentHeight;
+          animatedContentHeight.value = contentHeight + animatedFooterHeight.value;
         }

         if (onContentSizeChange) {
  1. Run npx patch-package

Documentation for patch-package for any other

Warning

This solution might not work if your footer changes layout on state update, in my case it was static so this solution did the trick.

@idrakimuhamad
Copy link

I also notice this. So if you use flat list fore example, some of the element would be behind the footer, instead of the sheet grow bigger.

@gorhom
Copy link
Owner

gorhom commented Feb 18, 2024

this should be fixed on the next release

@SergioAN007
Copy link

SergioAN007 commented Feb 20, 2024

Unfortunally don't work without this:

+    useAnimatedReaction(() => {
+      return animatedFooterHeight.value
+    }, (curr, prev) => {
+      animatedContentHeight.value += curr - (prev || 0);
+    })

"@gorhom/bottom-sheet": "^5.0.0-alpha.7",

@idrakimuhamad
Copy link

In my case also with latest 4.6.1 the footer height are still not being included. Do i need to wrap my footer with BottomSheetFooter? Or any flag to offset the height?

@weihanyau
Copy link

Unfortunally don't work without this:

+    useAnimatedReaction(() => {
+      return animatedFooterHeight.value
+    }, (curr, prev) => {
+      animatedContentHeight.value += curr - (prev || 0);
+    })

"@gorhom/bottom-sheet": "^5.0.0-alpha.7",

Same on 4.6.1 too, it seems like the fix uses the default value of animatedFooterHeight which is 0 instead of using the height when the footer is mounted

@weihanyau
Copy link

weihanyau commented Mar 7, 2024

@weihanyau @SergioAN007 nope guys, just add enableFooterMarginAdjustment={true} for BottomSheetScrollView whatever.
Don't apply this patch — it won't work for maxDynamicContentSize case.

Yeap, I knew about the flag, but the flag does nothing, played around with the source code for a while and nothing fix it except adding the useAnimatedReaction, works for maxDynamicContentSize perfectly too. Will do more testing on it

@buhny
Copy link

buhny commented Mar 11, 2024

Upgraded to 4.6.1 & still having this issue too.

@hssrrw
Copy link

hssrrw commented Mar 22, 2024

@gorhom I wonder, why enableFooterMarginAdjustment is the prop in BottomSheetView and Scrollables, and not in the BottomSheet itself. I would treat the footer height the same way as the handle height. But I'm not sure how to provide enableFooterMarginAdjustment to useNormalizedSnapPoints to enable the same conditional behavior.

diff --git a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheet/BottomSheet.tsx b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheet/BottomSheet.tsx
index f20e3dc..021c1f8 100644
--- a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheet/BottomSheet.tsx
+++ b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheet/BottomSheet.tsx
@@ -204,6 +204,7 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
       animatedContainerHeight,
       animatedContentHeight,
       animatedHandleHeight,
+      animatedFooterHeight,
       enableDynamicSizing,
       maxDynamicContentSize
     );
diff --git a/node_modules/@gorhom/bottom-sheet/src/hooks/useNormalizedSnapPoints.ts b/node_modules/@gorhom/bottom-sheet/src/hooks/useNormalizedSnapPoints.ts
index 872b2f8..65d23e0 100644
--- a/node_modules/@gorhom/bottom-sheet/src/hooks/useNormalizedSnapPoints.ts
+++ b/node_modules/@gorhom/bottom-sheet/src/hooks/useNormalizedSnapPoints.ts
@@ -14,6 +14,7 @@ import {
  * @param containerHeight BottomSheetContainer height.
  * @param contentHeight content size.
  * @param handleHeight handle size.
+ * @param footerHeight footer size.
  * @param enableDynamicSizing
  * @param maxDynamicContentSize
  * @returns {Animated.SharedValue<number[]>}
@@ -23,6 +24,7 @@ export const useNormalizedSnapPoints = (
   containerHeight: Animated.SharedValue<number>,
   contentHeight: Animated.SharedValue<number>,
   handleHeight: Animated.SharedValue<number>,
+  footerHeight: Animated.SharedValue<number>,
   enableDynamicSizing: BottomSheetProps['enableDynamicSizing'],
   maxDynamicContentSize: BottomSheetProps['maxDynamicContentSize']
 ) => {
@@ -56,7 +58,7 @@ export const useNormalizedSnapPoints = (
       _normalizedSnapPoints.push(
         containerHeight.value -
           Math.min(
-            contentHeight.value + handleHeight.value,
+            contentHeight.value + handleHeight.value + footerHeight.value,
             maxDynamicContentSize !== undefined
               ? maxDynamicContentSize
               : containerHeight.value

This was referenced Sep 8, 2024
This was referenced Sep 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

8 participants