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

Keyboard height wrong with notches #26296

Closed
TuurDutoit opened this issue Sep 2, 2019 · 11 comments
Closed

Keyboard height wrong with notches #26296

TuurDutoit opened this issue Sep 2, 2019 · 11 comments
Labels
API: Keyboard Bug Stale There has been a lack of activity on this issue and it may be closed soon.

Comments

@TuurDutoit
Copy link

The height of the keyboard as reported by the keyboardDidShow event is wrong when a notch is present.

React Native version: 0.59.8 (Expo 34.0.0)
Tested on Android

Steps To Reproduce

  1. Listen for keyboardDidShow events
  2. Inspect the event details (note the height and screenY properties in particular)
  3. Compare these values to Dimensions.get('screen') and Dimensions.get('window')

Expected behaviour

The height property of the keyboardDidShow event reflects the correct height of the keyboard

Actual behaviour

The height that is returned is slightly smaller than the expected value: the difference seems to be the height of the notch. I assume that the screenY property (correctly) reflects the offset from the top of the screen (upper side of the notch), but the height is calculated using the window dimensions (everything under the notch). For phones that don't have a notch, the top of the screen and window are the same, so the height is correct. However, on phones with a notch, the top of the window is the bottom of the notch and as a result, the height is too small.

Example with actual numbers

Pixel 3 (no notch):

  • screen height: 785
  • window height: 737 (excludes on-screen system controls at the bottom of the screen, but includes status bar)
  • keyboard screenY: 447
  • keyboard height: 289

Pixel 3 XL (with notch):

  • screen height: 845
  • window height: 748 (excludes system controls and notch)
  • keyboard screenY: 515
  • keyboard height: 233

As you can see, screenY + keyboardHeight = windowHeight in both cases. As screenY is a screen coordinate, not window, this should be equal to screenHeight instead.

Snack: https://snack.expo.io/Sk_yXw9Sr

@TuurDutoit
Copy link
Author

I think this is related to this line, where getWindowDisplayMetrics() should be getScreenDisplayMetrics()

@MichalKrakow
Copy link

Is there a workaround for this?
Same issue on galaxy s10 +

@TuurDutoit
Copy link
Author

As the screenY property seems to be correct, you probably can by relying on that, but it's going to be hard. Depends on what you try to achieve though.

@MichalKrakow
Copy link

Thanks - this worked. Other thing to notice about Dimensions.get('window') is that when using gesture action bar on s10+ it returns wrong height value so instead of depending on this you're better of with container onLayout values.

@vandrieu
Copy link

vandrieu commented Oct 8, 2019

@MichalKrakow could you please help me understand how you fixed it? I can't figure this out.

@MichalKrakow
Copy link

@vandrieu
(im using expo 30.0)
Basically you cannot rely on returned keyboard height and dimensions.get('window').
So instead use container wrapping whole screen layout and get it's height on 'onLayout' event - this is your screen height. Then 'keyboardDidShow' event returns correct endCoordinates.screenY value. so you get the top edge of keyboard. Subtract those and you have keyboard height. Double check if your container gets whole sceen (statusbar etc...) cause if not you have to add this in order to get correct height.

@vandrieu
Copy link

vandrieu commented Oct 8, 2019

@MichalKrakow Got it working!

Before:

My App:

export default class App extends Component {
   render() {
      return (
         <MySwitchNavigator />
      )
   }
}

My component with keyboard:

const keyboardHeight = event.endCoordinates.height;

After:

My App:

export default class App extends Component {
   render() {
      return (
         <View
            style={{ flex: 1 }}
            onLayout={({ nativeEvent }) => MyConstants.realScreenHeight = nativeEvent.layout.height}
         >
         <MySwitchNavigator />
         </View>
      )
   }
}

My component with keyboard:

const keyboardHeight = MyConstants.realScreenHeight - event.endCoordinates.screenY;

Thanks!

@stale
Copy link

stale bot commented Jan 6, 2020

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 6, 2020
@TuurDutoit
Copy link
Author

This is still a bug in react-native AFAIK.

@stale stale bot removed the Stale There has been a lack of activity on this issue and it may be closed soon. label Jan 6, 2020
@stale
Copy link

stale bot commented Apr 5, 2020

Hey there, it looks like there has been no activity on this issue recently. Has the issue been fixed, or does it still require the community's attention? This issue may be closed if no further activity occurs. You may also label this issue as a "Discussion" or add it to the "Backlog" and I will leave it open. Thank you for your contributions.

@stale stale bot added the Stale There has been a lack of activity on this issue and it may be closed soon. label Apr 5, 2020
@stale
Copy link

stale bot commented Apr 12, 2020

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please feel free to create a new issue with up-to-date information.

@stale stale bot closed this as completed Apr 12, 2020
@facebook facebook locked as resolved and limited conversation to collaborators Apr 13, 2020
facebook-github-bot pushed a commit that referenced this issue Sep 28, 2021
Summary:
fixes #27089 fixes #30191 fixes #26296 fixes #24353
Related #30052 #28004 #26536

The keyboard height of event keyboardDidShow is computed as the difference of two variables:

- The screen height excluding the Android Notch
DisplayMetricsHolder.getWindowDisplayMetrics().heightPixels returns the screen height excluding the Android Notch
- The Visible Area excluding the Keyboard, but including the Android Notch
getWindowVisibleDisplayFrame() which returns the visible area including the Android Notch

The computation of the keyboard height is wrong when the device has an Android Notch.
This pr adds the Android Notch computation for API levels 28+

More info at #27089 (comment)

## Changelog

<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->

[Android] [Fixed] - Compute Android Notch in keyboardDidShow height calculation API 28+

Pull Request resolved: #30919

Test Plan:
adding a ReactRootViewTest for keyboardDidShow verifying correct functionality on API < 28

**<details><summary>TEST CASE - BEFORE FIX</summary>**
<p>

**WITHOUT NOTCH**
- The black view on the bottom is visible
- The keyboard height is 282

| **Full Screen** | **Keyboard Did Show** |
|:-------------------------:|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/107212700-a1fd9d00-6a07-11eb-9248-26f9c4d92ae3.png" width="300" height="" /> | <img src="https://user-images.githubusercontent.com/24992535/107212590-7975a300-6a07-11eb-89f4-891a37a7c406.png"  width="300" height="" /> |

**WITH NOTCH**
- The black view on the bottom is **not** visible. The black view is not visible because keyboardDidHide is sending the wrong keyboard height value.
- The keyboard height changes to 234. The keyboard height is the same from the previous test, but the value sent from keyboardDidHide changed for the Notch.

| **Full Screen** | **Keyboard Did Show** |
|:-------------------------:|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/107212619-81cdde00-6a07-11eb-9630-7e7c8c34d798.png" width="300" height="" /> | <img src="https://user-images.githubusercontent.com/24992535/107212707-a4f88d80-6a07-11eb-9134-f077059c83a6.png"  width="300" height="" /> |

</p>
</details>

**<details><summary>TEST CASE - AFTER FIX</summary>**
<p>

**WITH NOTCH**
- The black view on the bottom is visible
- The keyboard height is 282

| **Full Screen** | **Keyboard Did Show** |
|:-------------------------:|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/107212619-81cdde00-6a07-11eb-9630-7e7c8c34d798.png" width="300" height="" /> | <img src="https://user-images.githubusercontent.com/24992535/107349053-0d0ea880-6ac8-11eb-9695-33128080b6b8.png"  width="300" height="" /> |

</p>
</details>

Reviewed By: ShikaSD

Differential Revision: D31207989

Pulled By: cortinico

fbshipit-source-id: 0955a3884201122166c5c0ae2aca988a0ed4af53
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
API: Keyboard Bug Stale There has been a lack of activity on this issue and it may be closed soon.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants