[Android] Fix wrong modal size in fullscreen

[rgngl]

Display.getCurrentSizeRange() doesn't include the size of the status
bar, so Modal windows on Android have a gap in the bottom that is the
same size as the status bar. This checks the current theme and adds the
size of status bar to the modal window height if necessary.

Test plan (required)

Run a React Native app on Android with a theme that doesn't show status bar and launch a modal dialog. See issue #11872 for an example.

[facebook-github-bot]

By analyzing the blame information on this pull request, we identified @andreicoman11 and @davepack to be potential reviewers.

[andreicoman11]

This doesn't look correct to me, I'm fairly certain this was fixed a few months back.
Have you tested this on several APIs (16 to 25), and different Themes?

[rgngl]

Yes, I tested with android devices with different themes and different configurations(with hw buttons and on screen navigation bar). I can reproduce the issue in #11872 on all. The documentation for Display.getCurrentSizeRange() always return the size as if the status bar is there. See the relevant documentation: https://developer.android.com/reference/android/view/Display.html#getCurrentSizeRange(android.graphics.Point,%20android.graphics.Point)

Filled in with the largest width and height that the application will encounter, in pixels (not dp units). Your application may of course still get larger space than this if, for example, screen decorations like the status bar are being hidden.

[rgngl]

@andreicoman11 I was wondering if there is actually a way to tell the first child inside ReactModalHostView to size itself as big as its parent. That way we wouldn't need to manually set the size here and the DialogRootViewGroup inside the Modal always sizes itself correctly.

[rgngl]

@andreicoman11 any comments? I can provide a small test app to show that this is broken.

[andreicoman11]

I need to have some free time to play with some of the Modals we already have. I'm fairly certain that they'll be working correctly.
I have a feeling that you are working with a theme that lets you draw the Modal underneath the status bar, which is not the general case and which we won't want to support by default. In that case, I recommend changing you modal to have top: status_bar_height in JS.

[rgngl]

I tried setting top: status_bar_height but it doesn't work. My app is not drawing behind the status bar, it's a full screen app where the status bar is not there. I saw that there is at least one more issue that was opened last May, so it means there are people who might need this. And I don't think having a modal without a status bar is an odd corner case.

Here's a small test app: https://github.com/rgngl/gaptest

It uses the branch I created for the PR #11958 so you can try out both things at once.

[rgngl]

I have noticed one thing: If the modal view gets an onSizeChanged after it shows up, it is correctly sized to fill the screen. But the initial size setting in the helper class is still wrong.

From what I can see, modal sizing was fixed in 4941cbc where you removed the shadow node for modal host view but then it got broken again in 922cd6d where you brought it back.

[andreicoman11]

Checked this locally, it's working as expected.
You can take a look at the UIExample/Modal, and add the following to the container style:

    borderWidth: 5,
    borderColor: 'red',

This will show you what the actual bounds of the Modal are. And they are indeed correct:

Since this is working as expected, I will close this PR. Feel free to open a new one with a JS example where the modal size is incorrect.

[rgngl]

Hi @andreicoman11 ,

I have already provided an example where it's not working. You can check the example I provided above. You can also see the screenshots I posted in #11872. In the screenshot you shared the status bar is there, so yes, in that situation there is no problem. The gap under the model window appears when the status bar is not visible.

Display.getCurrentSizeRange used in ModalHostHelper always returns the maximum screen size without status and navigation bars. Therefore, if the status bar is not visible, the gap happens.

Please let me know if I am missing a point here.

[rt2zz]

@andreicoman11 this is definitely still an issue, please note as described by @rgngl the issue pertains to the status bar visibility.

[ghost]

This needs to get reopened and merged asap, the maintainer was arrogant to not fully read what is happening and test it for him self in the wrong way. This only happens in fullscreen without the status bar.

[nihgwu]

👍

[rgngl]

Thanks @christopherdro !

[rgngl]

Hi @christopherdro, is this going to merge after all?

[christopherdro]

Yes I just wanted to do one last test myself to make sure everything was working as expected.

[christopherdro]

@facebook-github-bot shipit

[facebook-github-bot]

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