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

Proposal: Edge-to-edge support #47554

Draft
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

zoontek
Copy link
Contributor

@zoontek zoontek commented Nov 11, 2024

Summary:

Following our discussion on Discord, here's the WIP of a proposal to add edge-to-edge support directly in core. This is based on a few things:

A new gradle property: edgeToEdgeEnabled

This could be changed by the user (similar to hermesEnabled / newArchEnabled) and should be false by default (for the moment), and is usable by third-party libraries:

def isEdgeToEdgeEnabled() {
    return project.hasProperty("edgeToEdgeEnabled") && project.edgeToEdgeEnabled == "true"
}

//

defaultConfig {
    buildConfigField("boolean", "IS_EDGE_TO_EDGE_ENABLED", isEdgeToEdgeEnabled().toString())
    //
}
if (BuildConfig.IS_EDGE_TO_EDGE_ENABLED) {
  //
}

Appearance.isEdgeToEdge()

Similar to the native side detection, this PR also add JS side detection.

This is useful for packages that doesn't rely on native code (for example, it will be used by react-native-is-edge-to-edge to detect either react-native-edge-to-edge install or if edge-to-edge is enabled in core).

Update StatusBar props behavior

In order to keep edge-to-edge working, it's mandatory to disable some props behavior: backgroundColor and translucent have no effect when edge-to-edge is enabled (setBackgroundColor / setTranslucent methods too)

What's missing

  • Fixing the Dimensions module to includes status bar and navigation bar insets in window size computation
  • Writing some tests
  • Probably some comments 😅

Changelog:

[ANDROID] [ADDED] Edge-to-edge support

Test Plan:

Run the example app:

Screenshot_20241111-175212

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Nov 11, 2024
@janicduplessis
Copy link
Contributor

Why would StatusBar backgroundColor prop have no effect in edge to edge? I have cases where I use a semi transparent background in edge to edge mode.

@zoontek
Copy link
Contributor Author

zoontek commented Nov 12, 2024

@janicduplessis Setting status bar color (window.setStatusBarColor) is deprecated and has no effect on Android 15+ (when targetting SDK35) - source.

setStatusBarColor and R.attr#statusBarColor are deprecated and have no effect on Android 15.

Deprecated APIs

  The following APIs are deprecated and disabled:
  …
  Window#setStatusBarColor

Instead, you can still put an empty View on the top of their screen, with a backgroundColor and using top inset as height. Bonus: it will works on every platform, not just Android.

@alanleedev
Copy link
Contributor

@zoontek Would there be a way to avoid adding isEdgeToEdgeEnabled to ReactHost?

@zoontek
Copy link
Contributor Author

zoontek commented Nov 12, 2024

@alanleedev Would you be OK in using BuildConfig.IS_EDGE_TO_EDGE_ENABLED directly? Or maybe having a simple fun isEdgeToEdgeEnabled(): Boolean = BuildConfig.IS_EDGE_TO_EDGE_ENABLED somewhere?

@grahammendick
Copy link
Contributor

To enable edge-to-edge in SDK < 35 Android’s recommendation is to call enableEdgeToEdge on ComponentActivity. Why isn’t that the approach taken here? And shouldn’t Appearance.isEdgeToEdge return true if the Android recommended approach is used (even if that isn’t the approach used by React Native)?

@zoontek
Copy link
Contributor Author

zoontek commented Nov 12, 2024

@grahammendick Because Window.enableEdgeToEdge is also used for Modal, which extends Dialog. The current code is based on the AndroidX implementation, just not tied to ComponentActivity.

@grahammendick
Copy link
Contributor

@zoontek Most people that use React Native don't use the React Native Modal component. So they can turn on edge-to-edge by calling enableEdgeToEdge. Wouldn’t it be odd if Appearance.isEdgeToEdge returned false in that case? Maybe it would be better to work out if edge-to-edge is enabled, for example, by listening for windows insets instead? Then React Native can make its Modal edge-to-edge if insets are applied?

@zoontek
Copy link
Contributor Author

zoontek commented Nov 13, 2024

@grahammendick Android doesn't provide a way to properly detect if edge-to-edge is enabled, so even if let's say, the user call ComponentActivity.enableEdgeToEdge on their side (not by using enabledEdgeToEdge=true), Appearance.isEdgeToEdge will be false too.

Add the facts that:

  • it requires adding androidx.activity:activity to RN
  • it doesn't easily work with Modal, or any Dialog
  • the whole logic is ~30 lines:
public fun Window.enableEdgeToEdge() {
  val isDarkMode = ContextUtils.isDarkMode(context)

  WindowCompat.setDecorFitsSystemWindows(this, false)

  if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
    isStatusBarContrastEnforced = false
    isNavigationBarContrastEnforced = true
  }

  statusBarColor = Color.TRANSPARENT

  navigationBarColor =
    when {
      Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q -> Color.TRANSPARENT
      Build.VERSION.SDK_INT >= Build.VERSION_CODES.O && !isDarkMode -> Color.argb(0xe6, 0xFF, 0xFF, 0xFF)
      else -> Color.argb(0x80, 0x1b, 0x1b, 0x1b)
    }

  WindowInsetsControllerCompat(this, this.decorView).run {
    isAppearanceLightNavigationBars = !isDarkMode
  }

  if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.P) {
    attributes.layoutInDisplayCutoutMode =
      when {
        Build.VERSION.SDK_INT >= Build.VERSION_CODES.R -> WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_ALWAYS
        else -> WindowManager.LayoutParams.LAYOUT_IN_DISPLAY_CUTOUT_MODE_SHORT_EDGES
      }
  }
}

And it's hard to justify going that way.

@grahammendick
Copy link
Contributor

Android doesn't provide a way to properly detect if edge-to-edge is enabled

React Native can add a OnApplyWindowInsets listener to the root view. If any top/left/right/bottom inset is > 0 then edge-to-edge is enabled. That should work no matter how edge-to-edge is enabled

@zoontek
Copy link
Contributor Author

zoontek commented Nov 13, 2024

@grahammendick Even with that, you still need to have this function in RN to apply edge-to-edge on Modal. That's why the extra dependency isn't worth it.

Regarding using OnApplyWindowInsets for Appearance.isEdgeToEdge, this is an idea, but this could result in a different value than the BuildConfig.IS_EDGE_TO_EDGE_ENABLED one.
Let's suppose user are enabling edge-to-edge without using the built-in solution: libraries relying on BuildConfig.IS_EDGE_TO_EDGE_ENABLED on the native side will interferes as it will return false. Meanwhile, on the JS side, Apperance.isEdgeToEdge() will be true.

@cipolleschi
Copy link
Contributor

I'm not really the right person to review this as I don't have that extensive knowledge on Android.
I just want to mention that @cortinico is on PTO for a couple of weeks, and he probably have some opinions on this proposal, so I'd kindly ask for a bit of patience and to wait for him to come back. 🙏

@@ -59,6 +61,13 @@ internal object ProjectUtils {
HERMES_FALLBACK
}

internal val Project.isEdgeToEdgeEnabled: Boolean
get() =
(project.hasProperty(EDGE_TO_EDGE_ENABLED) &&
Copy link
Contributor

Choose a reason for hiding this comment

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

I feel like it's code repeat 😓 I found it difficult to read when we have a mixed && and || operators. After spending several seconds I realized what this check does, but maybe it's better to create a common function and re-use it?

internal val Project.isEdgeToEdgeEnabled: Boolean
    get() = isPropertyEnabled(EDGE_TO_EDGE_ENABLED) || isPropertyEnabled(SCOPED_EDGE_TO_EDGE_ENABLED)

private fun Project.isPropertyEnabled(propertyName: String): Boolean {
    return hasProperty(propertyName) && property(propertyName).toString().toBoolean()
}

Maybe such code will be better?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied the code structure above but I agree.

const state = getState();
const {NativeAppearance} = state;
if (NativeAppearance != null) {
if (state.edgeToEdge == null) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we use === instead of ==? Or there is a chance that edgeToEdge can be undefined?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's just to please the type system, copied from the line above.

private fun Window.statusBarShow() {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) {
private fun Window.statusBarShow(isEdgeToEdge: Boolean) {
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R && !isEdgeToEdge) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Always was hesitant to ask, but now I feel like it's a right time 🙂

From what I remember StatusBar management uses some deprecated methods for managing its properties. As a result it may break some animations driven by WindowInsetsAnimationCompat.Callback.

To overcome this problem I had to create own module based on WindowInsetsControllerCompat class and monkey-patch JS module afterward.

This is directly not related to this PR, but I'd like to discuss a possibility to rewrite StatusBar management to a new/modern API 🙌 (whether FB team is open to it and if not then what blocks/prevents that).

Copy link
Contributor

@alanleedev alanleedev Nov 19, 2024

Choose a reason for hiding this comment

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

@kirillzyusko We can discuss about it if we have bit more details on the change you wan to make. There is also react-native-edge-to-edge SystemBars. So may need to check if it would be better to update that instead.

else -> Color.argb(0x80, 0x1b, 0x1b, 0x1b)
}
if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
isStatusBarContrastEnforced = false
Copy link
Contributor

Choose a reason for hiding this comment

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

Also just a random thought in my head...

Would it be possible to expose a new component (NavigationBar, similar to StatusBar) and add a possibility to customize properties of NavigationBar (backgroundColor/translucent/style (dark/light))?

I know that there are libraries react-native-bars that were aiming to deliver this API. But since we add an ability to toggle edge-to-edge mode, maybe it would be reasonable to add other components as well, so that RN will have a full feature set of component out of the box?

Copy link
Contributor Author

@zoontek zoontek Nov 16, 2024

Choose a reason for hiding this comment

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

For me, having a NavigationBar component is a bit out-of-scope as a lot of these props will have no effect. react-native-edge-to-edge has a SystemBars component, which doesn't uses deprecated APIs and align to what is enforced on Android 15. And only hidden matters for navigation bar.

  • backgroundColor -> will have no effect with gesture navigation, only with buttons navigation
  • barStyle -> same, as gesture navigation style is automatic and based on contrast

For those two to be usable, we also need to set setStatusBarContrastEnforced to false, which doesn't align with what's enforced in Android 15 / what occurs when AndroidX coreenableEdgeToEdge is used.

By default, they keep a semi opaque nav bar (white or black, dependending on the color scheme) for buttons nav and a transparent one for gesture nav. That avoids the additional burden to handle nav bar color / style only for Android users with buttons navigation on (not even the default anymore).

I think it's not worth the developers extra work / the introduction of deprecated APIs, and that the Google choice is probably the good one here.

@kirillzyusko
Copy link
Contributor

Wouldn’t it be odd if Appearance.isEdgeToEdge returned false in that case? Maybe it would be better to work out if edge-to-edge is enabled, for example, by listening for windows insets instead? Then React Native can make its Modal edge-to-edge if insets are applied?

I also agree with @zoontek here. Theoretically we can listen to root view insets and based on that decide whether we are in edge-to-edge mode or not. But in my opinion it will add more code complexity and at some point of time there will appear a situations, when something not working etc.

The case that you described it a simple misconfiguration between two modules (user-defined code and RN). and such misconfiguration happens relatively frequently now - that's why @zoontek created react-native-edge-to-edge and react-native-is-edge-to-edge and opened PRs to many repos to adopt edge-to-edge in RN ecosystem.

I think the best way here would be to write a blog post explaining for people, that if they used edge-to-edge before (and maybe list third party dependencies that are enabling this mode by default), then they need to specify edgeToEdge=true in gradle.properties. It's always better to have a single source of truth (value inside gradle.properties) in a manageable/configurable way (in my opinion) 🙂

@@ -48,6 +48,8 @@ public interface ReactHost {
/** Routes memory pressure events to interested components */
public val memoryPressureRouter: MemoryPressureRouter

public fun isEdgeToEdgeEnabled(): Boolean
Copy link
Contributor

Choose a reason for hiding this comment

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

I have some concern with adding this field to ReactHost as this more of a UI configuration that will eventually go away as edge-to-edge will become the default for Android and as we raise the minSdk. Doesn't seem like it is something we should keep in ReactHost.

Copy link
Contributor Author

@zoontek zoontek Dec 3, 2024

Choose a reason for hiding this comment

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

@alanleedev I agree, just tried to copy the current code structure, but it's not optimal. What would you think about removing all of this and just have an EdgeToEdgeUtils.kt somewhere?

object EdgeToEdgeUtils {
  fun isEnabled(): Boolean {
    return BuildConfig.IS_EDGE_TO_EDGE_ENABLED
  }
}

EDIT: Hmm, this can't be done so easily as the BuildConfig is scoped to the project and can't be used in the react-native codebase directly, we still need to pass it. I'll have a check on how to remove it from ReactHost

@zoontek zoontek force-pushed the proposal-edge-to-edge branch from e101925 to 90ad847 Compare December 3, 2024 10:10
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants