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

[v4] BottomSheet close (while dismissing keyboard) snaps to intermediate position [Android only] #1072

Closed
bombillazo opened this issue Aug 22, 2022 · 17 comments
Labels
bug Something isn't working no-issue-activity

Comments

@bombillazo
Copy link

bombillazo commented Aug 22, 2022

Bug

Currently on Android, when programmatically closing an open BottomSheet at the same time the Keyboard is open (not using a swipe down gesture or dragging), the closing animations start but the BottomSheet closes to where the Keyboard edge was when open, not the bottom of the screen. This happens when you have a TextInput inside the BottomSheet that opens the Keyboard and you have a close button that closes both.

I've tried with close, forceClose, snapToIndex(-1) or snapToPosition(0) functions when using Keyboard.dismiss(). It works as expected on IOS but for Android calling the close command will not move the sheet all the way to the true screen bottom.

Environment info

Library Version
@gorhom/bottom-sheet ^4
react-native 0.69.4
react-native-reanimated 2.9.1
react-native-gesture-handler 2.5.0

Steps To Reproduce

  1. Open the BottomSheet
  2. Click on TextInput, wait for keyboard to pop up
  3. Click on close button inside bottomsheet
  4. The sheet will not fully close, instead stop partially down close to where the keyboard was open.

It seems the calculation for closing the BottomSheet on Android is using the screen height based on where the keyboard top edge ends and not the bottom edge of the screen. I need the close position to be the screen bottom end.

Describe what you expected to happen:

  1. I expect the bottomSheet to go completely to position 0, so the edge of the screen.

Reproducible sample code

const App = () => {
  // refs
  const bottomSheetRef = useRef(null);

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

  // callbacks
  const handleExpandPress = useCallback(() => {
    bottomSheetRef.current?.expand();
  }, []);
  const handleCollapsePress = useCallback(() => {
    bottomSheetRef.current?.collapse();
  }, []);
  const handleClosePress = useCallback(() => {
    bottomSheetRef.current?.close();
    Keyboard.dismiss();
  }, []);

  return (
    <View style={styles.container}>
      <Button onPress={handleExpandPress} title="Expand" />
      <Button onPress={handleCollapsePress} title="Collapse" />
      <BottomSheet
        ref={bottomSheetRef}
        snapPoints={snapPoints}>
          <BottomSheetScrollView
            alwaysBounceVertical={false}
            keyboardShouldPersistTaps="handled">
            <View>
              <Button onPress={handleClosePress} title="Close" />
            </View>
            <List type="FlatList" />
            <BottomSheetTextInput />
          </BottomSheetScrollView>
      </BottomSheet>
    </View>
  );
};

const styles = StyleSheet.create({
  container: {
    flex: 1,
    paddingVertical: 64,
    justifyContent: 'flex-start',
    backgroundColor: '#222',
  },
});

export default App;

Couldn't get the repo to work though.

image

@bombillazo bombillazo added the bug Something isn't working label Aug 22, 2022
@terrysahaidak
Copy link

Having the same issue.
We use the bottom sheet modal with react-navigation.
When I press "enter" on the keyboard - the callback to hide the modal will be executed after the keyboard hiding animation so it works.
But when I programmatically dismiss the modal and the keyboard - it would just stuck.

The current solution is to wait for the keyboard to hide and then call .close() on the bottom sheet's ref.

@bombillazo
Copy link
Author

Hey @terrysahaidak , how do you programmatically wait for the keyboard to hide to trigger the sheet close when done? I was trying to do that for Android as a hack but it always triggers simultaneously.

@gorhom Is there any way to specify/force the on close position to be relative to the screen dimension and not the dimensions of the screen - keyboard height? I think something along that is the culprit of this bug.

@bombillazo bombillazo changed the title [v4] Closing BottomSheet while dismissin keyboard snaps to intermediate position [Android only] [v4] BottomSheet close (while dismissing keyboard) snaps to intermediate position [Android only] Aug 22, 2022
@bombillazo
Copy link
Author

bombillazo commented Aug 22, 2022

This is the current behavior btw:

Working (IOS)

Screen.Recording.2022-08-22.at.1.47.23.PM.mov

Not Working (Android)

Screen.Recording.2022-08-22.at.1.46.00.PM.mov

@terrysahaidak
Copy link

Here is my workaround, not elegant, but seems to be working.
Make sure you use adjustPan or adjustResize for the screen otherwise the Keyboard event won't fire.
Also, for some reason, just subscribing for the didHide even in place didn't work for me.

https://github.com/rainbow-me/rainbow/pull/4055/files#diff-c12279017bcb91d44b49db027b1dee6a128d3993182f2c174a06fbf08d56aba2R126-R135

@bombillazo
Copy link
Author

Thanks for sharing, really appreciate it.

@bombillazo
Copy link
Author

bombillazo commented Aug 23, 2022

@gorhom I think I found the root cause of the differing behavior based on the platform:

In component BottomSheetContainer, function handleContainerLayout uses e.nativeEvent.layout to indirectly set the animatedContainerHeigth value used by the snap/close functions in the BottomSheet component. On Android, onLayout is called when the keyboard opens/closes and the height is recalculated to the Window - Keyboard height. This does not occur on ios:

# console log of values in handleContainerLayout callback
# android
height: 791.272705078125, WINDOW_HEIGHT: 791.2727272727273
height: 510.18182373046875, WINDOW_HEIGHT: 791.2727272727273 <---- changes when keyboard opens
height: 791.272705078125, WINDOW_HEIGHT: 791.2727272727273 <---- changes when keyboard closes

# ios
height: 844, WINDOW_HEIGHT: 844 <---- never changes when the keyboard opens

This leads to inconsistent behaviors across platforms to anything related to animatedContainerHeight and keyboard usage.

So a design decision must be made to fix this. Either Android shouldn't trigger the callback on keyboard open/close (or use WINDOW_HEIGHT instead of layout height) to match the behavior of ios (this would fix the original bug presented in this Issue) or somehow trigger the onLayout callback when the ios keyboard opens/closes (maybe add a listener to the ios keyboard?) so that the animatedContainerHeight is recalculated to the new height without the keyboard. The latter would in theory copy the current "buggy" behavior in android to ios so additional logic would be required so that the close callbacks can use the real container height position sans keyboard height to fully close the bottom sheet when the keyboard is dismissed.

I don't know the implications of these changes, but it's a start on getting this fixed.

** UPDATE **

The issue is contingent on having the Android keyboard in 'adjustResize' mode and not 'adjustPan'.

@terrysahaidak
Copy link

@bombillazo i tried both adjust resize and adjust pan and both are broken for me.
Also, my fix seems to be working only "sometimes" since for some reason, on some phones keyboardDidHide event won't fire.
For now, I'm gonna just use setTimeout to trigger the close on the sheet after Keyboard.dismiss() call.

@terrysahaidak
Copy link

terrysahaidak commented Aug 23, 2022

Seems like I had a typo where I was calling setAdjustPan(); and if I call Keyboard.dismiss() before closing the modal - it just works.

So basically when you navigate to this modal - make sure you use adjust pan.

@SkylerB1
Copy link

SkylerB1 commented Sep 5, 2022

@gorhom I am also facing this issue in version 4.4.3. Any solution for this?

@mihir0x69
Copy link

Facing this issue for 2.x. Here's the solution that worked for me (Android) -

const BottomSheetWrapper = () => {
  const [isVisible, setIsVisible] = useState(true);

  useEffect(() => {
    const keyboardDidShowListener = Keyboard.addListener('keyboardDidShow', () => {
      setIsVisible(false);
    });

    const keyboardDidHideListener = Keyboard.addListener('keyboardDidHide', () => {
      setIsVisible(true);
    });

    return () => {
      keyboardDidShowListener.remove();
      keyboardDidHideListener.remove();
    };

  }, []);

  if (!isVisible) {
    return null;
  }

  return (
    <BottomSheet />
  )
}

Hope this helps.

@github-actions
Copy link

github-actions bot commented Oct 7, 2022

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.

@github-actions
Copy link

This issue was closed because it has been stalled for 5 days with no activity.

@paulrostorp
Copy link

This is also affecting ios

@paulrostorp
Copy link

For any interested, I have a patch here while the PR is reviewed: #1164 (comment)

Please let me know if you run into any issue 😄

@johannessachse
Copy link

johannessachse commented Aug 24, 2023

@bombillazo i tried both adjust resize and adjust pan and both are broken for me. Also, my fix seems to be working only "sometimes" since for some reason, on some phones keyboardDidHide event won't fire. For now, I'm gonna just use setTimeout to trigger the close on the sheet after Keyboard.dismiss() call.

@terrysahaidak Thanks for sharing your workaround. Setting a timeout of 500ms on Android before closing the sheet works for me too.

In my case changing android_keyboardInputMode made no difference (tested on Pixel 7)

@serkan-bayram
Copy link

Setting a timeout only sometimes work. This bug is still happening.

@leighrubenff
Copy link

This is still an issue

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

No branches or pull requests

8 participants