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

Remove protectNavBars property #38

Merged
merged 1 commit into from
Oct 29, 2024

Conversation

Perfomer
Copy link
Contributor

@Perfomer Perfomer commented Oct 28, 2024

Hi there!

I need to make my system bars fully transparent. But ModalBottomSheet forces to protectNavBars = true.

I found the issue #12 and tried this approach, but if you make this:

val window = LocalModalWindow.current
LaunchedEffect(Unit) {
    window.navigationBarColor = Color.TRANSPARENT
    window.statusBarColor = Color.TRANSPARENT
    WindowInsetsControllerCompat(window, window.decorView).isAppearanceLightStatusBars = true
    WindowInsetsControllerCompat(window, window.decorView).isAppearanceLightNavigationBars = true
}

then half-transparent system bars background blinks for a moment anyway.

You can see it on a video:

Screen_recording_20241028_202132.mp4

So it would be great to allow to customize protectNavBars value to avoid it.

Pros:

  • No need to declare LaunchedEffect especially for ModalBottomSheet, you can setup it once for the whole application.
  • No blinking half-transparent systems bar background when opening ModalBottomSheet.

@alexstyl
Copy link
Member

alexstyl commented Oct 28, 2024

This is not an API I want to support as nav bars have nothing to do with bottom sheets. Plus it is something specific for Android and I don't want to polute the API with platform specific properties.

Can you do me a favor and change your PR to remove the protection code from the Modal instead? More specifically, remove the protectNavBars param from Modal and in the android actual remove the code that alters the window.

The protect param was there before LocalModalWindow was introduced and it's super easy to customize now, so it's not really needed anymore.

PS: I am under the assumption that removing them with cause the nav bar to stay as it was before showing the modal, but you would need to test it out.

@alexstyl
Copy link
Member

alexstyl commented Oct 28, 2024

Btw, unrelated to this PR but I noticed that the sheet in your video is not smooth. (does not appear instantly and skips frames)

Not sure if this is the recording or not, but if it is your app, make sure you are not blocking the UI thread. Might even fix your issue

@Perfomer
Copy link
Contributor Author

Perfomer commented Oct 29, 2024

I removed protectNavBars property in the first commit: ac3fc96

And turned back recoloring of the navigation bar when ModalBottomSheet opened (as it was before my changes) in 39c3155, because navigation bar icons become white on white background.

LaunchedEffect(modalSheetState.targetDetent) {
onTargetSheetDetentChange(modalSheetState.targetDetent)
}

Copy link
Member

Choose a reason for hiding this comment

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

The Demo is intended for the documentation website. can you reset the change on the demo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, sure

private fun recolorNavigationBar(isAppearanceLight: Boolean) {
val windowInsetsController = WindowInsetsControllerCompat(window, window.decorView)
windowInsetsController.isAppearanceLightNavigationBars = isAppearanceLight
}
Copy link
Member

Choose a reason for hiding this comment

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

The Demo is intended for the documentation website. can you reset the change on the demo?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

@Perfomer
Copy link
Contributor Author

Btw, unrelated to this PR but I noticed that the sheet in your video is not smooth. (does not appear instantly and skips frames)

Not sure if this is the recording or not, but if it is your app, make sure you are not blocking the UI thread. Might even fix your issue

It's okay, just lagging emulator

@Perfomer Perfomer changed the title Allow to customize protectNavBars property Remove protectNavBars property Oct 29, 2024
@alexstyl alexstyl merged commit 42685f3 into composablehorizons:main Oct 29, 2024
@alexstyl
Copy link
Member

Looks great. Thanks a lot for the contribution. Merged

@alexstyl
Copy link
Member

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants