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

Incorrect KeyboardAvoidingView calculations on Android #334

Closed
joeporpeglia opened this issue Jan 16, 2024 · 8 comments · Fixed by #468
Closed

Incorrect KeyboardAvoidingView calculations on Android #334

joeporpeglia opened this issue Jan 16, 2024 · 8 comments · Fixed by #468
Assignees
Labels
🤖 android Android specific 🐛 bug Something isn't working 📚 components Anything related to the exported components of this library KeyboardAvoidingView 🧪 Anything related to KeyboardAvoidingView component

Comments

@joeporpeglia
Copy link

KeyboardAvoidingView relies on useWindowDimensions to calculate the bottom offset:

const { height: screenHeight } = useWindowDimensions();
const relativeKeyboardHeight = useCallback(() => {
"worklet";
const keyboardY =
screenHeight - keyboard.heightWhenOpened.value - keyboardVerticalOffset;
return Math.max(frame.value.y + frame.value.height - keyboardY, 0);
}, [screenHeight, keyboardVerticalOffset]);

However, useWindowDimensions doesn't account for translucent system UI (e.g. status bar) on all devices/emulators (see discussion in facebook/react-native#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 kirillzyusko added 🐛 bug Something isn't working 🤖 android Android specific labels Jan 17, 2024
@kirillzyusko
Copy link
Owner

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
Copy link
Author

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

Unexpected behavior on Pixel 5 device (extra bottom padding)

Screenshot (Jan 17, 2024 11_49_33 AM)

@kirillzyusko
Copy link
Owner

@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
Copy link
Author

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

  • in render, console.log("useWindowDimensions", useWindowDimensions().height)
  • in Container's onLayout, console.log("onLayout", e.nativeEvent.layout.height)

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 kirillzyusko added the 📚 components Anything related to the exported components of this library label Jan 17, 2024
@kirillzyusko
Copy link
Owner

@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
Copy link
Owner

@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
Copy link
Author

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 facebook/react-native#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 kirillzyusko added the KeyboardAvoidingView 🧪 Anything related to KeyboardAvoidingView component label May 29, 2024
@kirillzyusko kirillzyusko linked a pull request Jun 12, 2024 that will close this issue
2 tasks
kirillzyusko added a commit that referenced this issue Jun 13, 2024
## 📜 Description

Implemented own `useWindowDimensions` hook for Android.

## 💡 Motivation and Context

The problem with default implementation of `useWindowDimensions` on
Android is that it doesn't work well with `edge-to-edge` mode and you
can not retrieve an actual size of the screen. Here is a brief
comparison of values captured on my device (Pixel 7 Pro).

Translucent `StatusBar`:

```sh
height: 867.4285888671875 <- own hook
height: 867.4285888671875, y: 0 <- useSafeAreaFrame
height: 891.4285714285714 <- Dimensions.get('screen')
height: 826.2857142857143 <- Dimensions.get('window')
```

Non translucent `StatusBar`:

```sh
height: 867.4285888671875 <- own hook
height: 826.2857055664062, y: 41.14285659790039 <- useSafeAreaFrame
height: 891.4285714285714 <- Dimensions.get('screen')
height: 826.2857142857143 <- Dimensions.get('window')
```

So as you can see it doesn't react properly on the case when `StatusBar`
is translucent and reports incorrect values, which later on causes
incorrect layout calculation in components like `KeyboardAvoidingView`
or `KeyboardAwareScrollView`.

Theoretically we could workaround this problem by original
`useWindowDimensions().height + StatusBar.currentHeight`, but everything
become trickier when we add translucent `navigationBar` (+ translucent
`statusBar`):

```sh
height: 891.4285888671875 <- own hook
height: 891.4285888671875, y: 0 <- useSafeAreaFrame
height: 891.4285714285714 <- Dimensions.get('screen')
height: 826.2857142857143 <- Dimensions.get('window')
```

In this case derived value `useWindowDimensions().height +
StatusBar.currentHeight` (867.4285888671875) still will produce
incorrect value and all calculations will be broken. So I decided to
create own version of the hook which will cover all the cases.

Issue for reference:
facebook/react-native#41918

Closes
#434
#334

## 📢 Changelog

<!-- High level overview of important changes -->
<!-- For example: fixed status bar manipulation; added new types
declarations; -->
<!-- If your changes don't affect one of platform/language below - then
remove this platform/language -->

### JS

- export own `useWindowDimensions` hook;
- started to use own `useWindowDimensions` in `KeyboardAwareScrollView`
and `KeyboardAvoidingView` components;
- added mock for `useWindowDimensions` hook as default RN
implementation;

### Android

- added `WindowDimensionsListener` class;
- added `ThemedReactContext.content` extension;
- added `ThemedReactContext.setupWindowDimensionListener` extension;

## 🤔 How Has This Been Tested?

Tested manually on Pixel 7 Pro (API 34).

Tested on CI via e2e (API 28).

## 📸 Screenshots (if appropriate):

Pixel 7 Pro (Android 14), `KeyboardAwareScrollView`:

### KeyboardAwareScrollView

|Before|After|
|-------|-----|

|![telegram-cloud-photo-size-2-5429580266013318443-y](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/4874f962-2726-4cd0-ba96-4b57c076b6f5)|![telegram-cloud-photo-size-2-5429580266013318444-y](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/28a3b276-f8b7-40fa-a17d-bcc4e58b28b7)|

### KeyboardAvoidingView

|Initial|Before|After|
|------|------|-----|

|![telegram-cloud-photo-size-2-5429580266013318470-y](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/570d0092-c846-4005-97b0-c596169b91f8)|![telegram-cloud-photo-size-2-5429580266013318469-y](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/8909fee6-aa42-45a3-87b6-65c59831c703)|![telegram-cloud-photo-size-2-5429580266013318471-y](https://github.com/kirillzyusko/react-native-keyboard-controller/assets/22820318/e64c5dfd-cdc1-4cf5-b731-3a25d3a2fde3)|

## 📝 Checklist

- [x] CI successfully passed
- [x] I added new mocks and corresponding unit-tests if library API was
changed
@kirillzyusko
Copy link
Owner

Should be fixed in 1.12.3 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🤖 android Android specific 🐛 bug Something isn't working 📚 components Anything related to the exported components of this library KeyboardAvoidingView 🧪 Anything related to KeyboardAvoidingView component
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants