-
-
Notifications
You must be signed in to change notification settings - Fork 19
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
react-native-edge-to-edge #28
Conversation
For me it would be perfect to get following metadata (from Android/C++):
or reliable Ofc I can do it by myself, but Im also facing a lot of edge cases. So single API for all would be the best! |
android/src/main/java/com/zoontek/rnedgetoedge/RNEdgeToEdgeModuleImpl.kt
Outdated
Show resolved
Hide resolved
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.
Dope stuff as usual! Left some thoughts. Hope they help
<item name="android:windowTranslucentStatus">true</item> | ||
<item name="android:windowTranslucentNavigation">true</item> | ||
<style name="Theme.EdgeToEdge" parent="Theme.EdgeToEdge.Common"> | ||
<item name="android:navigationBarColor">@color/systemBarDarkScrim</item> |
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 would like to suggest adding a variant with Theme.AppCompat.Light.NoActionBar
theme, as not all applications that have to be created are designed to support dark theme and dynamic switching.
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.
Something like this
Theme.EdgeToEdge
- default (current implementation)
Theme.EdgeToEdge.Light
- will be using Theme.AppCompat.Light.NoActionBar
theme under the hood
android/src/main/java/com/zoontek/rnedgetoedge/RNEdgeToEdgeModuleImpl.kt
Outdated
Show resolved
Hide resolved
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.
Looks good @zoontek
@@ -39,8 +41,8 @@ android { | |||
} | |||
defaultConfig { | |||
buildConfigField("boolean", "IS_NEW_ARCHITECTURE_ENABLED", isNewArchitectureEnabled().toString()) | |||
minSdkVersion safeExtGet("minSdkVersion", 21) | |||
targetSdkVersion safeExtGet("targetSdkVersion", 33) | |||
minSdkVersion safeExtGet("minSdkVersion", 23) |
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.
Heads up, 0.76 will have a minSdkVersion of 24
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.
@alanjhughes This library is compatible with RN 0.73+, that's why I keep minSdkVersion
to 23
for now.
No description provided.