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: add ability to turn off adding extra padding for keyboard #770

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

Conversation

xotahal
Copy link

@xotahal xotahal commented Jan 21, 2025

📜 Description

Added an option to specify whether to include keyboard height padding. This is particularly useful for scenarios like bottom sheets, where the position of the modal is already managed relative to the keyboard, and additional padding is unnecessary.

💡 Motivation and Context

This change resolves the issue of redundant keyboard padding in components like bottom sheets, which already handle modal positioning relative to the keyboard. By providing an option to disable the keyboard height padding, developers have more control over the layout and can avoid unwanted spacing.

📢 Changelog

JS

  • Added a new property to control whether to add keyboard height padding.

iOS

  • No platform-specific changes.

Android

  • No platform-specific changes.

🤔 How Has This Been Tested?

Tested by integrating the new property with a bottom sheet component to ensure that keyboard height padding is correctly applied or excluded based on the property value. Checked the behavior on both iOS and Android devices.

📸 Screenshots (if appropriate):

Before:

Screen.Recording.2025-01-21.at.11.18.47.AM.mov

After:

Screen.Recording.2025-01-21.at.11.19.15.AM.mov

📝 Checklist

  • CI successfully passed
  • I added new mocks and corresponding unit-tests if library API was changed

@xotahal xotahal changed the title feat: add ability to turn off adding extra padding for keyboard fix: add ability to turn off adding extra padding for keyboard Jan 21, 2025
@kirillzyusko kirillzyusko self-assigned this Jan 21, 2025
@kirillzyusko kirillzyusko added the KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component label Jan 21, 2025
@kirillzyusko
Copy link
Owner

@xotahal I think it's not very clear from the example that you provided, but why KeyboardAwareScrollView is needed here?

KeyboardAwareScrollView is needed for large forms, and padding in the end is needed to be able to show the input above the keyboard (i. e. to scroll to the end and thus making TextInput to be above the keyboard). What is the point to use it in your example? In your example I believe you can use plain ScrollView and it should work?

@xotahal
Copy link
Author

xotahal commented Jan 22, 2025

Hi @kirillzyusko,

Thanks for the quick reply! I really appreciate your time and help 🙏

I’ve uploaded an example that will hopefully clarify the issue a bit better. I’m working with a bottom sheet that contains a “long” form and isn’t detached from the bottom edge. The bottom sheet essentially covers the whole screen, making it feel more like a regular screen with a long form.

Before:

Screen.Recording.2025-01-22.at.2.51.31.PM.mov

After:

Screen.Recording.2025-01-22.at.2.51.57.PM.mov

To make scrolling work smoothly within the bottom sheet, I’m relying on the scrollables provided by the library (https://gorhom.dev/react-native-bottom-sheet/scrollables). This approach works fine and is understandable. Following your suggestion, I’m also trying to use the KeyboardAwareScrollView in combination with the bottom sheet library (https://kirillzyusko.github.io/react-native-keyboard-controller/docs/api/components/keyboard-aware-scroll-view#gorhombottom-sheet).

First, let me say how much I like your library! It’s incredibly reliable and offers features that other libraries don’t, which I find essential for my use case.

That said, the challenge arises because the bottom sheet library also has its own way of handling keyboard avoidance. If I only rely on their approach, I run into some of the issues you’ve outlined in your docs (https://kirillzyusko.github.io/react-native-keyboard-controller/docs/api/components/keyboard-aware-scroll-view#comparison), and I lose access to some of the valuable features your library provides.

On the other hand, if I only use your KeyboardAwareScrollView, I run into a different problem. When the bottom sheet is smaller than the keyboard (for example, in compact states), your library adds padding to the scroll view, but the entire bottom sheet itself doesn’t move above the keyboard—it stays behind it. So in this scenario, I still need the bottom sheet library’s keyboard-handling logic to push the sheet above the keyboard.

See this example:

Screen.Recording.2025-01-22.at.3.24.35.PM.mov

Essentially, I need to combine the strengths of both libraries to get the desired behavior. However, this creates an issue with “double” padding, where both libraries add padding at the same time, which isn’t what I need.

Hopefully, this explanation makes at least a little sense. I truly appreciate all the hard work you’ve put into this library—it’s been a game-changer in so many ways! If you have any suggestions or ideas for tackling this situation, I’d be grateful for your guidance.

Thanks again!

@kirillzyusko
Copy link
Owner

On the other hand, if I only use your KeyboardAwareScrollView, I run into a different problem. When the bottom sheet is smaller than the keyboard (for example, in compact states), your library adds padding to the scroll view, but the entire bottom sheet itself doesn’t move above the keyboard—it stays behind it. So in this scenario, I still need the bottom sheet library’s keyboard-handling logic to push the sheet above the keyboard.

Interesting - KeyboardAwareScrollView should work here 🤔 Can you provide a code snippet so that I can put it into my example and check on my own?

Essentially, I need to combine the strengths of both libraries to get the desired behavior. However, this creates an issue with “double” padding, where both libraries add padding at the same time, which isn’t what I need.

Yeah, and my thinking is that bottom-sheet keyboard handling should be disabled 🙂 So that both libs don't conflict with each other. Really curious to investigate a case on your video why it doesn't work. Maybe because we have a container with a fixed height, so ScrollView doesn't grow because of that?

cc @hirbod as I know you were working with integrating bottom-sheet and KeyboardAwareScrollView, maybe you run into similar problems?

@hirbod
Copy link
Contributor

hirbod commented Jan 30, 2025

The thread is super long, so I didn't read it. What we done was to disable all the Keyboard handling that was shipped by Gorhom and manage everything with this library.

  // this moves the bottom sheet up when the keyboard is open
    const viewStyle = useAnimatedStyle(() => {
      return {
        transform: [{ translateY: shouldAdjustForKeyboard && isFocused ? height.value : 0 }],
      }
    })

    return (
        <AnimatedView className="flex-grow" style={viewStyle}>
          <GestureHandlerRootView className="flex-grow">
            <BottomSheet ....

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants