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

Sync Modal system bars visibility with current activity #48516

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

mateoguzmana
Copy link
Contributor

@mateoguzmana mateoguzmana commented Jan 7, 2025

Summary:

Fixes #37801

This PR fixes the system bars visibility not being in sync as with the activity it is displayed on.

Here I am also taking into account the feedback given in this PR which was also aiming to address this issue. Unfortunately, I tried in multiple ways to get this to work by simply extending the whole system bars behaviour in the dialog with the activity system bars behaviour but found out that that solution won't work as we currently clear the flag "FLAG_NOT_FOCUSABLE" and unless we remove that line, extending the behaviour won't work. Removing that line is not an option as it would cause other side effects in the dialog itself.

With the above said, I ended up doing this in a more explicit way, by checking whether the status or navigation bars are hidden in the activity and then hiding then as well in the dialog or otherwise, similar as we are currently doing with the status bars appearance.

Changelog:

[ANDROID] [FIXED] - Sync Modal system bars visibility with current activity

Test Plan:

In order to test this, you need to trigger any method that hides or shows the system bars using window.insetsController. Here a very small example of it:

fun toggleSystemBarsVisibility(shouldHide: Boolean) {
  val window = currentActivity?.window
  val controller = window?.insetsController
  if (shouldHide) {
    controller?.hide(WindowInsets.Type.systemBars())
  } else {
    controller?.show(WindowInsets.Type.systemBars())
  }
}

You can do this optionally from JS to make testing different cases easier. Below is a screen recording of how the solutions looks like:

Screen recording showcasing the solution in the test plan
Screen.Recording.2024-12-20.at.23.34.09.mov

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team. labels Jan 7, 2025
@facebook-github-bot
Copy link
Contributor

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

@alanleedev
Copy link
Contributor

@mateoguzmana Thanks for submitting the PR. I'm looking into a few things;

  • using WindowInsetsControllerCompat to make it backwards compatible
  • testing on various API versions to make sure it works as expected.

@mateoguzmana
Copy link
Contributor Author

@mateoguzmana Thanks for submitting the PR. I'm looking into a few things;

  • using WindowInsetsControllerCompat to make it backwards compatible
  • testing on various API versions to make sure it works as expected.

Thanks for looking into this! if there is anything I can help with among those lines just let me know.

@TonyFTannous-Byd
Copy link

My react native is 0.76.6, the system navigation bar still appear, the system bar still appear when modal display
any solution

thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Shared with Meta Applied via automation to indicate that an Issue or Pull Request has been shared with the team.
Projects
None yet
4 participants