gorhom / react-native-bottom-sheet

A performant interactive bottom sheet with fully configurable options 🚀
https://gorhom.dev/react-native-bottom-sheet/
MIT License
7.1k stars 780 forks source link

[v5] [v4] Android keyboard issue in combination with Reanimated 3 #1356

Closed callaars closed 11 months ago

callaars commented 1 year ago

Bug

On Android, when using Reanimated 3 (any 3.x version) the BottomSheetModal immediately closes itself when it is of a relatively small height (<200) and focuses a BottomSheetTextInput. This does not happen when using a regular BottomSheet.

Incorrect: android

Correct (when using a larger height): android-big

Logs:

Incorrect:

[BottomSheetModal::handlePresent] 
[BottomSheet::render] animatedSnapPoints:781.3333129882812 animatedCurrentIndex:-1 providedIndex:0
[BottomSheetHandleContainer::handleContainerLayout] height:24
[fun::useAnimatedReaction::OnMount] isLayoutCalculated:true animatedSnapPoints:781.3333129882812 nextPosition:781.3333129882812
[fun::bound animateToPosition] currentPosition:1029.3333333333333 position:781.3333129882812 velocity:0
[BottomSheet::handleOnAnimate] toIndex:0 fromIndex:-1
[fun::bound animateToPositionCompleted] animatedCurrentIndex:-1 animatedNextPosition:781.3333129882812 animatedNextPositionIndex:0
[fun::useAnimatedReaction::OnChange] animatedCurrentIndex:-1 animatedIndex:0
[BottomSheet::handleOnChange] index:0 animatedCurrentIndex:0
[BottomSheetModal::handleBottomSheetOnChange] minimized:false forcedDismissed:false
[BottomSheetContainer::handleContainerLayout] height:669.3333129882812
[fun::useAnimatedReaction::OnSnapPointChange] snapPoints:469.33331298828125
[fun::bound animateToPosition] currentPosition:781.3333129882812 position:469.33331298828125 velocity:0
[BottomSheet::handleOnAnimate] toIndex:0 fromIndex:-1
[fun::bound animateToPositionCompleted] animatedCurrentIndex:0 animatedNextPosition:469.33331298828125 animatedNextPositionIndex:0
[fun::useAnimatedReaction::OnChange] animatedCurrentIndex:0 animatedIndex:-1
[BottomSheet::handleOnChange] index:-1 animatedCurrentIndex:-1
[BottomSheetModal::handleBottomSheetOnChange] minimized:false forcedDismissed:false
[fun::useAnimatedReaction::onClose] animatedCurrentIndex:-1 animatedIndex:-1
[BottomSheetModal::handleBottomSheetOnClose] minimized:false forcedDismissed:false
[BottomSheetModal::unmount] 
[BottomSheetModal::resetVariables] 
[BottomSheetModal::handlePortalOnUnmount] minimized:false forcedDismissed:false
[fun::useAnimatedReaction::OnChange] animatedCurrentIndex:-1 animatedIndex:0
[BottomSheet::handleOnChange] index:0 animatedCurrentIndex:0
[BottomSheetModal::handleBottomSheetOnChange] minimized:false forcedDismissed:false
[BottomSheetContainer::handleContainerLayout] height:981.3333129882812

Correct:

[BottomSheetModal::handlePresent] 
[BottomSheet::render] animatedSnapPoints:581.3333129882812 animatedCurrentIndex:-1 providedIndex:0
[BottomSheetHandleContainer::handleContainerLayout] height:24
[fun::useAnimatedReaction::OnMount] isLayoutCalculated:true animatedSnapPoints:581.3333129882812 nextPosition:581.3333129882812
[fun::bound animateToPosition] currentPosition:1029.3333333333333 position:581.3333129882812 velocity:0
[BottomSheet::handleOnAnimate] toIndex:0 fromIndex:-1
[fun::bound animateToPositionCompleted] animatedCurrentIndex:-1 animatedNextPosition:581.3333129882812 animatedNextPositionIndex:0
[fun::useAnimatedReaction::OnChange] animatedCurrentIndex:-1 animatedIndex:0
[BottomSheet::handleOnChange] index:0 animatedCurrentIndex:0
[BottomSheetModal::handleBottomSheetOnChange] minimized:false forcedDismissed:false
[BottomSheetContainer::handleContainerLayout] height:669.3333129882812
[fun::useAnimatedReaction::OnSnapPointChange] snapPoints:269.33331298828125
[fun::bound animateToPosition] currentPosition:581.3333129882812 position:269.33331298828125 velocity:0
[BottomSheet::handleOnAnimate] toIndex:0 fromIndex:0
[fun::bound animateToPositionCompleted] animatedCurrentIndex:0 animatedNextPosition:269.33331298828125 animatedNextPositionIndex:0

Environment info

Library Version
@gorhom/bottom-sheet 4.4.5
react-native 0.71.7
react-native-reanimated 3.1.0
react-native-gesture-handler 2.9.0

Steps To Reproduce

  1. Have a BottomSheetModal with a relative small height (<200) and a BottomSheetTextInput.
  2. Run the app on Android
  3. Try to focus the BottomSheetTextInput

Describe what you expected to happen:

When focussing the TextInput on a BottomSheetModal of any size I expect it not to close the bottom sheet.

Reproducible sample code

https://github.com/callaars/bottom-sheet-android-keyboard-issue

github-actions[bot] commented 1 year ago

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

callaars commented 1 year ago

Not stale.

larsmunkholm commented 1 year ago

I was experiencing this as well. Solved it by snapping the bottom sheet at 100%

callaars commented 1 year ago

Using adjustPan as the window resize mode seems to help in a lot of cases but still has similar issues with multiple modals. Not to mention that moving a whole app to work with adjustPan is just crazy talk.

I unfortunately cannot snap my bottom sheet to a 100%.

varshith5c commented 1 year ago

@larsmunkholm @callaars Is there a code example that I can see to solve this issue

larsmunkholm commented 1 year ago

@varshith5c This is a simplification, but essentially what I did:

export const Component = (props) => {
    const [inputHasFocus, setInputHasFocus] = useState(false);

    return (
        <BottomSheet
            snapPoints={[inputHasFocus ? "100%" : 200, "100%"]}
            onClose={props.onClose}
        >
            <TextInput
                onFocus={() => setInputHasFocus(true)}
                onBlur={() => setInputHasFocus(false)}
            />
        </BottomSheet>
    );
};
callaars commented 1 year ago

Yes I put the reproducible code in the top issue description.

https://github.com/callaars/bottom-sheet-android-keyboard-issue

AkmalFairuz commented 1 year ago

I can confirm this, when TextInput focused, onClose function on BottomSheet props will called

https://github.com/gorhom/react-native-bottom-sheet/assets/35138228/5a046a1c-6096-480b-81ed-a648fa71804a

emilioschepis commented 1 year ago

Hello, did anyone find a more stable solution to this issue? I started looking into it today and found a couple of interesting things.

The most important part is: I was able to get a definite value for how high the snap point must be before the whole sheet is closed. Turns out it is exactly the height of the expanded keyboard.

If the bottom sheet is even one pixel shorter than the keyboard, this condition momentarily evaluates to true and also this block of code is executed.

Unfortunately I am testing on a pretty complex project so there are a ton of moving parts, I plan on trying on a brand new project to hopefully find a suitable patch either for this library or reanimated itself.

With that said, I'd be curious to know if a more "official" solution is in the works.

TarikHuber commented 1 year ago

I am using BottomSheetModal and I could solve it just by adding a second snapPoint ['25%','100%‘] that is 100%. To make the modal behave more natural I added a snapToIndex(0) to a keyboardDidHide event:

useEffect(() => {
    const hideSubscription = Keyboard.addListener('keyboardDidHide', () => {
      bottomSheetModalRef.current?.snapToIndex(0);
    });

    return () => {
      hideSubscription.remove();
    };
  }, []);

Here is the whole component:

import React, {useCallback, useEffect, useMemo, useRef} from 'react';
import {BottomSheetModal, BottomSheetBackdrop} from '@gorhom/bottom-sheet';
import {Keyboard} from 'react-native';
import {BottomSheetDefaultBackdropProps} from '@gorhom/bottom-sheet/lib/typescript/components/bottomSheetBackdrop/types';
import {ModalsContext} from '../context/modals';

type Props = {
  children: React.ReactNode;
  name: string;
  height: string;
  modalProps?: any;
};

export const CustomModal: React.FC<Props> = ({
  children,
  name,
  height,
  modalProps,
}) => {
  const {isOpenModal, setOpenModal} = React.useContext(ModalsContext);

  const bottomSheetModalRef = useRef<BottomSheetModal>(null);

  useEffect(() => {
    const hideSubscription = Keyboard.addListener('keyboardDidHide', () => {
      bottomSheetModalRef.current?.snapToIndex(0);
    });

    return () => {
      hideSubscription.remove();
    };
  }, []);

  const open = useMemo(() => isOpenModal(name), [isOpenModal, name]);

  useEffect(() => {
    if (open) {
      bottomSheetModalRef.current?.present();
    } else {
      bottomSheetModalRef.current?.dismiss();
    }
  }, [open]);

  const snapPoints = useMemo(() => [height, '100%'], [height]);

  const renderBackdrop = useCallback(
    (
      props: React.JSX.IntrinsicAttributes & BottomSheetDefaultBackdropProps,
    ) => (
      <BottomSheetBackdrop
        {...props}
        disappearsOnIndex={-1}
        appearsOnIndex={0}
      />
    ),
    [],
  );

  return (
    <BottomSheetModal
      ref={bottomSheetModalRef}
      index={0}
      snapPoints={snapPoints}
      onChange={index => {
        if (index === -1) {
          setOpenModal(name, false);
        }
      }}
      backdropComponent={renderBackdrop}
      {...modalProps}>
      {children}
    </BottomSheetModal>
  );
};

It's a general component where the modal open states are managed through a context.

Paul12pp commented 1 year ago

any update about this issue? I'm having this problem in production.

callaars commented 1 year ago

For me the way to resolve this was to go to adjustPan system wide. Fortunately we only have very few problems converting to that but it isn't ideal.

cheloveq commented 1 year ago

Having exactly the same issue

SickanK commented 1 year ago

Having the same issue as well! @TarikHuber workaround works as a temporary fix! 😄

emilioschepis commented 1 year ago

I managed to reproduce this issue and sort of understand why this happens. Using a brand new React Native 0.71 project, which supports both Reanimated v2 and v3, I was able to determine that the issue only exists in v3, probably in withTiming.

I have opened a PR that adds a 1ms delay to the resize animation, which solves the issue both on my test project and real project. If anyone knows more about how withTiming or Reanimated in general work, we could create a PR to fix the root issue.

However, for now, the PR can be used as workaround.

ronickg commented 1 year ago

Can confirm got the same problem with reanimated 3 and v5.

ronickg commented 1 year ago

I swapped out android:windowSoftInputMode="adjustResize" to android:windowSoftInputMode="adjustPan" in the AndroidManifest.xml and now it seems to be working.

sondreluc commented 1 year ago

I've been spending a couple of days on this issue now, both trying to understand as much of the animation flow, and what triggers the different changes to all the internal values. What seems to be the culprit of the issue is that deriving of the animatedIndex value (here) is running earlier with Reanimated v3, than it does with v2. This earlier execution gives animatedIndex the value of -1 instead of 0, and cause the onChange and onClose handles to be triggered.

The execution flow I have observed is as follows:

With Reanimated v2:

  1. OnSnapPointsChange An animated reaction caused by change in containerHeight (container height reduces when keyboard appear).
  2. animateToPosition is triggered at the end of the OnSnapPointsChange reaction in 1., which performs an animation with configs.duration: 0, which changes animatedPosition from having the value of the old snapPoint position to the current snapPoint position and ending the animation right away, setting animatedAnimationState.value = ANIMATION_STATE.STOPPED;
  3. animatedIndex is derived here by interpolation based on the animaitedPosition generated in 2. The derived value ends up beeing 0.
  4. An onChange animatiedReaction is triggered by the change of animatedIndex and animatedPostition. The flow ends here, as none of the conditionals in this block resolves to true.

With Reanimated v3:

  1. The flow actually starts with the deriving of animatedIndex here. The derived value ends up beeing -1, since animatedPostition has not been updated yet.
  2. Step 1. and 2. from the Reanimated v2 flow executes.
  3. An onChange animatiedReaction is triggered by the change of animatedIndex and animatedPostition. The time it ends up triggering both the onChange and the onClose handles, as animatedIndex is -1 and animatedCurrentIndex is still 0.

I have not been able to understand why the flows are in different order with Reanimated v2 and v3. I have how ever understood why https://github.com/gorhom/react-native-bottom-sheet/pull/1497 by @emilioschepis works as a workaround. By changing the duration of the animation in animateToPosition from 0 to 1ms, the animation is stopped one frame later, leaving the animatedAnimationState.value = ANIMATION_STATE.RUNNING when the onChange animatiedReaction is triggered, causing an early exit here I can not say if that workaround is safe enough to be merged or not, as I do not have enough knowledge of the rest of the code. I suppose @gorhom is the best to decide, or finding out how to get the flow back to how it was with Reanimated v2 is best.

sondreluc commented 1 year ago

After some further investigation, I've come across this change in Reanimated, that is part of v3 and not v2. My understanding of this change is that they change the abstraction around Shared Values, and looking at the part of the code which I can understand, it looks like they changed from performing change detection in native code to js code. I suspect that change can have altered the timing of when useAnimatedReaction and/or useDerivedValue is triggered, in turn causing the different execution flows in this library.

I also came across a different issue (https://github.com/gorhom/react-native-bottom-sheet/issues/1416) which actually seems to be the same as the one we are experiencing here, just triggered by other actions than keyboard appearance, eg. window resizing on web and device rotation on mobile (not only on Android, but on iOS as well). I've published a reproduction repo where you can easily experience the issue, either by focusing the BottomSheetTextInput on Android, or by rotating the device while the sheet is open on iOS.

thespacemanatee commented 1 year ago

@gorhom could you take a look at this please?

github-actions[bot] commented 1 year ago

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

callaars commented 1 year ago

Not stale.

gorhom commented 11 months ago

@callaars could you test this pr.

thanks @sondreluc for diving into the code base and detail the root cause.

johnnywang commented 10 months ago

@gorhom I see that the above fix was merged and pushed out to v5, but is there a plan to push it out to v4 as well?

ybentz commented 7 months ago

@gorhom I'm still experiencing this on 5.0.0-alpha.9. I'm on Expo v50 in case that matters, I have a bottom sheet modal with a simple form with 2 inputs (BottomSheetTextInput), the entire form is not very tall but when I set the snapPoints to anything lower than 100% it seems to sometimes work and sometimes the modal closes when either of the inputs is focused and the keyboard opens. Most of the times the bottom sheet closes.

For now as a hopefully temporary workaround I'm gonna leave it at 100% but I'm really hoping to figure out a fix.

Any ideas why it doesn't work for me after the fix?

thomasgrivet commented 7 months ago

Hello, we have the same issue and like @ybentz, the fix pushed in v5 doesn't seems to work for us ; We are also using Expo 50

https://github.com/gorhom/react-native-bottom-sheet/assets/18561703/dee80d9a-1561-4953-83d1-7c6de7df4124

Rc85 commented 7 months ago

I find that if you add 100% to your snapPoints and then set keyboardBehavior to either extend or fillParent, it fixes it. But thenyou get the stupid extra padding that blocks/shrinks the content inside the modal.

The only workaround is setting my smallest snapPoints to 51%.