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

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

Closed
callaars opened this issue Apr 27, 2023 · 27 comments
Closed

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

callaars opened this issue Apr 27, 2023 · 27 comments
Assignees
Labels
bug Something isn't working v5

Comments

@callaars
Copy link

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

@callaars callaars added the bug Something isn't working label Apr 27, 2023
@callaars callaars changed the title [v4] Android keyboard issue in combination with Reanimated 3 [v5]|[v4] Android keyboard issue in combination with Reanimated 3 Apr 27, 2023
@callaars callaars changed the title [v5]|[v4] Android keyboard issue in combination with Reanimated 3 [v5] [v4] Android keyboard issue in combination with Reanimated 3 Apr 27, 2023
@github-actions
Copy link

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
Copy link
Author

Not stale.

@larsmunkholm
Copy link

larsmunkholm commented Jun 13, 2023

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

@callaars
Copy link
Author

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
Copy link

varshith5c commented Jun 15, 2023

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

@larsmunkholm
Copy link

@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
Copy link
Author

callaars commented Jun 15, 2023

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

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

@AkmalFairuz
Copy link

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

bottom.sheet.bug.mp4

@emilioschepis
Copy link

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
Copy link

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
Copy link

Paul12pp commented Jul 22, 2023

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

@callaars
Copy link
Author

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
Copy link

cheloveq commented Aug 7, 2023

Having exactly the same issue

@SickanK
Copy link

SickanK commented Aug 22, 2023

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

emilioschepis added a commit to emilioschepis/react-native-bottom-sheet that referenced this issue Aug 28, 2023
Using Reanimated 3, this resize animation with duration 0
causes the bottom sheet modal to disappear.

Refs: gorhom#1356
@emilioschepis
Copy link

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.

@BubbleTrouble14
Copy link

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

@BubbleTrouble14
Copy link

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

@sondreluc
Copy link

sondreluc commented Sep 19, 2023

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 #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
Copy link

sondreluc commented Sep 20, 2023

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 (#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
Copy link

@gorhom could you take a look at this please?

Copy link

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
Copy link
Author

callaars commented Nov 20, 2023

Not stale.

@gorhom
Copy link
Owner

gorhom commented Dec 26, 2023

@callaars could you test this pr.

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

@gorhom gorhom closed this as completed Jan 1, 2024
@johnnywang
Copy link

@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
Copy link

ybentz commented Apr 2, 2024

@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
Copy link

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

Android.keyboard.issue.mov

@Rc85
Copy link

Rc85 commented Apr 10, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working v5
Projects
None yet
Development

Successfully merging a pull request may close this issue.