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

Fix issue with simultaneous keyboard dismiss and sheet close

Open paulrostorp opened this issue 2 years ago • 24 comments

Motivation

This aims to address https://github.com/gorhom/react-native-bottom-sheet/issues/1159 and https://github.com/gorhom/react-native-bottom-sheet/issues/1072

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

This may conflict with https://github.com/gorhom/react-native-bottom-sheet/commit/a1ec74dbbc85476bb39f3637e9a97214e0cad9a0 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 avatar Nov 01 '22 09:11 paulrostorp

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;

paulrostorp avatar Nov 01 '22 09:11 paulrostorp

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

princefishthrower avatar Nov 07 '22 22:11 princefishthrower

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[bot] avatar Dec 08 '22 09:12 github-actions[bot]

not stale

paulrostorp avatar Dec 12 '22 15:12 paulrostorp

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[bot] avatar Jan 12 '23 09:01 github-actions[bot]

not stale

paulrostorp avatar Jan 12 '23 10:01 paulrostorp

@gorhom any feedback on this PR?

bombillazo avatar Jan 12 '23 14:01 bombillazo

Will look into this pr this weekend

gorhom avatar Jan 12 '23 14:01 gorhom

Tested the Discord fork and it worked for us 🎊

felixaa avatar Jan 26 '23 09:01 felixaa

@gorhom any chance for a recent review?

bombillazo avatar Feb 01 '23 01:02 bombillazo

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[bot] avatar Mar 03 '23 09:03 github-actions[bot]

not stale, @gorhom ?

brahimmouhamou avatar Mar 08 '23 22:03 brahimmouhamou

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[bot] avatar Apr 09 '23 09:04 github-actions[bot]

Not stale

paulrostorp avatar Apr 10 '23 06:04 paulrostorp

No way to merge this? 😷

PrimulaX avatar Apr 27 '23 09:04 PrimulaX

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[bot] avatar May 27 '23 09:05 github-actions[bot]

no stale, gringo.

PrimulaX avatar May 27 '23 09:05 PrimulaX

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

bhupendra1011 avatar Jun 16 '23 12:06 bhupendra1011

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[bot] avatar Jul 17 '23 09:07 github-actions[bot]

🫠

PrimulaX avatar Jul 17 '23 09:07 PrimulaX

Is the fix merged ?

JulesVision avatar Aug 03 '23 15:08 JulesVision

@paulrostorp could you provide the issue repro sample code.

gorhom avatar Aug 05 '23 17:08 gorhom

@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();
  };

johannessachse avatar Aug 24 '23 11:08 johannessachse

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

fjmorant avatar May 14 '24 11:05 fjmorant