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

feat: detect edge-to-edge and bypass some props #2464

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

Conversation

zoontek
Copy link

@zoontek zoontek commented Oct 29, 2024

Description

This follow a discussion started on the react-navigation repository.

Motivation

The future of Android is edge-to-edge, and to make the React Native developer experience seamless in this regard, the ecosystem needs to transition from “opaque system bars by default” to “edge-to-edge by default.”

To prevent library authors from implementing their own edge-to-edge solutions—which could interfere with other libraries—and because it’s not possible to reliably detect if edge-to-edge is already enabled on Android, we have collaborated with Expo to create a library that handles this functionality and is detectable using a simple helper: react-native-is-edge-to-edge.

This approach allows you to bypass certain options and props (in this case, Screen statusBarTranslucent, navigationBarTranslucent, statusBarColor and navigationBarColor - setting background color is deprecated). All are Android only props and are now obsolete / deprecated (at least, when running in edge-to-edge mode)

Changes

  • Add react-native-is-edge-to-edge to bypass statusBarTranslucent, navigationBarTranslucent, statusBarColor and navigationBarColor values
  • Add a warning about their usage when running in edge-to-edge mode.

Test code and steps to reproduce

Warning

For the StatusBar example, react-native headerTopInsetEnabled might interfere when statusBarTranslucent is explicitely set to false, but the user will receive a warning inviting him to remove the option / prop, so that's acceptable.

More

If you want to bypass some Kotlin code directly, consider this util:

object EdgeToEdgeUtil {
  val ENABLED: Boolean
    get() = try {
      // we cannot detect edge-to-edge, but we can detect react-native-edge-to-edge install
      Class.forName("com.zoontek.rnedgetoedge.EdgeToEdgePackage")
      true
    } catch (exception: ClassNotFoundException) {
      false
    }
}

Checklist

@kkafar
Copy link
Member

kkafar commented Oct 30, 2024

Hey @zoontek I took first glance & I want to reiterate to make sure I get this right:

the goal here is to extract status bar / navigation bar management in terms of its visibility from all packages beside react-native-is-edge-to-edge, so that there is one, centralised solution they (other packages) can use to detect, whether the app is in edge-to-edge. Moreover - manipulation of visibility of these elements should be done only through API of this package.

Note that changes you suggest are breaking. Haven't read your package code, but it assumes that either we're in in edge-to-edge or not, e.g. without possibility of navigation bar being solid (not edge-to-edge) and status bar being edge-to-edge.

I like the direction in general, assuming that we get broader adoption this might be a good thing. Do yo have a list of packages that need to undergo these changes prepared? If so, please share it. I'm asking for it, because it is related to my another point below 👇🏻

I'm reluctant to introduce any third party dependency, especially one with native code. react-native-screens is a upstream package and such dependencies become liabilities especially when it comes to adopting to breaking changes rolled out in react-native between versions. We have this situation already partially (only in example-apps, not in library directly) with different package, which requires us often to provide patches and upstream them, adding maintenance burden. Having this is mind I wonder whether it would be maybe better direction to get rid of this set of functionalities all together from react-native-screens? I haven't made my mind yet. I'm willing to change it. I'll forward this PR internally in order to get some comments on whether we should do it or not.

PS: if we were able to rely on information from react-native-is-edge-to-edge we could improve the situation with initial content jumps on Android (right now, to eliminate it completely the user is required to set appropriate value for statusBarTranslucent, because otherwise we're guessing whether it is translucent or not).

PS 2: the ideal situation would be, that such "centralized solution" would be upstreamed directly to react-native and not a third party package.

@satya164
Copy link
Collaborator

I'm reluctant to introduce any third party dependency, especially one with native code.

afaik it's a JS only and lightweight dep that checks if the react-native-edge-to-edge module is available.

@zoontek
Copy link
Author

zoontek commented Oct 30, 2024

so that there is one, centralised solution they (other packages) can use to detect, whether the app is in edge-to-edge. Moreover - manipulation of visibility of these elements should be done only through API of this package.

As Android 15+ (targetting SDK 15) doesn't allow opaque system bars and enforces apps to draw under, the library enforces edge-to-edge and actually don't allow switching back to opaque bars anymore (because it will be impossible in the future - same for setting bars background color) - it doesn't want to handle their visibility, it aligns with (future) Android enforced behaviour.

Note that changes you suggest are breaking. Haven't read your package code, but it assumes that either we're in in edge-to-edge or not, e.g. without possibility of navigation bar being solid (not edge-to-edge) and status bar being edge-to-edge.

Not at all, this has no impact if react-native-edge-to-edge isn't installed, aka if the user explicitely requested this. By default, react-native-screens continues to work exactly as it works today. See this as a guard to avoid libraries to inferes with each others.

I'm reluctant to introduce any third party dependency, especially one with native code.

I understand, but there's currently no native code involved in the PR, it just detect if another package is installed.

Note that as there's no way to properly detect if WindowCompat.setDecorFitsSystemWindows(window, false) has been set (it would be too easy 😅), so we had only two options: a common library + lib detection or the feature being included in RN core (we asked for it, it might arrive at some point, but not fot the moment).

If in the future, edge-to-edge is officially part of RN core, react-native-is-edge-to-edge will be updated to detect first if edge-to-edge is enabled in RN (we can imagine a gradle property), then if the library is installed, smoothing the transition for everyone (before sunsetting the library).

I like the direction in general, assuming that we get broader adoption this might be a good thing. Do yo have a list of packages that need to undergo these changes prepared?

Currently:

react-native-bootsplash (it's mine, so it's a bit easy). For splash screen, edge-to-edge wide support is really important, because since Android 12 and SplashScreen API the system shows your splash screen before app start, and this one is edge-to-edge, so if your app isn't, a jump happen. Bootsplash solves this with statusBarTranslucent and navigationBarTranslucent to apply negative margins - those options that are now bypassed when react-native-edge-to-edge is installed - it just works).

expo-splash-screen is adopting this API too to finally get rid of these irritating issues - for that the app needs to be edge-to-edge too.

Screenshot 2024-10-30 at 11 11 30

react-native-keyboard-controller, a widely used replacement for KeyboardAvoidingView that use modern method and requires edge-to-edge too:

Screenshot 2024-10-30 at 11 23 20

As you can see, there's multiple good indications that Google wants edge-to-edge by default (enforced on Android 12+, enforced on splash screen, requires to use modern APIs…)

Note that a few other libraries are currently in the work (unistyles, react-native-true-sheet…)

On the framework side, and as I've been commissioned by Expo to work on this, I think it's a matter of time until this is available directly in it (at first, opt-in, then opt-out)

Finally, we are also bringing edge-to-edge to RN core Modal. Meta team is aware of this, they actually asked me to open the PR (and they already recommend the library to deal with the issue).

Let me know if you have further questions!

@kkafar
Copy link
Member

kkafar commented Oct 31, 2024

As Android 15+ (targetting SDK 15) doesn't allow opaque system bars and enforces apps to draw under, the library enforces edge-to-edge and actually don't allow switching back to opaque bars anymore (because it will be impossible in the future - same for setting bars background color) - it doesn't want to handle their visibility, it aligns with (future) Android enforced behaviour.

I see, makes even more sense now.

I understand, but there's currently no native code involved in the PR, it just detect if another package is installed.

Yeah, I've missed the difference between react-native-is-edge-to-edge and react-native-edge-to-edge and inspected only the code of the latter. Dependency on react-native-is-edge-to-edge is much more acceptable.

[...] we had only two options: a common library + lib detection or the feature being included in RN core (we asked for it, it might arrive at some point, but not fot the moment).

It would be good to upstream it to core at some point, definitely. I see your point though.

Thank you for detailed explanation! I'm leaning towards accepting this change, but need some more time to think this through (maybe we want to go more radically and get rid of some future-scope from this library entirely).

@zoontek
Copy link
Author

zoontek commented Oct 31, 2024

@kkafar If it helps take a decision, a new library added support: react-native-bottom-tabs (which plays nice with react-navigation)

@zoontek
Copy link
Author

zoontek commented Nov 7, 2024

@kkafar FYI, we continue pushing for edge-to-edge support. The react native team just merged edge-to-edge for Modal into core.

@satya164
Copy link
Collaborator

satya164 commented Nov 18, 2024

@zoontek I'm curious whether these also apply to React Native's StatusBar component/module since it also changes status bar's color/translucency like the code here. Does it make sense to prevent the user from changing statusbar color if they enable edge to edge? What if they want to do it on one screen?

@satya164
Copy link
Collaborator

@zoontek the other thing you mentioned here is that user needs to manually add these options until this PR is merged zoontek/react-native-edge-to-edge#45 (comment)

But by default these values are undefined iirc, and they'll also be undefined after this PR is merged. So unless user is explicitly overriding somewhere, then there shouldn't be any change in the default behavior, right? Or was this a misunderstanding?

@zoontek
Copy link
Author

zoontek commented Nov 18, 2024

@satya164 Yes, there's no need to set them, only to avoid that those are set to false. My first though here was that something set statusBarTranslucent / navigationBarTranslucent to false somewhere in the dependencies chain (in react-navigation? in expo-router? I didn't check yet at the time I wrote the reply), but false alarm, the issue was with the builtin StatusBar.

If those stay undefined, there's indeed no issue. But it's very easy to imagine that a dependency, somewhere use:

type Props = {
  statusBarTranslucent?: boolean
}

// Here, defaulting to `false` breaks edge-to-edge, it's vicious
const MyComponent = ({ statusBarTranslucent = false }: Props) => {
  return <Screen statusBarTranslucent={statusBarTranslucent} />
}

This kind of issues are super hard to debug if you don't understand the topic perfectly, so a nightmare for users that only want to enable edge-to-edge and stop thinking about it. If react-native-screens detects, ignores and warns about edge-to-edge misconfiguration this became a non-issue and a lot of debug time saved.

@zoontek I'm curious whether these also apply to React Native's StatusBar component/module since it also changes status bar's color/translucency like the code here. Does it make sense to prevent the user from changing statusbar color if they enable edge to edge? What if they want to do it on one screen?

StatusBar is the elephant in the room. They put a warning about backgroundColor in the documentation:

Screenshot 2024-11-18 at 14 04 23

But there's still in the module some code that breaks edge-to-edge: we saw it with the expo-router issue, that just mounted a StatusBar in ExpoRoot. For the moment, react-native-edge-to-edge SystemBars component is the safe way to go.

To tackle the edge-to-edge issue, Meta team asked me for a proposal, it started here (very WIP). The idea is similar:

  • Having a way to enable edge-to-edge easily (enableEdgeToEdge=true)
  • Give to library authors an easy way to detect if it's enabled in native code (BuildConfig.IS_EDGE_TO_EDGE_ENABLED)
  • Give to non-native libraries authors a way to detect if it's enabled in JS (Appearance.isEdgeToEdge())

And of course, this PR takes care of fixing the StatusBar module behavior.

Libraries relying on react-native-is-edge-to-edge will have a smooth transition to builtin edge-to-edge if it lands in core at some time. The detection will be updated to test first if it's enabled in core, then if react-native-edge-to-edge is installed.

Fortunately, the list of libraries that are preparing themselves is growing fast:

I still need to open a PR on reanimated too (to set useAnimatedKeyboard isStatusBarTranslucent to true by default when the app is edge-to-edge)

@zoontek
Copy link
Author

zoontek commented Nov 18, 2024

Does it make sense to prevent the user from changing statusbar color if they enable edge to edge?

For me, yes. As it will not work in the future (when targetSDK = 35 and on Android 15+):

Screenshot 2024-11-18 at 15 17 55

Note that the users can create a <View /> with height: insets.top and a background color. This still works on Android 15+, but also on iOS.

@satya164
Copy link
Collaborator

I think if edge-to-edge will be part of core, in addition to these, it'll be better to mark these props as deprecated as well so they can be removed entirely in the future.

@kkafar
Copy link
Member

kkafar commented Nov 19, 2024

Note: List of resources is for future me 😅

I've familiarised myself with pretty much all the resources you guys linked:

  1. Android behaviour changes when targeting API Level 35+
  2. @zoontek's Proposal to core
  3. Last year's timeline of enforcing minimal targetSdk for PlayStore apps
  4. Once again with all the discussion above ☝🏻
  5. Source code of both react-native-edge-to-edge & react-native-is-edge-to-edge

Conclusions:

the arguments made here are compelling. We'll proceed with merging this PR, after I'm done with testing (should be able to do this by the end of the week). My only concerns right now are Fabric-related, as there is no such thing as undefined value for props on new architecture - codegened component's code always passes some non-null (non-undefined) default value, therefore we need to ensure that these defaults are appropriate.

Another thing I want to address is handling of topInsetEnabled prop on HeaderConfig (link). It would be nice if we had knowledge of whether the app is in edge-to-edge in react-navigation code, to be able to set this prop and prevent content jumping appropriately - I'm thinking whether we should not consider adding react-native-is-edge-to-edge as dependency downstream as well @satya164. Alternative approach would be to remove this topInsetEnabled property entirely - but this is not feasible before the changes land in core, I believe.

Summary for future me:

Android enforces edge-to-edge for all apps with targetSdk >= 35. There is an opt-out mechanism for developers, however it is not recommended for wide use & provided rather as a last resort mechanism. AFAIK current targetSdk requirement for new apps in PlayStore is 34 since August 31 2024 and 33 for existing apps (link). I haven't found any announcements for plans to bumping this to 35, I do not also know Google's track record on bumping the requirements for PlayStore, therefore I do not know how credible are projections that targetSdk >= 35 will be required for new apps since late summer 2025. What I want to highlight is the fact that even if this will be the case it will apply most likely to new apps, not all the existing ones.

Having above ☝🏻 in mind we end up in following situation:

  • For foreseeable future required targetSdk is 34 for new apps,
  • therefore some apps might opt-in into edge-to-edge, but it is not required yet,
  • therefore we should keep providing configuration options of status & navigation bar on Android for sake of non-edge-to-edge applications, however we should deprecate them and mark for future removal,
  • if an application wants to use edge-to-edge it should rely on third party package, such as react-native-edge-to-edge or in the best case, once it is merged, it should rely on functionalities provided by react native core,
  • we should integrate with react-native-is-edge-to-edge to simplify handling edge-to-edge for end user, which is hard now due to fragmentation in the ecosystem.

@kkafar kkafar self-requested a review November 19, 2024 13:58
@kkafar kkafar self-assigned this Nov 19, 2024
@zoontek
Copy link
Author

zoontek commented Nov 19, 2024

My only concerns right now are Fabric-related, as there is no such thing as undefined value for props on new architecture - codegened component's code always passes some non-null (non-undefined) default value, therefore we need to ensure that these defaults are appropriate.

@kkafar We could also update the code to be something like this:

let {
  // Filter out active prop in this case because it is unused and
  // can cause problems depending on react-native version:
  // https://github.com/react-navigation/react-navigation/issues/4886
  active,
  activityState,
  children,
  isNativeStack,
  gestureResponseDistance,
  onGestureCancel,
  style,
  ...props
} = rest;

const {
  // Filter out edge-to-edge related props
  navigationBarColor,
  navigationBarTranslucent,
  statusBarColor,
  statusBarTranslucent,
  ...edgeToEdgeCompatProps
} = props;

const transformedProps = EDGE_TO_EDGE
  ? {
      ...edgeToEdgeCompatProps,
      statusBarTranslucent: true,
      navigationBarTranslucent: true,
    }
  : props;

if (__DEV__) {
  controlEdgeToEdgeValues({
    navigationBarColor,
    navigationBarTranslucent,
    statusBarColor,
    statusBarTranslucent,
  });
}
    
// …

return (
  <DelayedFreeze freeze={freezeOnBlur && activityState === 0}>
    <AnimatedScreen
      {...transformedProps}

@zoontek
Copy link
Author

zoontek commented Nov 26, 2024

@kkafar @satya164 I rebased against main and updated the PR to avoid passing statusBarTranslucent / navigationBarTranslucent / statusBarColor / navigationBarColor completly (not even when undefined, not even true which is what @satya164 suggested here)

EDIT: After discussion with @satya164, he prefers enforcing them to true, I pushed the commit, the PR is ready.

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.

3 participants