fix(regression): updated keyboard handling reaction (by @yusufyildirim)

[yusufyildirim]

This PR fixes #946.

Motivation

Looks like a regression is introduced with the PR #931. With that PR, getKeyboardHeightInContainer renamed to animatedKeyboardHeightInContainer and started using useDerivedValue instead of useWorkletCallback which made it a SharedValue instead of a function to fix another bug. Yet introduced #946, a tricky bug. Here's the screen recording:

Screen.Recording.2022-06-09.at.13.12.09.mov

Here's where I placed the console.logs to debug:

• Input focused: https://github.com/yusufyildirim/react-native-bottom-sheet/blob/fix/keyboard-handling/src/components/bottomSheetTextInput/BottomSheetTextInput.tsx#L17
• Keyboard Height Changed: https://github.com/yusufyildirim/react-native-bottom-sheet/blob/fix/keyboard-handling/src/hooks/useKeyboard.ts#L55
• Handle Interactive Behavior: https://github.com/yusufyildirim/react-native-bottom-sheet/blob/fix/keyboard-handling/src/components/bottomSheet/BottomSheet.tsx#L589
• Keyboard state change detected: https://github.com/yusufyildirim/react-native-bottom-sheet/blob/fix/keyboard-handling/src/components/bottomSheet/BottomSheet.tsx#L1438
• animatedKeyboardHeightInContainer recalculated: https://github.com/yusufyildirim/react-native-bottom-sheet/blob/fix/keyboard-handling/src/components/bottomSheet/BottomSheet.tsx#L309

Why it's happening?

There's OnKeyboardStateChange reaction that reacts to keyboard height and state changes to push the bottom sheet up when necessary. It calls getNextPosition function (right here) to understand where the bottom sheet should be placed so it can animate it to that position. So far so good. getNextPosition function depends on animatedKeyboardHeightInContainer value --used to be getKeyboardHeightInContainer-- make the calculation. animatedKeyboardHeightInContainer derives itself from animatedKeyboardHeight which is pretty much the keyboard height value. This is where the bug is.

If you check the screen recording above carefully, you'll see on the second line that the keyboard height value is immediately set. But yet, on the third line, animatedKeyboardHeightInContainer value is 0 when it's accessed within the getNextPosition function. I said animatedKeyboardHeightInContainer is derived from the keyboard height, so "how's this possible?" you might ask. The answer is on the fifth line, animatedKeyboardHeightInContainer value is being recalculated after OnKeyboardStateChange reaction and getNextPosition function are called.

It used to be not a problem because getNextPosition was using getKeyboardHeightInContainer which was a synchronous function call. getKeyboardHeightInContainer was accessing the latest keyboard height value which was already set at the time it's called. Since it's replaced with animatedKeyboardHeightInContainer, a derived state, whenever we access it, we get the latest memoized result. If it's not recalculated before we call it, we don't actually get the latest state.

To fix this problem, I added animatedKeyboardHeightInContainer to the OnKeyboardStateChange reaction as an input hoping it'll be re-triggered even if animatedKeyboardHeightInContainer calculation occurs later like it happens in our case. But, while I was waiting for a re-trigger, it never happened. animatedKeyboardHeightInContainer started to be calculated before, all the time, which is good. I believe reanimated running some sort of dependency analysis algorithm that started to understand it should recalculate animatedKeyboardHeightInContainer before calling the OnKeyboardStateChange.

Quite a story for a one-line change but I think it is super important to explain. I hope it helps!

[gorhom]

@yusufyildirim thanks for diving deep investigating this bug, and it is a regression ( my bad 🤦‍♂️ ). However your propose solution will trigger the reaction when the derived value changes, this may cause duplicate calls and other issues later on.

Thanks for your investigation, i have converted the animatedKeyboardHeightInContainer to a shared value that will be set on the keyboard height reaction, so its value will be updated instantly before executing any reaction.

@yusufyildirim could you please test my latest commit and confirm if it fixes the initial issue, thanks !

[yusufyildirim]

However your propose solution will trigger the reaction when the derived value changes, this may cause duplicate calls and other issues later on.

I did test that, wouldn't propose it as a solution if it was the case 😄 That's what I was trying to explain by saying;

But, while I was waiting for a re-trigger, it never happened. animatedKeyboardHeightInContainer started to be calculated before, all the time, which is good. I believe reanimated running some sort of dependency analysis algorithm that started to understand it should recalculate animatedKeyboardHeightInContainer before calling the OnKeyboardStateChange.

It doesn't mean it won't happen, though. I'll check your commit when I find some time. Thanks!

[yusufyildirim]

@gorhom Tested. It works!

[gorhom]

thanks @yusufyildirim !

[fa11erX]

Not Work corretly for me :(

[3runoDesign]

Not Work corretly for me :(