-
-
Notifications
You must be signed in to change notification settings - Fork 3.4k
Use DisplayCompat.getMode(). #6638
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
Use DisplayCompat.getMode(). #6638
Conversation
|
Dev notes: Be sure to read the changes between Core 1.3.2 and 1.6.0: https://developer.android.com/jetpack/androidx/releases/core |
b4d0467 to
57ce695
Compare
| windowManager.getDefaultDisplay()); | ||
|
|
||
| final int viewPagerVisibleHeight = displaySize.y - pagerHitRect.top; | ||
| final int viewPagerVisibleHeight = mode.getPhysicalHeight() - pagerHitRect.top; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not correct. viewPagerVisibleHeight should represent the height of the view pager that is visible on the screen, but here the size of Android's bottom bar is also summed, since getPhysicalHeight() is used. But now that I see it, it seems like it was also wrong before, since getDefaultDisplay().getSize() returned the height of the app and the above difference was also wrong. What viewPagerVisibleHeight should be equal to is "y_of_the_bottom_of_the_activity (i.e. just above Android's bottom bar) - y_of_the_top_of_viewPager". Is there a way to do this with the compat library? I saw that getDefaultDisplay().getSize() was deprecated in API 30 in favour of WindowMetrics, and WindowMetrics#getBounds() would provide us with "y_of_the_bottom_of_the_activity", but that method has not yet been backported in the compat library.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There's a Window Manager library which backports WindowMetrics, but it's still in alpha.
Mmmh, then I'd rather not merge this and keep the old behaviour, that has proven to work well (I haven't seen any issue about it and for me it works fine). If you or someone from the team disagrees, feel free to reopen ;-) |
What is it?
Description of the changes in your PR
DisplayCompat.getMode()to get the current display size.APK testing
The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.
Due diligence