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

Using KeyboardAwareScrollView on BottomSheet #789

Open
xotahal opened this issue Jan 30, 2025 · 4 comments
Open

Using KeyboardAwareScrollView on BottomSheet #789

xotahal opened this issue Jan 30, 2025 · 4 comments
Assignees
Labels
KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component

Comments

@xotahal
Copy link

xotahal commented Jan 30, 2025

Describe the bug
There is a problem with calculating position of text input when used on bottom sheet component (@gorhom/bottom-sheet). Let me explain what's happening with these two videos.

When I fully open the bottom sheet then everything seems to be fine.

Screen.Recording.2025-01-30.at.3.51.02.PM.mov

When I make the bottom sheet smaller than keyboard it scrolls wrong. You can see when I press input 0, the bottom sheet itself extend to fully open position and that means the KeyboardAwareScrollView should do nothing, because eventually the 0 input is above the keyboard.

The same behaviour happening even after I hide keyboard and focus the same #0 input. Only after I change to 1 input it works fine and it also works fine if I focus 0 field after I focused 1 field.

Hope it makes sense.

Screen.Recording.2025-01-30.at.3.51.36.PM.mov

Code snippet
I am looking at the code and I think I know what the problem is. I am happy to fix it and prepare PR, but I would maybe appreciate some guidance so the PR meets expectations. I think there might be multiple problems with this kind of use case (including the PR I created already - #770 - I would maybe close the PR and solve it differently because if the view is not there is creates "hard" jump when hiding keyboard)

  • select #0 (when bottom sheet is smaller than keyboard)
  • hide keyboard
  • select #0 again (when bottom sheet is fully opened)

In this case, the onFocusedInputLayoutChanged is not called second time, because the focus input is the same. The problem is it still remembers the previous position of the input so it thinks it is behind the keyboard and it does the scrolling. Once I select #1 input and after 0 it gets the correct position on the screen.

  const inputLayoutHandler = useFocusedInputLayoutHandler(
    {
      onFocusedInputLayoutChanged: (e) => {
        "worklet";

        if (e.target !== -1) {
          layout.value = e;
        } else {
          layout.value = null;
        }
      },
    },
    [],
  );

What I think should happen is to check the current position of text input in all those functions maybeScroll, onStart, onMove, onEnd and based on that decide if we need to scroll. Because the absolute position of the text field is changing when keyboard is showing.

Another option would be to listen for the position of the current focused text field if possible and then check the current position in maybeScroll.

What do you think?

Repo for reproducing
I have this code setup where I use BottomSheet with BottomSheetTextInputs and this libraries' KeyboardAwareScrollView, which is just wrapped by createBottomSheetScrollableComponent.

      <KeyboardProvider>
        <BottomSheet
          ref={bottomSheetRef}
          onChange={handleSheetChanges}
          snapPoints={snapPoints}
          topInset={100}
        >
        <BottomSheetScrollView>
          {(new Array(15).fill(undefined)).map((_, index) => {
            return (
              <View key={index} style={{ backgroundColor: 'blue', margin: 8, borderBottomWidth: 2, paddingVertical: 16}}>
                <BottomSheetTextInput key={index} placeholder={`Input #${index}`} style={{ backgroundColor: 'yellow' }} />
              </View>
            )
          })}
        </BottomSheetScrollView>
        </BottomSheet>
      </KeyboardProvider>

To Reproduce
Explained above

Expected behavior
Well I think it should just do nothing actually :) Because TextInput will be eventually above the keyboard.

Screenshots
Uploaded above.

Smartphone (please complete the following information):

  • Desktop OS: [MacOS 15
  • Device: iPhone 16 pro simulator
  • OS: latest
  • RN version: 76
  • RN architecture: old
  • JS engine: jsc
  • Library version: latest

Please let me know if any questions. I appreciate the help.

@kirillzyusko
Copy link
Owner

Hello @xotahal

Sorry for being non-responsive on your PR - I wanted to prepare stable 1.16.0 release and then merge various fixes.

Regarding you problem - don't you think that default keyboard handling from gorhom/bottom-sheet simply conflicts with the keyboard handling provided by KeyboardAwareScrollView.

Let's consider step by step how it works in your second video.

1‍⃣ Bottom sheet is collapsed

When you focus input#0 - it's obviously that it located close to the bottom of the screen, so KeyboardAwareScrollView starts its scrolling. Additionally you expand your bottom-sheet and change the layout, so as a result position of input#0 becomes invalid.

One potential fix I'm thinking of is to disable KeyboardAwareScrollView until bottom-sheet is fully expanded?

2‍⃣ Focus again input#0

I think the reason, why it input get invisible is because KeyboardAwareScrollView somehow memoizes the input position when you open it the first time 🤔 As you wrote here:

the onFocusedInputLayoutChanged is not called second time, because the focus input is the same

I think it happens here:

// focus was changed
if (focusWasChanged) {
  tag.value = e.target;

  // save position of focused text input when keyboard starts to move
  layout.value = input.value;

The idea is that we refresh position of input before keyboard movement (i. e. in onStart). And then we work with memoized position to calculate scroll interpolation for each frame. However in your case the position gets changed, because bottom-sheet gets extended and thus coordinates of input are getting updated.

My assumption is that except the same input id/the same keyboard size we also should add a condition the same absoluteY position, so we should update layout.value = input.value; in case if absoluteY has been changed.

From the other side, I think it'll break current behavior (I'm not sure, but it can) - for example on Android, when keyboard gets closed we will not have a back-scroll animation.


One potential solution I've been thinking of is to disable KeyboardAwareScrollView until it gets expanded, and update layout.value only when the component gets enabled. But it'll produce issues like this: #414

So here is the problem with over-memoizing. Just need to figure out the safe way how to write new data to layout.value when we understand, that previous cache is incorrect 👀

Let me know what do you think 🙌

@kirillzyusko kirillzyusko added the KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component label Jan 30, 2025
@kirillzyusko
Copy link
Owner

The original idea with memoization was basically needed to implement a "back-scroll on keyboard hide" functionality - i. e. when you close the keyboard you expect, that the input will be returned to the same position.

When keyboard becomes hidden, then theoretically we should return "noFocusedInputEvent" event (however I'm not sure it's the case on Android, because on Android you can simple close the keyboard via "V" button in nav bar and focus will be still in your input).

And the idea with memoization of field was to have a coordinates before all layout shifts, so that when keyboard becomes invisible, we return all elements to its initial position (and for that we need to know that initial position, so we save it).

@kirillzyusko
Copy link
Owner

kirillzyusko commented Jan 30, 2025

Or maybe there is a simple fix - always invalidate layout.value = null whenever keyboard gets closed?

So the final fix consist from two parts:

  • disable KeyboardAwareScrollView until bottom-sheet fully expanded (in your code);
  • invalidate input.layout when keyboard gets closed (in the library code).

What do you think?

@kirillzyusko
Copy link
Owner

Aso you can reach out to me via discord (nickname is kiryl.ziusko) - I'm responding there much faster 👀
And we can discuss all problem there - it'll be easier and faster!

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

No branches or pull requests

2 participants