react-native-bottom-sheet icon indicating copy to clipboard operation
react-native-bottom-sheet copied to clipboard

[v5-alpha] Web - animatedIndex changes by itself and causes the BottomSheet to open up on navigating back to it

Open 5ZYSZ3K opened this issue 1 year ago • 7 comments

Bug

On web, when your BottomSheet did render, and then you navigated away from it, the _animatedContainerHeight property is set to 0. It causes animatedIndex variable to be 0 - and so, when you navigate back to the screen with the BottomSheet, it opens up immediately.

Environment info

Library Version
@gorhom/bottom-sheet 5.0.0-alpha.11
react-native 0.74.3
react-native-reanimated 3.10.1
react-native-gesture-handler 2.16.1

Steps To Reproduce

  1. Open up an app on web
  2. Navigate to the screen with BottomSheet
  3. Navigate away from it
  4. Navigate back to it - BottomSheet will pop up immediately (animatedIndex changed to 0)

Reproducible sample code

Here is a demo repo: https://github.com/5ZYSZ3K/bottom-sheet-web-issue-demo On web, navigate away from the screen with BottomSheet, and then back to it - it will pop up immediately

5ZYSZ3K avatar Aug 07 '24 08:08 5ZYSZ3K

Here is a diff that fixed the issue:

diff --git a/node_modules/@gorhom/bottom-sheet/lib/commonjs/components/bottomSheetContainer/BottomSheetContainer.js b/node_modules/@gorhom/bottom-sheet/lib/commonjs/components/bottomSheetContainer/BottomSheetContainer.js
index 07cbe0a..6c23503 100644
--- a/node_modules/@gorhom/bottom-sheet/lib/commonjs/components/bottomSheetContainer/BottomSheetContainer.js
+++ b/node_modules/@gorhom/bottom-sheet/lib/commonjs/components/bottomSheetContainer/BottomSheetContainer.js
@@ -9,6 +9,7 @@ var _reactNative = require("react-native");
 var _constants = require("../../constants");
 var _utilities = require("../../utilities");
 var _styles = require("./styles");
+const { INITIAL_CONTAINER_HEIGHT } = require("../bottomSheet/constants");
 function _getRequireWildcardCache(e) { if ("function" != typeof WeakMap) return null; var r = new WeakMap(), t = new WeakMap(); return (_getRequireWildcardCache = function (e) { return e ? t : r; })(e); }
 function _interopRequireWildcard(e, r) { if (!r && e && e.__esModule) return e; if (null === e || "object" != typeof e && "function" != typeof e) return { default: e }; var t = _getRequireWildcardCache(r); if (t && t.has(e)) return t.get(e); var n = { __proto__: null }, a = Object.defineProperty && Object.getOwnPropertyDescriptor; for (var u in e) if ("default" !== u && Object.prototype.hasOwnProperty.call(e, u)) { var i = a ? Object.getOwnPropertyDescriptor(e, u) : null; i && (i.get || i.set) ? Object.defineProperty(n, u, i) : n[u] = e[u]; } return n.default = e, t && t.set(e, n), n; }
 function BottomSheetContainerComponent({
@@ -38,7 +39,7 @@ function BottomSheetContainerComponent({
       }
     }
   }) {
-    containerHeight.value = height;
+    containerHeight.value = height || INITIAL_CONTAINER_HEIGHT;
     containerRef.current?.measure((_x, _y, _width, _height, _pageX, pageY) => {
       if (!containerOffset.value) return;
       containerOffset.value = {
diff --git a/node_modules/@gorhom/bottom-sheet/lib/module/components/bottomSheetContainer/BottomSheetContainer.js b/node_modules/@gorhom/bottom-sheet/lib/module/components/bottomSheetContainer/BottomSheetContainer.js
index 26fcae4..5aaffaa 100644
--- a/node_modules/@gorhom/bottom-sheet/lib/module/components/bottomSheetContainer/BottomSheetContainer.js
+++ b/node_modules/@gorhom/bottom-sheet/lib/module/components/bottomSheetContainer/BottomSheetContainer.js
@@ -3,6 +3,7 @@ import { StatusBar, View } from 'react-native';
 import { WINDOW_HEIGHT } from '../../constants';
 import { print } from '../../utilities';
 import { styles } from './styles';
+import { INITIAL_CONTAINER_HEIGHT } from '../bottomSheet/constants';
 function BottomSheetContainerComponent({
   containerHeight,
   containerOffset,
@@ -30,7 +31,7 @@ function BottomSheetContainerComponent({
       }
     }
   }) {
-    containerHeight.value = height;
+    containerHeight.value = height || INITIAL_CONTAINER_HEIGHT;
     containerRef.current?.measure((_x, _y, _width, _height, _pageX, pageY) => {
       if (!containerOffset.value) return;
       containerOffset.value = {
diff --git a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetContainer/BottomSheetContainer.tsx b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetContainer/BottomSheetContainer.tsx
index abca0a5..893671a 100644
--- a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetContainer/BottomSheetContainer.tsx
+++ b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheetContainer/BottomSheetContainer.tsx
@@ -10,6 +10,7 @@ import { WINDOW_HEIGHT } from '../../constants';
 import { print } from '../../utilities';
 import { styles } from './styles';
 import type { BottomSheetContainerProps } from './types';
+import { INITIAL_CONTAINER_HEIGHT } from '../bottomSheet/constants';
 
 function BottomSheetContainerComponent({
   containerHeight,
@@ -44,7 +45,7 @@ function BottomSheetContainerComponent({
         layout: { height },
       },
     }: LayoutChangeEvent) {
-      containerHeight.value = height;
+      containerHeight.value = height || INITIAL_CONTAINER_HEIGHT;
 
       containerRef.current?.measure(
         (_x, _y, _width, _height, _pageX, pageY) => {

The containerHeight is being calculated to 0, which causes a series of event that changes animatedIndex to 0. Setting it to INITIAL_CONTAINER_HEIGHT instead fixes the problem.

5ZYSZ3K avatar Aug 07 '24 08:08 5ZYSZ3K

https://github.com/user-attachments/assets/60682aa6-ba01-4598-9438-cdb4b4ac467f

tested your provided package, and i do not see the bottom sheet snapping .. but i noticed that you were using v4 instead of v5

gorhom avatar Aug 31 '24 16:08 gorhom

This is happening to me. I have version 5.0.6 and when I navigate away and then back, the sheet opens.

edwinvrgs avatar Feb 19 '25 19:02 edwinvrgs

Could clearly reproduce this with the current release version 5.1.2 and could also fix it with the exact fix @5ZYSZ3K proposed in the diff.

Also updated the demo repository to v5 to have an updated reproducible example. Will follow-up with a Pull Request in order to fix this.

InnigerM avatar Apr 17 '25 10:04 InnigerM

Pull Request created: https://github.com/gorhom/react-native-bottom-sheet/pull/2252

InnigerM avatar Apr 22 '25 05:04 InnigerM

@gorhom this one is making the bottom sheet unusable on web for cases where it starts out in closed state. i've checked out the demo repository & pull request above from InnigerM and they seem to be on point

bashbruno avatar May 15 '25 13:05 bashbruno

@gorhom We're seeing this on v5.1.1 with Expo SDK 52

johnjensenish avatar May 23 '25 19:05 johnjensenish