kirillzyusko / react-native-keyboard-controller

Keyboard manager which works in identical way on both iOS and Android
https://kirillzyusko.github.io/react-native-keyboard-controller/
MIT License
1.72k stars 74 forks source link

Incorrect KeyboardAvoidingView calculations on Android #334

Closed joeporpeglia closed 4 months ago

joeporpeglia commented 9 months ago

KeyboardAvoidingView relies on useWindowDimensions to calculate the bottom offset: https://github.com/kirillzyusko/react-native-keyboard-controller/blob/cb715fe1e4541a5df38e73bf9cc4930aa4ee5238/src/components/KeyboardAvoidingView/index.tsx#L69-L78

However, useWindowDimensions doesn't account for translucent system UI (e.g. status bar) on all devices/emulators (see discussion in https://github.com/facebook/react-native/issues/33735).

This causes inconsistent bottom offsets on certain android devices (I've observed this on a Pixel 5 device, but not on a Pixel 5 emulator). I've worked around this issue by implementing a simpler keyboard avoiding view that directly applies useReanimatedKeyboardAnimation().height as bottom padding to an animated view.

Is there any reason to calculate the bottom offset as "relative keyboard height" instead of using the current keyboard height directly?

kirillzyusko commented 9 months ago

Hello @joeporpeglia

Thank you for your issue!

Is there any reason to calculate the bottom offset as "relative keyboard height" instead of using the current keyboard height directly?

This is how original KeyboardAvoidingView was working so I just followed the same approach. I think from UI perspective it's better to rely on "relative keyboard height".

Let's consider a case when you have a view in the top part of the screen and keyboard appears (let's say that in this case only 10px of view will be obscured by keyboard). If you push the view to the entire keyboard height (let's say 336), then a significant part of the view will be obscured by window traits (i. e. status bar) 😓 So I think "relative keyboard height" is a preferred choice in this case.

If you clone example app - does the problem happen as well? (If yes, then it explains why I've used keyboardVerticalOffset property - I also have Pixel 7 Pro device and I think it should have the same problem, right?)

joeporpeglia commented 9 months ago

Thank you for the explanation @kirillzyusko!

I tweaked the example app to reproduce the issue and I can confirm that the bottom padding is inconsistent between android devices. See https://github.com/joeporpeglia/react-native-keyboard-controller/tree/keyboard-avoiding-view-repro (Changes to demonstrate the issue in this commit)

Correct behavior on Pixel 5 emulator ![image](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/1399969/84cc6d27-c5b5-4635-9cbf-4945b806e91e)
Unexpected behavior on Pixel 5 device (extra bottom padding) ![Screenshot (Jan 17, 2024 11_49_33 AM)](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/1399969/062db042-f378-46c3-8e79-0250b929a50f)
kirillzyusko commented 9 months ago

@joeporpeglia and what is the output if you log a height from useWindowDimensions on these two devices? Just theoretically they are should be equal?

Also in your screenshots I see a difference in elements positioning:

Emulator Real device
image image

Maybe you have some ideas how to fix this problem, by the way? I checked FB code - they are sending screen coordinates when keyboard is open/closed (but I thought it's an overkill because we can calculate in on JS).

joeporpeglia commented 9 months ago

Using the following logs in example/src/screens/Examples/KeyboardAvoidingView:

Device output:

 LOG  useWindowDimensions 777.4545454545455
 LOG  onLayout 826.9091186523438

Emulator output:

 LOG  useWindowDimensions 802.9090909090909
 LOG  onLayout 802.9091186523438

Also in your screenshots I see a difference in elements positioning:

That gap to the left is due to the notch on my device, but you're right it does seem like the emulator uses a different system UI than the physical device.

Maybe you have some ideas how to fix this problem, by the way?

I've been using a custom keyboard avoiding view that only uses the keyboard height as bottom padding. This fixes the issue because it avoids useWindowDimensions, but it doesn't support vertical offset like you mentioned. I haven't had a need for that behavior yet, so the workaround component has solved for now.

kirillzyusko commented 9 months ago

@joeporpeglia okay, I got you. I'll try to have a look into the issue when I get more free time 👀

One solution I was thinking of is the usage Dimensions.get("screen") API, but I think it may not work properly in somce cases as well 🤔

I need to play around with the example that you provided on my device to understand where the calculations are wrong and how to fix them 👍

kirillzyusko commented 6 months ago

@joeporpeglia As a potential workaround you can try to use keyboardVerticalOffset={-StatusBar.currentHeight} 👀

In a meantime I'll think what is the best way to solve the problem 🤔

joeporpeglia commented 6 months ago

Hey @kirillzyusko, the custom KeyboardAvoidingView I mentioned is still covering all of our use cases, but I'll make a note to use this workaround if we need the vertical offset behavior.

On a related note, the react-native issue I originally linked to was closed, so I created a new one with a repro here https://github.com/facebook/react-native/issues/41918. Seems like this would impact any library relying on that behavior, so ideally we can get it fixed upstream in react-native. That issue hasn't received much traction though.

kirillzyusko commented 4 months ago

Should be fixed in 1.12.3 🤞