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

Fix android issues with dynamic snap points and open keyboard + onAnimate values #1163

Open
wants to merge 5 commits into
base: v4
Choose a base branch
from

Conversation

paulrostorp
Copy link

@paulrostorp paulrostorp commented Nov 1, 2022

Motivation

I was facing an issue on android when using dynamic snap points and the adjustPan setting when content height changed while the keyboard was open. I think the following video best showcases the problem:

Record_2022-10-31-12-56-43.mp4

as you can see, the height of the content changes when the validation text appears or disappears, thereby adding or removing about twenty pixels.

I've narrowed down the issue: because of the keyboard height, the appropriate snapPoint index is not found and triggers the intermediary collapse of the sheet.

I've tested this on android and ios:

  • with and without dynamic snap points setting
  • with single and multiple snap points
  • with modal/regular bottom sheet
  • with bottomsheet scrollview

I have not found this to cause any regressions, but there may be a better solution to this problem.

Edit:
Upon further investigation, I also found some issues with onAnimate being provided with the wrong values (both ios and android), I've fixed that in 0c7e153, but I'm starting to think my fixes are not tackling the root issue, @gorhom please advise 🙂

@paulrostorp
Copy link
Author

paulrostorp commented Nov 1, 2022

for any interested, here is a patch:

@gorhom+bottom-sheet+4.4.5.patch

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 b166440..ce74512 100644
--- a/node_modules/@gorhom/bottom-sheet/src/components/bottomSheet/BottomSheet.tsx
+++ b/node_modules/@gorhom/bottom-sheet/src/components/bottomSheet/BottomSheet.tsx
@@ -693,9 +693,9 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
         /**
          * store next position
          */
-        animatedNextPosition.value = position;
+        animatedNextPosition.value = position + animatedKeyboardHeight.value;
         animatedNextPositionIndex.value =
-          animatedSnapPoints.value.indexOf(position);
+          animatedSnapPoints.value.indexOf(position + animatedKeyboardHeight.value);

const keyboardOffset = animatedKeyboardState.value == KEYBOARD_STATE.SHOWN ? animatedKeyboardHeight.value : 0;

animatedNextPosition.value = position + keyboardOffset;
animatedNextPositionIndex.value = position == 0 ? INITIAL_SNAP_POINT : animatedSnapPoints.value.indexOf(position + keyboardOffset);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When the position was 0 (from a fully extended sheet), the index was still specified as -1. But that had the implication that the sheet was closed/closing.
I thought to instead set it to INITIAL_SNAP_POINT (-999), as that conveys a better meaning, especially in the context of the onAnimate callback

@paulrostorp paulrostorp changed the title Fix android issues with dynamic snap points and open keyboard Fix android issues with dynamic snap points and open keyboard + onAnimate values Nov 1, 2022
@github-actions
Copy link

github-actions bot commented Dec 2, 2022

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@paulrostorp
Copy link
Author

not stale cc @gorhom

paulrostorp added a commit to paulrostorp/react-native-bottom-sheet that referenced this pull request Jan 6, 2023
@github-actions
Copy link

github-actions bot commented Jan 7, 2023

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@paulrostorp
Copy link
Author

not stale cc @gorhom

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@paulrostorp
Copy link
Author

not stale cc @gorhom

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@paulrostorp
Copy link
Author

Not stale

@github-actions
Copy link

This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 10 days.

@github-actions github-actions bot closed this Apr 28, 2023
@gorhom gorhom self-assigned this Apr 30, 2023
@gorhom gorhom added v4 Written in Reanimated v2 v5 labels Apr 30, 2023
@gorhom gorhom reopened this Apr 30, 2023
@gorhom
Copy link
Owner

gorhom commented Apr 30, 2023

Hi @paulrostorp, thanks for investigating and submitting this pr, could you please provide a reproducible sample code to test out the issue and validate the proposed solution ? thanks

@XantreDev
Copy link

Probably it's related with this
#1821

@idrakimuhamad
Copy link

idrakimuhamad commented May 17, 2024

I think it may not be Android only as I'm having this on my app for both platform. It happened when the content size of the sheet changed. In my case, I have a helper text that changed its content depending on the input validation. I think what happened here is its trying to recalculate the sheet size since I've enabled dynamic sizing. The current workaround is to set a fixed height to the content.

Simulator.Screen.Recording.-.iPhone.15.Pro.-.2024-05-17.at.22.05.13.mp4

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
v4 Written in Reanimated v2 v5
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants