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 issue with simultaneous keyboard dismiss and sheet close #1164

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paulrostorp
Copy link

Motivation

This aims to address #1159 and #1072

This simply exits the OnKeyboardStateChange callback if the sheet is about close.

This may conflict with a1ec74d but as I am unsure of the calendar for the v5 release, I thought I'd open a PR for this fix only. Please advise

@paulrostorp
Copy link
Author

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
@@ -1402,6 +1402,7 @@ const BottomSheetComponent = forwardRef<BottomSheet, BottomSheetProps>(
           (Platform.OS === 'android' &&
             keyboardBehavior === KEYBOARD_BEHAVIOR.interactive &&
             android_keyboardInputMode === KEYBOARD_INPUT_MODE.adjustResize)
+          || animatedNextPositionIndex.value == -1
         ) {
           animatedKeyboardHeightInContainer.value = 0;
           return;

@princefishthrower
Copy link

@paulrostorp while this patch did prevent my modal bottom sheet from staying / snapping back up to it's highest snap point after dismissing the keyboard, for bottom sheets with more than one snap point, I noticed that after the keyboard closed (by way of dragging the sheet down), my bottom sheet was stuck at it's first (non 0) snap point.

I believe this to be momentum-based since hard swipes down fully close the modal, but if you swipe too lightly, the sheet gets stuck in this weird halfway position...

I could only fully close the modal by doing something like this:

useTimeout(() => bottomSheetRef.current?.dismiss(), keyboardShown ? null : 10);

so far I've only tested on android; my bottom sheet modal has the following (relevant) properties:

keyboardBehavior="extend"
keyboardBlurBehavior="restore"
android_keyboardInputMode="adjustPan"

@github-actions
Copy link

github-actions bot commented Dec 8, 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

@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

@bombillazo
Copy link

@gorhom any feedback on this PR?

@gorhom
Copy link
Owner

gorhom commented Jan 12, 2023

Will look into this pr this weekend

@felixaa
Copy link

felixaa commented Jan 26, 2023

Tested the Discord fork and it worked for us 🎊

@bombillazo
Copy link

@gorhom any chance for a recent review?

@github-actions
Copy link

github-actions bot commented Mar 3, 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.

@brahimmouhamou
Copy link

not stale, @gorhom ?

@github-actions
Copy link

github-actions bot commented Apr 9, 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

@PrimulaX
Copy link

No way to merge this? 😷

@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.

@PrimulaX
Copy link

no stale, gringo.

@bhupendra1011
Copy link

@paulrostorp , is there any update when this fix will be merged.

@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.

@PrimulaX
Copy link

🫠

@gorhom gorhom added v4 Written in Reanimated v2 v5 and removed no-pr-activity labels Jul 17, 2023
@gorhom gorhom self-assigned this Jul 17, 2023
@JulesVision
Copy link

Is the fix merged ?

@gorhom
Copy link
Owner

gorhom commented Aug 5, 2023

@paulrostorp could you provide the issue repro sample code.

@johannessachse
Copy link

@paulrostorp I tested this PR with Pixel 7. Not sure why but in my case this fix isn't working correctly.

This is my component:

        <BottomSheet
          index={bottomSheetIndex}
          ref={sheetRef}
          snapPoints={['70%']}
          enablePanDownToClose={true}
          handleComponent={null}
          backdropComponent={renderBackdrop}
        >
        {...}
        </BottomSheet>
  const close = () => {
    Keyboard.dismiss();
    setBottomSheetIndex(-1);
    sheetRef.current?.close();
  };

@fjmorant
Copy link

Any chance in making this work?, facing the same issue

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.

None yet