Do not bring app out of immersive mode when a modal is presented

[olfek]

This does fix the issue, however, another, perhaps related issue persists, the dialog/modal occasionally uses what would be the height of the app in non-immersive mode, in immersive mode. Meaning that the background, what ever it is set as, does not use the full available height.

See https://stackoverflow.com/a/23207365 for more info.

Known Problems:

• Date/time picker still brings app out of immersive mode - The date/time picker dialog needs the same treatment (this MR) as RN Modal using a wrapper.
• Focusing on text input, which brings up keyboard, also brings app out of immersive mode. Sometimes temporarily, sometimes permanently - Needs investigating. I have tried this, unfortunately it doesn't work. Workaround I'm using for this, is to call a native module method to re-apply immersive mode flags after keyboardDidHide on JS side.

Changelog:

[Android] [Fixed] - Dialog (RN Modal) brings app permanently out of immersive mode

Test Plan:

• RN Modal no longer brings app out of immersive mode

[facebook-github-bot]

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 up 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 the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[elicwhite]

Looks like you have a bad rebase

[olfek]

@TheSavior So will this commit: bd668d72e84b7582f8931738ec47466c31562c79 be accepted?

[hramos]

Can you fix the PR? It looks like a lot of unrelated changes made it in. We can't review the PR in its current state.

[olfek]

@hramos Okay, I'll do it tomorrow, I'll place my commit on the master branch rather than v0.56.0 so it gets rid of all the noise.

I can only confirm that the changes I made work on top of v0.56.0, they probably will still work when applied to master, but you'll have to test that.

[hramos]

We'll let Circle CI run the tests then. It's your responsibility as the author of the PR to make sure your code does not break anything.

[olfek]

@hramos The original PR was tested with v0.56.0, you now want something different, so that responsibility is now yours.

[hramos]

We do not take pull requests against any of the release branches. React Native is continuously developed on master. If you're not willing to support your change on top of master, that's fine -- I'm letting you know what maintainers look for when deciding which pull requests to merge into core.

[olfek]

@hramos I've updated the MR

[pull-bot]

	Warnings
⚠️	📋 Test Plan - This PR appears to be missing a Test Plan.

Generated by 🚫 dangerJS

[olfek]

@hramos Hello, could I get an update on this, thanks.

[hramos]

Would you mind adding a test plan to your PR?

[olfek]

What do you mean by test plan? please be more specific.

[hramos]

The pull request template provides a "Test Plan" section: https://github.com/facebook/react-native/blob/master/.github/PULL_REQUEST_TEMPLATE.md

The general idea is to describe, in plain language, how you verified that your change works as intended. It's an important element of a code review, as it lets the reviewer know if there might be a blind spot in your testing where additional bugs may be introduced.

[olfek]

@hramos I've added a test plan

[olfek]

This is how to start the app in immersive mode.

package [REDACTED];

import android.view.View;
import com.facebook.react.ReactActivity;

public class MainActivity extends ReactActivity {

    @Override
    public void onWindowFocusChanged(boolean hasFocus) {
        super.onWindowFocusChanged(hasFocus);
        if(hasFocus){
            hideSystemUI();
        }
    }

    public void hideSystemUI(){
        View decorView = getWindow().getDecorView();
        decorView.setSystemUiVisibility(
            View.SYSTEM_UI_FLAG_IMMERSIVE_STICKY |
            View.SYSTEM_UI_FLAG_FULLSCREEN |
            View.SYSTEM_UI_FLAG_HIDE_NAVIGATION
        );
    }

    /**
     * Returns the name of the main component registered from JavaScript.
     * This is used to schedule rendering of the component.
     */
    @Override
    protected String getMainComponentName() {
        return "[REDACTED]";
    }
}

[olfek]

Workaround I'm using for this, is to call a native module method to re-apply immersive mode flags after keyboardDidHide on JS side. #21078 (comment)

package [REDACTED];

import android.content.Context;
import android.os.Handler;
import android.util.Log;
import com.facebook.react.bridge.ReactApplicationContext;
import com.facebook.react.bridge.ReactContextBaseJavaModule;
import com.facebook.react.bridge.ReactMethod;

public class ReApplyImmersiveModeModule extends ReactContextBaseJavaModule {

    public ReApplyImmersiveModeModule(ReactApplicationContext reactContext) {
        super(reactContext);
    }

    @Override
    public String getName() {
        return "ReApplyImmersiveMode";
    }

    // Need this because of this issue: https://stackoverflow.com/q/24187728
    // The answer didn't work, so this is a workaround.
    @ReactMethod
    public void now(){

        final ReactApplicationContext context = getReactApplicationContext();

        Runnable runnable = new Runnable() {
            @Override
            public void run() {
                MainActivity.hideSystemUI(context.getCurrentActivity().getWindow().getDecorView());
            }
        };

        if(context.getCurrentActivity()!=null) {
            Handler mainHandler = new Handler(context.getMainLooper());
            mainHandler.post(runnable);
        }
    }
}

[olfek]

@hramos ...

[hramos]

Thanks for adding the test plan. It looks like it's lacking a bit of detail. I'm looking for a list of steps you followed to ensure your change fixes the issue, as well as steps followed to ensure no side effects or bugs are introduced.

We also will need a CLA to be signed before the code can be merged.

[facebook-github-bot]

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

[olfek]

My testing produced no evidence of any side effects or bugs being introduced.

I discovered this issue when attempting to display a dialog (RN Modal) in an immersive (full screen) application I was building.

When opening a dialog, it would bring the application out of immersive mode until the dialog was closed again.

After a quick Google search, I found that the issue was with Android and not RN (more info).

So I applied the fix mentioned in the q&a linked above which resolved the problem.

So no specific dedicated testing has been done apart from being able to see that the problem went away after applying the fix.

Regarding my previous attempt at writing a test plan, the 'Known Problems' section lists other issues that are similar to this one but are unrelated and are not as a result of this MR.

Changelog:

[Android] [Fixed] - Dialog (RN Modal) brings app permanently out of immersive mode

Test Plan:

• RN Modal no longer brings app out of immersive mode

Known Problems:

• Date/time picker still brings app out of immersive mode - The date/time picker dialog needs the same treatment (this MR) as RN Modal using a wrapper.
• Focusing on text input, which brings up keyboard, also brings app out of immersive mode. Sometimes temporarily, sometimes permanently - Needs investigating. I have tried this, unfortunately it doesn't work. Workaround I'm using for this, is to call a native module method to re-apply immersive mode flags after keyboardDidHide on JS side.

[facebook-github-bot]

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

[react-native-bot]

@olfek merged commit f8a4d28 into facebook:master.