Skip to content

Commit

Permalink
fix: don't react to snap point changes if height is 0 (#855)(by @simo…
Browse files Browse the repository at this point in the history
…n-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.
  • Loading branch information
simon-abbott authored Sep 9, 2022
1 parent 1e99e8d commit 29af238
Showing 1 changed file with 2 additions and 1 deletion.
3 changes: 2 additions & 1 deletion src/components/bottomSheet/BottomSheet.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,8 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
if (
JSON.stringify(snapPoints) === JSON.stringify(_previousSnapPoints) ||
!isLayoutCalculated.value ||
!isAnimatedOnMount.value
!isAnimatedOnMount.value ||
containerHeight <= 0
) {
return;
}
Expand Down

0 comments on commit 29af238

Please sign in to comment.