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.

Sign up for GitHub

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

Jump to bottom

fix(Android): fix build deprecations #2116

Merged
merged 5 commits into from
May 6, 2024
Merged

fix(Android): fix build deprecations #2116

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
1c8db08
avoid using deprecated systemWindowInsetTop
alduzy Apr 29, 2024
4c36950
replaced deprectaed targetElavation with elevation
alduzy Apr 29, 2024
f1b34fe
color filter deprecation fix
alduzy Apr 29, 2024
b47f95f
removed unused deprecated import
alduzy Apr 29, 2024
d6d68e9
fixed guarded runnable deprecated warning
alduzy Apr 30, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

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?

Copy link
Member Author

@alduzy alduzy Apr 30, 2024 •
edited
Loading

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.

Copy link
Member

Choose a reason for hiding this comment

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

Ok

isToolbarShadowHidden = hidden
}
}
Expand Down Expand Up @@ -125,7 +126,7 @@ class ScreenStackFragment : ScreenFragment, ScreenStackFragmentWrapper {

view?.addView(appBarLayout)
if (isToolbarShadowHidden) {
appBarLayout?.targetElevation = 0f
appBarLayout?.elevation = 0f
}
toolbar?.let { appBarLayout?.addView(recycleView(it)) }
setHasOptionsMenu(true)
Expand Down
8 changes: 6 additions & 2 deletions android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,14 @@ package com.swmansion.rnscreens

import android.content.Context
import android.graphics.PorterDuff
import android.graphics.PorterDuffColorFilter
import android.os.Build
import android.text.TextUtils
import android.util.TypedValue
import android.view.Gravity
import android.view.View.OnClickListener
import android.view.ViewGroup
import android.view.WindowInsets
import android.widget.ImageView
import android.widget.TextView
import androidx.appcompat.app.AppCompatActivity
Expand Down Expand Up @@ -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)
Copy link
Member

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?

Copy link
Member Author

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

Copy link
Member

@kkafar kkafar May 6, 2024 •
edited
Loading

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?

Copy link
Member

@kkafar kkafar May 6, 2024 •
edited
Loading

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

Copy link
Member

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 🤷🏻

rootWindowInsets.getInsets(WindowInsets.Type.systemBars()).top
else if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.M)
rootWindowInsets.systemWindowInsetTop
else
// Hacky fallback for old android. Before Marshmallow, the status bar height was always 25
Expand Down Expand Up @@ -228,7 +232,7 @@ class ScreenStackHeaderConfig(context: Context) : ViewGroup(context) {

// color
if (tintColor != 0) {
toolbar.navigationIcon?.setColorFilter(tintColor, PorterDuff.Mode.SRC_ATOP)
toolbar.navigationIcon?.colorFilter = PorterDuffColorFilter(tintColor, PorterDuff.Mode.SRC_ATOP)
}

// subviews
Expand Down
1 change: 0 additions & 1 deletion android/src/main/java/com/swmansion/rnscreens/events/HeaderHeightChangeEvent.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package com.swmansion.rnscreens.events
import com.facebook.react.bridge.Arguments
import com.facebook.react.bridge.WritableMap
import com.facebook.react.uimanager.events.Event
import com.facebook.react.uimanager.events.RCTEventEmitter

class HeaderHeightChangeEvent(
surfaceId: Int,
Expand Down
Loading