-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
feature: Detect edge-to-edge and set isStatusBarTranslucentAndroid / isNavigationBarTranslucentAndroid #6732
base: main
Are you sure you want to change the base?
Conversation
Hi @zoontek, thanks for this PR. We briefly discussed this internally and @bartlomiejbloniarz will take a look on it. |
@tomekzaw @bartlomiejbloniarz Any news? It continues to create hard to detect issues. |
Hi @zoontek. I changed the version of |
@bartlomiejbloniarz Sure, no worry 🙂 |
e04f041
to
5d91625
Compare
@bartlomiejbloniarz I rebased against main to fix the conflict. |
@@ -89,7 +89,8 @@ | |||
"@babel/plugin-transform-unicode-regex": "^7.0.0-0", | |||
"@babel/preset-typescript": "^7.16.7", | |||
"convert-source-map": "^2.0.0", | |||
"invariant": "^2.2.4" | |||
"invariant": "^2.2.4", | |||
"react-native-is-edge-to-edge": "1.1.6" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure about it. If react-native-is-edge-to-edge
is supposed to be embraced by the community, we should have it as a peer dependency instead to avoid conflicts for the user, no?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But then anyone who uses just Reanimated would also need to install react-native-is-edge-to-edge
which is a bit cumbersome for us.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tjzel Yes, that means an extra step for everyone, not a good idea. BTW react-native-is-edge-to-edge
(the detection library, not react-native-edge-to-edge
- I think that's where the confusion lies) is less then 100 bytes in production builds.
@bartlomiejbloniarz @tjzel Note that if it's blocking for you to add a dependency, we can also just inline the check and remove the dep. This is super lightweight, no dependencies, but also no control / warnings. |
Don't worry @zoontek. I looked into the packages code and I understood the context more and I think there won't be problems even if there are multiple versions loaded in. |
Currently we are freezing the main branch for maintenance reasons so it will land there a bit later. |
…isNavigationBarTranslucentAndroid # Conflicts: # packages/react-native-reanimated/src/core.ts # Conflicts: # yarn.lock
5d91625
to
bde185f
Compare
I rebased against |
Summary
Similar to the PR I opened on the
react-native-screens
repository (I highly recommend to read the discussion there to understand the motivation behind this), this PR detects if the user enabled edge-to-edge and act accordingly:useAnimatedKeyboard
are ignored, set totrue
automatically. If those are set, a warning is logged:It at some point this proposal lands in core,
react-native-is-edge-to-edge
will be updated to support both the library and the core edge-to-edge flag, making the transition seamless for the users.Test plan
isStatusBarTranslucentAndroid
/isNavigationBarTranslucentAndroid
, or set them to something else thantrue