Skip to content

Commit

Permalink
fix(Android): prevent crash when we can not insert any subviews into …
Browse files Browse the repository at this point in the history
…child (#2531)

## Description

Attempt to patch changes from
#2495. It
improved the situation in most of the cases
but caused crashes in cases where we tried to insert subviews into
viewgroups that have any kind of assertions on their child count, e.g.
`NestedScrollView` or `ViewPager`.

Fixes #2529
Supersedes #2527 

## Changes

In this PR I do two things:

* apply the workaround only to views that could actually have clipped
subviews
* catch any kind of exception thrown on view insertion; if one is thrown
we stop to add subviews for given child

## Test code and steps to reproduce

Test2282 works in both configurations (complex screen with nested
flatlists + my simple reproduction of the issue)

## Checklist

- [ ] Included code example that can be used to test this change
- [ ] Updated TS types
- [ ] Updated documentation: <!-- For adding new props to native-stack
-->
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/guides/GUIDE_FOR_LIBRARY_AUTHORS.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/native-stack/README.md
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/types.tsx
- [ ]
https://github.com/software-mansion/react-native-screens/blob/main/src/native-stack/types.tsx
- [ ] Ensured that CI passes
  • Loading branch information
kkafar authored Nov 21, 2024
1 parent b58f13a commit 6aaf56b
Showing 1 changed file with 20 additions and 3 deletions.
23 changes: 20 additions & 3 deletions android/src/main/java/com/swmansion/rnscreens/Screen.kt
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import androidx.swiperefreshlayout.widget.SwipeRefreshLayout
import com.facebook.react.bridge.GuardedRunnable
import com.facebook.react.bridge.ReactContext
import com.facebook.react.uimanager.PixelUtil
import com.facebook.react.uimanager.ReactClippingViewGroup
import com.facebook.react.uimanager.UIManagerHelper
import com.facebook.react.uimanager.UIManagerModule
import com.facebook.react.uimanager.events.EventDispatcher
Expand Down Expand Up @@ -383,6 +384,7 @@ class Screen(
parent?.let {
for (i in 0 until it.childCount) {
val child = it.getChildAt(i)

if (parent is SwipeRefreshLayout && child is ImageView) {
// SwipeRefreshLayout class which has CircleImageView as a child,
// does not handle `startViewTransition` properly.
Expand All @@ -394,20 +396,35 @@ class Screen(
} else {
child?.let { view -> it.startViewTransition(view) }
}

if (child is ScreenStackHeaderConfig) {
// we want to start transition on children of the toolbar too,
// which is not a child of ScreenStackHeaderConfig
startTransitionRecursive(child.toolbar)
}

if (child is ViewGroup) {
// The children are miscounted when there's removeClippedSubviews prop
// set to true (which is the default for FlatLists).
// Unless the child is a ScrollView it's safe to assume that it's true
// and add a simple view for each possibly clipped item to make it work as expected.
// See https://github.com/software-mansion/react-native-screens/pull/2495
if (child !is ReactScrollView && child !is ReactHorizontalScrollView) {
for (j in 0 until child.childCount) {
child.addView(View(context))

if (child is ReactClippingViewGroup &&
child.removeClippedSubviews &&
child !is ReactScrollView &&
child !is ReactHorizontalScrollView
) {
// We need to workaround the issue until our changes land in core.
// Some views do not accept any children or have set amount and they throw
// when we want to brute-forcefully manipulate that.
// Is this ugly? Very. Do we have better option before changes land in core?
// I'm not aware of any.
try {
for (j in 0 until child.childCount) {
child.addView(View(context))
}
} catch (_: Exception) {
}
}
startTransitionRecursive(child)
Expand Down

0 comments on commit 6aaf56b

Please sign in to comment.