From 29af238d9eed31f0d9cad39ade8a43cf37ca2e72 Mon Sep 17 00:00:00 2001 From: Simon Abbott Date: Fri, 9 Sep 2022 15:18:31 -0500 Subject: [PATCH] fix: don't react to snap point changes if height is 0 (#855)(by @simon-abbott) In React Native 65 the 'OnSnapPointChange' reaction sometimes fires when a view is going out of sight. Specifically it fires *after* the `animatedContainerHeight` has been set to `0`. This causes the computed snap points to all be `0` themselves, which, in turn, causes the `index` calculation to select the last item in the snap point array. All of this is not really an issue except that if the view then comes *back* into view without being recreated (like if a user presses 'back' on a navigation stack) the index is preserved when rendering the recomputed now-non-zero snap points, so the sheet appears to snap to the fully open position. Here is an example series of events: 1. The BottomSheet is created with defined snap points `["25%", "75%"]` and index `0`. 2. The view loads with a height of `1000`. 3. The snap points are correctly calculated to be `[750, 250]` 4. The sheet is rendered at height `750` 5. The view is dismissed by navigating to another screen 6. The `animatedContainerHeight` is calculated to be `0` 7. The snap points are calculated to be `[0, 0]` 8. The `OnSnapPointChange` reaction is fired, which tries to animate to the nearest snap point, which is at position `0`. Since both snap points are `0` it picks the last one, so `index` becomes `1`, and position is set to `0`. 9. The navigation stack is popped, and the original View is displayed again. 10. The `animatedContainerHeight` is once again calculated to be `1000` 11. The snap points are again calculated to be `[750, 250]`. 12. The `OnSnapPointChange` reaction is fired, which again animates to the nearest snap point. Since step 8 set the position to `0`, the nearest snap point is always the last one, which in this case has index `1` (height = `250`), and renders as fully open. This patch fixes this issue by only running the `OnSnapPointChange` reaction when `animatedContainerHeight > 0`. If the height is zero then the internal state of the sheet does not matter since it won't be rendered anyway. With this change the aforementioned issue is resolved. Specifically, step 8 is skipped which means that step 12 runs against the last _valid_ position, and the sheet stays where it should. --- src/components/bottomSheet/BottomSheet.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/components/bottomSheet/BottomSheet.tsx b/src/components/bottomSheet/BottomSheet.tsx index 095d2a98a..b16644070 100644 --- a/src/components/bottomSheet/BottomSheet.tsx +++ b/src/components/bottomSheet/BottomSheet.tsx @@ -1294,7 +1294,8 @@ const BottomSheetComponent = forwardRef( if ( JSON.stringify(snapPoints) === JSON.stringify(_previousSnapPoints) || !isLayoutCalculated.value || - !isAnimatedOnMount.value + !isAnimatedOnMount.value || + containerHeight <= 0 ) { return; }