-
-
Notifications
You must be signed in to change notification settings - Fork 521
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
fix(Android): fix build deprecations #2116
Conversation
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.
Just a single question, rest of the changes seems perfectly fine 🟢
@@ -62,7 +62,8 @@ class ScreenStackFragment : ScreenFragment, ScreenStackFragmentWrapper { | |||
|
|||
override fun setToolbarShadowHidden(hidden: Boolean) { | |||
if (isToolbarShadowHidden != hidden) { | |||
appBarLayout?.targetElevation = if (hidden) 0f else PixelUtil.toPixelFromDIP(4f) | |||
appBarLayout?.elevation = if (hidden) 0f else PixelUtil.toPixelFromDIP(4f) | |||
appBarLayout?.stateListAnimator = null |
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.
Why null
here, instead of leaving default?
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.
Without it the elevation is not set properly on initial render, I suppose the shadow is there by default so we have to reset it.
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.
Ok
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 think we can merge 🟢 Thank you!
@@ -80,7 +80,7 @@ class Screen(context: ReactContext?) : FabricEnabledViewGroup(context) { | |||
private fun updateScreenSizePaper(width: Int, height: Int) { | |||
val reactContext = context as ReactContext | |||
reactContext.runOnNativeModulesQueueThread( | |||
object : GuardedRunnable(reactContext) { | |||
object : GuardedRunnable(reactContext.exceptionHandler) { |
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 saw the warning you pasted in PR description, but I checked in source code & this is not deprecated 🤷🏻♂️
It's fine I guess
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.
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've found similar fix here: software-mansion/react-native-reanimated#1031
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.
@@ -62,7 +62,8 @@ class ScreenStackFragment : ScreenFragment, ScreenStackFragmentWrapper { | |||
|
|||
override fun setToolbarShadowHidden(hidden: Boolean) { | |||
if (isToolbarShadowHidden != hidden) { | |||
appBarLayout?.targetElevation = if (hidden) 0f else PixelUtil.toPixelFromDIP(4f) | |||
appBarLayout?.elevation = if (hidden) 0f else PixelUtil.toPixelFromDIP(4f) | |||
appBarLayout?.stateListAnimator = null |
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.
Ok
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.
LGTM! Just a small note from me 😄
@@ -82,7 +84,9 @@ class ScreenStackHeaderConfig(context: Context) : ViewGroup(context) { | |||
// we want to save the top inset before the status bar can be hidden, which would resolve in | |||
// inset being 0 | |||
if (headerTopInset == null) { | |||
headerTopInset = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M) | |||
headerTopInset = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.R) |
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.
Do you know if VERSION_CODES.R
is already defined on lower supported Android versions?
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 don't think it is, but it should be converted to a number during build time so it does not matter according to this thread: https://stackoverflow.com/questions/21874227/why-do-android-os-build-version-codes-work-on-older-platforms
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.
Yeah, but where does the guarantee that this symbol is available in build time come from?
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.
We have this guarantee only since August 2023.
I'm leaving link here for future git-blaming activities.
https://developer.android.com/google/play/requirements/target-sdk?hl=pl
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.
Actually we don't have this guarantee, cause this link is about target
. I guess we can reasonably safely assume that target
& compile
point to the same version 🤷🏻
I think they're popping because |
## Description This PR intents to fix header shadow not being hidden after navigating back to a screen. The bug was visible when using `headerTransparent: true` option. The origin of this bug is [This PR](#2116) fixing build depracations. Setting the `elevation` on appBarLayout requires resetting the `stateListAnimator` to work properly, as opossed to deprecated `targetElevation` used before. For more info visit: https://developer.android.com/reference/com/google/android/material/appbar/AppBarLayout#setTargetElevation(float) Fixes #2212 . ## Changes - added missing `appBarLayout?.stateListAnimator` reset. ## Screenshots / GIFs Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. ### Before ![Screenshot 2024-06-27 at 11 39 50](https://github.com/software-mansion/react-native-screens/assets/91994767/78c42cd3-4523-46f2-93ea-2aa4a6f4da4f) ### After ![Screenshot 2024-06-27 at 11 39 29](https://github.com/software-mansion/react-native-screens/assets/91994767/b0d32daa-ee98-438e-90cb-621bb6b13ad0) ## Test code and steps to reproduce 1. use `Test1097.tsx` repro 2. navigate into second or third screen 3. go back to the first screen ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
## Description This PR intents to fix header shadow not being hidden after navigating back to a screen. The bug was visible when using `headerTransparent: true` option. The origin of this bug is [This PR](#2116) fixing build depracations. Setting the `elevation` on appBarLayout requires resetting the `stateListAnimator` to work properly, as opossed to deprecated `targetElevation` used before. For more info visit: https://developer.android.com/reference/com/google/android/material/appbar/AppBarLayout#setTargetElevation(float) Fixes #2212 . ## Changes - added missing `appBarLayout?.stateListAnimator` reset. ## Screenshots / GIFs Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. ### Before ![Screenshot 2024-06-27 at 11 39 50](https://github.com/software-mansion/react-native-screens/assets/91994767/78c42cd3-4523-46f2-93ea-2aa4a6f4da4f) ### After ![Screenshot 2024-06-27 at 11 39 29](https://github.com/software-mansion/react-native-screens/assets/91994767/b0d32daa-ee98-438e-90cb-621bb6b13ad0) ## Test code and steps to reproduce 1. use `Test1097.tsx` repro 2. navigate into second or third screen 3. go back to the first screen ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
## Description This PR intents to fix some "Deprecated in Java" warnings when building on android. I decided not to suppress warnings manually when it is inescapable to avoid the warning. Fixes software-mansion#2018 ## Changes - replaced deprecated targetElavation with elevation - adjusted usage of porter duff color filter - calculating header top inset using recent method if android API >= 30 (the warning persists as the previous APIs still require deprecated method) - fixed guarded runnable deprecated warning - removed unused import ## Screenshots / GIFs ### Before ``` > Task :react-native-screens:compileDebugKotlin 'compileDebugJavaWithJavac' task (current target is 11) and 'compileDebugKotlin' task (current target is 17) jvm target compatibility should be set to the same Java version. Consider using JVM toolchain: https://kotl.in/gradle/jvm/toolchain w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/Screen.kt:83:22 'constructor GuardedRunnable(ReactContext!)' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt:65:27 'setter for targetElevation: Float' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt:128:27 'setter for targetElevation: Float' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.kt:86:34 'getter for systemWindowInsetTop: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.kt:231:37 'setColorFilter(Int, PorterDuff.Mode): Unit' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:63:22 'constructor GuardedRunnable(ReactContext!)' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:108:22 'constructor GuardedRunnable(ReactContext!)' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:135:47 'replaceSystemWindowInsets(Int, Int, Int, Int): WindowInsetsCompat' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:136:51 'getter for systemWindowInsetLeft: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:138:51 'getter for systemWindowInsetRight: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:139:51 'getter for systemWindowInsetBottom: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/SearchBarManager.kt:100:22 Parameter 'view' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/SearchBarManager.kt:100:43 Parameter 'placeholder' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/SearchBarView.kt:147:43 Parameter 'flag' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/events/HeaderHeightChangeEvent.kt:6:44 'RCTEventEmitter' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/paper/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt:11:42 Parameter 'width' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/paper/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt:11:54 Parameter 'height' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/paper/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt:11:67 Parameter 'headerHeight' is never used ``` ### After ``` > Task :react-native-screens:compileDebugKotlin 'compileDebugJavaWithJavac' task (current target is 11) and 'compileDebugKotlin' task (current target is 17) jvm target compatibility should be set to the same Java version. Consider using JVM toolchain: https://kotl.in/gradle/jvm/toolchain w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.kt:90:34 'getter for systemWindowInsetTop: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:135:47 'replaceSystemWindowInsets(Int, Int, Int, Int): WindowInsetsCompat' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:136:51 'getter for systemWindowInsetLeft: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:138:51 'getter for systemWindowInsetRight: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/ScreenWindowTraits.kt:139:51 'getter for systemWindowInsetBottom: Int' is deprecated. Deprecated in Java w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/SearchBarManager.kt:100:22 Parameter 'view' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/SearchBarManager.kt:100:43 Parameter 'placeholder' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/main/java/com/swmansion/rnscreens/SearchBarView.kt:147:43 Parameter 'flag' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/paper/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt:11:42 Parameter 'width' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/paper/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt:11:54 Parameter 'height' is never used w: file:///Users/alexduzy/Projects/react-native-screens/Example/node_modules/react-native-screens/android/src/paper/java/com/swmansion/rnscreens/FabricEnabledViewGroup.kt:11:67 Parameter 'headerHeight' is never used ``` ## Test code and steps to reproduce - build android app to compare deprecation warnings quantity - run the example app and see wether the header shadow, backButton color and top inset behave correctly ## Checklist - [ ] Ensured that CI passes
) ## Description This PR intents to fix header shadow not being hidden after navigating back to a screen. The bug was visible when using `headerTransparent: true` option. The origin of this bug is [This PR](software-mansion#2116) fixing build depracations. Setting the `elevation` on appBarLayout requires resetting the `stateListAnimator` to work properly, as opossed to deprecated `targetElevation` used before. For more info visit: https://developer.android.com/reference/com/google/android/material/appbar/AppBarLayout#setTargetElevation(float) Fixes software-mansion#2212 . ## Changes - added missing `appBarLayout?.stateListAnimator` reset. ## Screenshots / GIFs Here you can add screenshots / GIFs documenting your change. You can add before / after section if you're changing some behavior. ### Before ![Screenshot 2024-06-27 at 11 39 50](https://github.com/software-mansion/react-native-screens/assets/91994767/78c42cd3-4523-46f2-93ea-2aa4a6f4da4f) ### After ![Screenshot 2024-06-27 at 11 39 29](https://github.com/software-mansion/react-native-screens/assets/91994767/b0d32daa-ee98-438e-90cb-621bb6b13ad0) ## Test code and steps to reproduce 1. use `Test1097.tsx` repro 2. navigate into second or third screen 3. go back to the first screen ## Checklist - [x] Included code example that can be used to test this change - [x] Ensured that CI passes
Description
This PR intents to fix some "Deprecated in Java" warnings when building on android. I decided not to suppress warnings manually when it is inescapable to avoid the warning.
Fixes #2018
Changes
Screenshots / GIFs
Before
After
Test code and steps to reproduce
Checklist