Skip to content

[Android] keyboardDidHide wrong screenY coordinates with windowTranslucentStatus=true - #29292

Closed
fabOnReact wants to merge 1 commit into
react:masterfrom
fabOnReact:fix-keyboard-avoiding-view
Closed

[Android] keyboardDidHide wrong screenY coordinates with windowTranslucentStatus=true#29292
fabOnReact wants to merge 1 commit into
react:masterfrom
fabOnReact:fix-keyboard-avoiding-view

Conversation

@fabOnReact

@fabOnReactfabOnReact commented Jul 7, 2020

Copy link
Copy Markdown
Contributor

Summary

This issue fixes#27526fixes#30052, related issue #27089
Avoid returning the wrong screenY coordinates on event keyboardDidHide.
getWindowVisibleDisplayFrame retrieves the overall visible display size in which the window this view is attached to has been positioned in. This takes into account screen decorations above the window, for both cases where the window itself is being position inside of them or the window is being placed under then and covered insets are used for the window to position its content inside. In effect, this tells you the available area where content can be placed and remain visible to users, since anything below the StatusBar is not visible to the user, the method does not work with translucent StatusBar (android:windowTranslucentStatus).

This commit fixes this issues removing the white bar at the bottom of the view when using windowTranslucentStatus.

Changelog

[Android] [Fixed] - keyboardDidHide wrong screenY coordinates with windowTranslucentStatus=true

Test Plan

Works in all scenarios, but I did not change RNTester windowTranslucentStatus. I would like to discuss the potential breaking changes connected to not using getWindowVisibleDisplayFrame with keyboardDidHide.

I would be happy to build an alternative functionality to calculate the WindowVisibleDisplayFrameHeight under request from the Facebook Team.

WITHOUTandroid:windowTranslucentStatus

BEFORE
AFTER

WITHandroid:windowTranslucentStatus

BEFORE
BEFORE (log)
AFTER
AFTER (log)

RNTester WITHOUTandroid:windowTranslucentStatus

BEFOREAFTER

I remain available to do improvements. Thanks a lot. Fabrizio.

@facebook-github-botfacebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jul 7, 2020
@fabOnReactfabOnReact changed the title keyboardDidHide wrong screenY coordinates with windowTranslucentStatus=true[Android] keyboardDidHide wrong screenY coordinates with windowTranslucentStatus=trueJul 7, 2020
@react-native-botreact-native-bot added Bug Platform: Android Android applications. labels Jul 7, 2020
@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
androidhermesarm64-v8a6,763,750107
androidhermesarmeabi-v7a6,426,68583
androidhermesx867,151,383100
androidhermesx86_647,041,303100
androidjscarm64-v8a8,935,703100
androidjscarmeabi-v7a8,591,04598
androidjscx868,766,44685
androidjscx86_649,342,008100

Base commit: e23e932

@analysis-bot

Copy link
Copy Markdown
PlatformEngineArchSize (bytes)Diff
ios-universaln/a--

Base commit: e23e932

@facebook-github-botfacebook-github-bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mdvacca has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

@mdvacca

Copy link
Copy Markdown
Contributor

Importing to review

@react-native-bot

Copy link
Copy Markdown
Collaborator

This pull request was successfully merged by @fabriziobertoglio1987 in 45954ac.

When will my fix make it into a release? | Upcoming Releases

@react-native-botreact-native-bot added the Merged This PR has been merged. label Jul 15, 2020
kelset pushed a commit that referenced this pull request Sep 29, 2020
…s=true (#29292)
Summary:
This issue fixes#27526, related issue #27089
Avoid returning the wrong screenY coordinates on event keyboardDidHide.
getWindowVisibleDisplayFrame retrieves the overall visible display size in which the window this view is attached to has been positioned in. This takes into account screen decorations above the window, for both cases where the window itself is being position inside of them or the window is being placed under then and covered insets are used for the window to position its content inside. In effect, this tells you the available area where content can be placed and remain visible to users, since anything below the StatusBar is not visible to the user, the method does not work with translucent StatusBar (android:windowTranslucentStatus).
This commit fixes this issues removing the white bar at the bottom of the view when using windowTranslucentStatus.
## 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] - keyboardDidHide wrong screenY coordinates with windowTranslucentStatus=true
Pull Request resolved: #29292
Test Plan:
Works in all scenarios, but **I did not change RNTester `windowTranslucentStatus`**. I would like to discuss the potential breaking changes connected to not using [getWindowVisibleDisplayFrame](https://developer.android.com/reference/android/view/View#getWindowVisibleDisplayFrame(android.graphics.Rect)) with `keyboardDidHide`.
I would be happy to build an alternative functionality to calculate the WindowVisibleDisplayFrameHeight under request from the Facebook Team.
**WITHOUT** `android:windowTranslucentStatus`
| **BEFORE** |
|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/86804255-0eca4680-c077-11ea-8c79-95ba297d05ba.gif" />|
| **AFTER** |
|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/86804265-10940a00-c077-11ea-8cb8-a304cc5de363.gif" /> |
**WITH** `android:windowTranslucentStatus`
| **BEFORE** |
|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/86804458-3a4d3100-c077-11ea-8f3e-7c43eaa08d70.gif" height="" />|
| **BEFORE (log)** |
|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/86806330-0d018280-c079-11ea-9266-c3bcf23a35da.png" height="" />|
| **AFTER** |
|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/86804464-3b7e5e00-c077-11ea-8487-34c87f076e5f.gif" height="" /> |
| **AFTER (log)** |
|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/86806042-c4e26000-c078-11ea-9c3c-cac5319bef65.png" height="" /> |
RNTester **WITHOUT** `android:windowTranslucentStatus`
| **BEFORE** | **AFTER** |
|:-------------------------:|:-------------------------:|
| <img src="https://user-images.githubusercontent.com/24992535/86805348-176f4c80-c078-11ea-8e2a-e2d84af5c278.gif" width="300" height="" />| <img src="https://user-images.githubusercontent.com/24992535/86805324-13432f00-c078-11ea-9e70-fa2606eee86b.gif" width="300" height="" /> |
I remain available to do improvements. Thanks a lot. Fabrizio.
Reviewed By: JoshuaGross
Differential Revision: D22521125
Pulled By: mdvacca
fbshipit-source-id: e2cb90163abb1baa00b1916e431971b011522565
@fabOnReact

Copy link
Copy Markdown
ContributorAuthor

Thanks a lot David Vacca 🙏

@andres-torres-marroquin

Copy link
Copy Markdown

This fix breaks the keyboardDidHide event in Android by sending screenY a value that is not correct, it basically crops the view at about 90% of the original height when the keyboard is hidden. I've tested 0.63.3 and 0.63.2, and the 0.63.2 works as intended. I'm using react-native-navigation with TopBar and BottomTabs, if that helps.

I just want to be sure if this change is actually correct?

@fabOnReact

fabOnReact commented Jan 20, 2021

Copy link
Copy Markdown
ContributorAuthor

@andres-torres-marroquin I'll check into it this weekend, could you open and issue in react-native and reference this pr?
The issues needs to have a minimum reproducible example. Thanks a lot 🙏

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BugCLA SignedThis label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.MergedThis PR has been merged.Platform: AndroidAndroid applications.

Projects

None yet

6 participants

@fabOnReact@analysis-bot@mdvacca@react-native-bot@andres-torres-marroquin@facebook-github-bot