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

Fix calculating View position within a Window in split-screen mode on Android #28449

Closed

Conversation

jakubkinst
Copy link

Summary

On Android, when using split-screen mode, the Window-relative position of a native View is not calculated properly when the app is running on the right (or bottom) half of the screen. The coordinates are currently calculated only based on View.getLocationOnScreen() with subtracting status bar height. Scenarios, where the window does not fill the entire screen (such as split-screen mode) are not supported.

We need to use a more general solution to subtract the actual position of the window from position of the view within the screen. This PR fixes the issue by subtracting coordinates of the Window retrieved from View.getWindowVisibleDisplayFrame(), which covers all scenarios when Window can be in a different position than the screen (incl. status bar offset).

Changelog

[Android] [Fixed] - Calculating view position within the window in split-screen mode

Test Plan

  1. Run an app in split-screen mode on the right half of the screen
  2. Call UIManagerModule.measureInWindow() from JS to fetch a position of a View within a Window
  3. Observe the wrong coordinates returned
  4. Try the same with the fix

@facebook-github-bot
Copy link
Contributor

Hi @jakubkinst!

Thank you for your pull request and welcome to our community.We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file.

In order for us to review and merge your code, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

If you have received this in error or have any questions, please contact us at [email protected]. Thanks!

@react-native-bot react-native-bot added Bug Platform: Android Android applications. labels Mar 30, 2020
@analysis-bot
Copy link

Platform Engine Arch Size (bytes) Diff
android hermes arm64-v8a 6,596,546 -42
android hermes armeabi-v7a 6,273,711 -30
android hermes x86 6,936,698 -46
android hermes x86_64 6,866,498 -49
android jsc arm64-v8a 8,904,933 -39
android jsc armeabi-v7a 8,545,428 -19
android jsc x86 8,729,103 -46
android jsc x86_64 9,305,139 -53

Base commit: 7007df6

@analysis-bot
Copy link

analysis-bot commented Mar 30, 2020

Platform Engine Arch Size (bytes) Diff
ios - universal 10,800,224 9,833,328

Base commit: 7007df6

@elicwhite elicwhite requested a review from sammy-SC March 30, 2020 20:14
@elicwhite
Copy link
Member

Adding @sammy-SC as he is most familiar with measure and this would probably need to be reflected in fabric as well.

@react-native-bot react-native-bot added the No CLA Authors need to sign the CLA before a PR can be reviewed. label Mar 31, 2020
@thewulf7
Copy link

thewulf7 commented Apr 1, 2020

@sammy-SC can you have a look pls? We need this fix asap :)

@sammy-SC
Copy link
Contributor

sammy-SC commented Apr 4, 2020

I'm not familiar with Android, maybe @mdvacca or @JoshuaGross

@shergin
Copy link
Contributor

shergin commented Apr 20, 2020

@jakubkinst Could you please sing the CLA?

@jakubkinst
Copy link
Author

@jakubkinst Could you please sing the CLA?

done

@facebook-github-bot facebook-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 Apr 20, 2020
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@react-native-bot react-native-bot removed the No CLA Authors need to sign the CLA before a PR can be reviewed. label Apr 20, 2020
Copy link
Contributor

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

Choose a reason for hiding this comment

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

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

@react-native-bot
Copy link
Collaborator

This pull request was successfully merged by Jakub Kinst in b020e7c.

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged This PR has been merged. Platform: Android Android applications.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants