Skip to content

Commit aa9aa73

Browse files
kkafarja1ns
authored andcommitted
fix(Android, Fabric): jumping content with native header (software-mansion#2169)
## Description This PR intents to solve the *eternal* issue with "jumping content" on Android & Fabric. The issue itself is present on every platform / RN architecture combination, however this PR scope is to solve situation only on Android + Fabric. Android + Paper, and iOS will be solved in separate PRs. > [!note] > These videos are recorded with `topInsetEnabled: false`, as this prop implementation causes another series of issues that will be handled separately. Here is before & after comparison (best way to see is to go frame-by-frame) | Before | After | |--------|--------| | <video width="320" height="240" controls src="https://github.com/software-mansion/react-native-screens/assets/50801299/e1e995b5-885b-4bd4-941a-57cdc6b321b2"></video> | <video width="320" height="240" controls src="https://github.com/software-mansion/react-native-screens/assets/50801299/6ca87888-0f05-4dfc-b6db-bfd08e3735b3"></video> | This will even work with irregular font sizes! ### Short issue genesis > [!note] > The flow described here below is a simplification, but should give you a good grasp on the issue. Basically during the very first Yoga layout, that happens on secondary thread, React layout mechanism has no knowledge of the header size (there isn't even a node representing the header at appropriate tree-level present in shadow tree), thus the `Screen` content is layouted with more available space that it has in reality. These dimensions are then send to UI thread, and propagated bottom-up (children before parents) and `Screen` contents do receive too high frame. Then, when `ScreenContainer` / `ScreenStack` does receive its frame from RN, it triggers a fragmentary pass of native layout using `CoordinatorLayout` (the layout stops at `Screen`), offsetting the `Screen` by just-computed-header-height on Y axis, and in consequence pushing some of the `Screen`'s contents off the screen (exactly a strip of header height). The situation is then salvaged by sending state update from `Screen` to React Native with new frame size. ### Implemented solution During the first Yoga layout pass (when there is not state from UI thread received yet) we utilise the fact that RN clones our ShadowNode & calls `adapt` on it. In the adapt method we call into JVM where we have set up a dummy view hierarchy with coordinator layout & header, we layout it & return result to C++ layer where we set bottom padding on the Screen so that its contents do have right amount of space. > [!important] > Downside of this solution is the fact, that the Yoga state / Shadow Tree still indicates that the `Screen`'s origin is at `(x, y) = (0, 0)` and it still will have wrong dimensions. Setting dummy dimension on `HeaderConfig` shadow node will improve situation only slightly, as the `Screen` will still have wrong origin, but it will have appropriate size immediately, **hence `Screen`'s state update might not trigger follow-up transaction**. Thus I'm thinking now that I will update the solution. ### Yet ~un~tested approaches * ~I want to try making custom descriptor for `ScreenStack`, and try to customise shadownode's layout method.~ <- tested this & I believe this will not work due to the fact, that `ShadowNode.layout` does not use `layoutContext.viewportOffset` at all (so we can not use this to offset our descendants). At the same time the `layout` method does not propagate layout metrics - they are extracted for each shadow node directly from it's yoga node and this process does not take into consideration parent's layout metrics. **However, if the `x, y` view origin coordinates determined by yoga are in parent node coordinate system** we can use `HeaderConfig` to ensure appropriate `Screens` size and at the same time set `frame.y` manually! ## Test code and steps to reproduce I've added `TestHeader` test case. It's best to run it with `FabricTestExample`, record it, and then see frame-by-frame that the content no longer jumps. ## Checklist - [x] Included code example that can be used to test this change - [ ] Ensured that CI passes
1 parent 46e71f4 commit aa9aa73

File tree

14 files changed

+697
-38
lines changed

14 files changed

+697
-38
lines changed

android/src/main/java/com/swmansion/rnscreens/RNScreensPackage.kt

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,15 +7,29 @@ import com.facebook.react.module.annotations.ReactModuleList
77
import com.facebook.react.module.model.ReactModuleInfo
88
import com.facebook.react.module.model.ReactModuleInfoProvider
99
import com.facebook.react.uimanager.ViewManager
10+
import com.swmansion.rnscreens.utils.ScreenDummyLayoutHelper
11+
1012

1113
@ReactModuleList(
1214
nativeModules = [
1315
ScreensModule::class
1416
]
1517
)
1618
class RNScreensPackage : TurboReactPackage() {
17-
override fun createViewManagers(reactContext: ReactApplicationContext) =
18-
listOf<ViewManager<*, *>>(
19+
// We just retain it here. This object helps us tackle jumping content when using native header.
20+
// See: https://github.com/software-mansion/react-native-screens/pull/2169
21+
private var screenDummyLayoutHelper: ScreenDummyLayoutHelper? = null
22+
23+
24+
override fun createViewManagers(reactContext: ReactApplicationContext): List<ViewManager<*, *>> {
25+
// This is the earliest we lay our hands on react context.
26+
// Moreover this is called before FabricUIManger has finished initializing, not to mention
27+
// installing its C++ bindings - so we are safe in terms of creating this helper
28+
// before RN starts creating shadow nodes.
29+
// See https://github.com/software-mansion/react-native-screens/pull/2169
30+
screenDummyLayoutHelper = ScreenDummyLayoutHelper(reactContext)
31+
32+
return listOf<ViewManager<*, *>>(
1933
ScreenContainerViewManager(),
2034
ScreenViewManager(),
2135
ModalScreenViewManager(),
@@ -24,6 +38,7 @@ class RNScreensPackage : TurboReactPackage() {
2438
ScreenStackHeaderSubviewManager(),
2539
SearchBarManager()
2640
)
41+
}
2742

2843
override fun getModule(
2944
s: String,
@@ -51,4 +66,8 @@ class RNScreensPackage : TurboReactPackage() {
5166
moduleInfos
5267
}
5368
}
69+
70+
companion object {
71+
const val TAG = "RNScreensPackage"
72+
}
5473
}

android/src/main/java/com/swmansion/rnscreens/Screen.kt

Lines changed: 36 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,8 @@ class Screen(context: ReactContext?) : FabricEnabledViewGroup(context) {
6666
val height = b - t
6767

6868
val headerHeight = calculateHeaderHeight()
69-
val totalHeight = headerHeight.first + headerHeight.second // action bar height + status bar height
69+
val totalHeight =
70+
headerHeight.first + headerHeight.second // action bar height + status bar height
7071
if (BuildConfig.IS_NEW_ARCHITECTURE_ENABLED) {
7172
updateScreenSizeFabric(width, height, totalHeight)
7273
} else {
@@ -171,7 +172,13 @@ class Screen(context: ReactContext?) : FabricEnabledViewGroup(context) {
171172
ScreenWindowTraits.applyDidSetStatusBarAppearance()
172173
}
173174
field = statusBarStyle
174-
fragmentWrapper?.let { ScreenWindowTraits.setStyle(this, it.tryGetActivity(), it.tryGetContext()) }
175+
fragmentWrapper?.let {
176+
ScreenWindowTraits.setStyle(
177+
this,
178+
it.tryGetActivity(),
179+
it.tryGetContext()
180+
)
181+
}
175182
}
176183

177184
var isStatusBarHidden: Boolean? = null
@@ -204,7 +211,13 @@ class Screen(context: ReactContext?) : FabricEnabledViewGroup(context) {
204211
ScreenWindowTraits.applyDidSetStatusBarAppearance()
205212
}
206213
field = statusBarColor
207-
fragmentWrapper?.let { ScreenWindowTraits.setColor(this, it.tryGetActivity(), it.tryGetContext()) }
214+
fragmentWrapper?.let {
215+
ScreenWindowTraits.setColor(
216+
this,
217+
it.tryGetActivity(),
218+
it.tryGetContext()
219+
)
220+
}
208221
}
209222

210223
var navigationBarColor: Int? = null
@@ -213,7 +226,12 @@ class Screen(context: ReactContext?) : FabricEnabledViewGroup(context) {
213226
ScreenWindowTraits.applyDidSetNavigationBarAppearance()
214227
}
215228
field = navigationBarColor
216-
fragmentWrapper?.let { ScreenWindowTraits.setNavigationBarColor(this, it.tryGetActivity()) }
229+
fragmentWrapper?.let {
230+
ScreenWindowTraits.setNavigationBarColor(
231+
this,
232+
it.tryGetActivity()
233+
)
234+
}
217235
}
218236

219237
var isNavigationBarTranslucent: Boolean? = null
@@ -248,20 +266,23 @@ class Screen(context: ReactContext?) : FabricEnabledViewGroup(context) {
248266

249267
private fun calculateHeaderHeight(): Pair<Double, Double> {
250268
val actionBarTv = TypedValue()
251-
val resolvedActionBarSize = context.theme.resolveAttribute(android.R.attr.actionBarSize, actionBarTv, true)
269+
val resolvedActionBarSize =
270+
context.theme.resolveAttribute(android.R.attr.actionBarSize, actionBarTv, true)
252271

253272
// Check if it's possible to get an attribute from theme context and assign a value from it.
254273
// Otherwise, the default value will be returned.
255-
val actionBarHeight = TypedValue.complexToDimensionPixelSize(actionBarTv.data, resources.displayMetrics)
256-
.takeIf { resolvedActionBarSize && headerConfig?.isHeaderHidden != true && headerConfig?.isHeaderTranslucent != true }
257-
?.let { PixelUtil.toDIPFromPixel(it.toFloat()).toDouble() } ?: 0.0
258-
259-
val statusBarHeight = context.resources.getIdentifier("status_bar_height", "dimen", "android")
260-
// Count only status bar when action bar is visible and status bar is not hidden
261-
.takeIf { it > 0 && isStatusBarHidden != true && actionBarHeight > 0 }
262-
?.let { (context.resources::getDimensionPixelSize)(it) }
263-
?.let { PixelUtil.toDIPFromPixel(it.toFloat()).toDouble() }
264-
?: 0.0
274+
val actionBarHeight =
275+
TypedValue.complexToDimensionPixelSize(actionBarTv.data, resources.displayMetrics)
276+
.takeIf { resolvedActionBarSize && headerConfig?.isHeaderHidden != true && headerConfig?.isHeaderTranslucent != true }
277+
?.let { PixelUtil.toDIPFromPixel(it.toFloat()).toDouble() } ?: 0.0
278+
279+
val statusBarHeight =
280+
context.resources.getIdentifier("status_bar_height", "dimen", "android")
281+
// Count only status bar when action bar is visible and status bar is not hidden
282+
.takeIf { it > 0 && isStatusBarHidden != true && actionBarHeight > 0 }
283+
?.let { (context.resources::getDimensionPixelSize)(it) }
284+
?.let { PixelUtil.toDIPFromPixel(it.toFloat()).toDouble() }
285+
?: 0.0
265286

266287
return actionBarHeight to statusBarHeight
267288
}

android/src/main/java/com/swmansion/rnscreens/ScreenStackHeaderConfig.kt

Lines changed: 16 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import com.swmansion.rnscreens.events.HeaderDetachedEvent
2626
class ScreenStackHeaderConfig(context: Context) : ViewGroup(context) {
2727
private val configSubviews = ArrayList<ScreenStackHeaderSubview>(3)
2828
val toolbar: CustomToolbar
29-
var isHeaderHidden = false // named this way to avoid conflict with platform's isHidden
29+
var isHeaderHidden = false // named this way to avoid conflict with platform's isHidden
3030
var isHeaderTranslucent = false // named this way to avoid conflict with platform's isTranslucent
3131
private var headerTopInset: Int? = null
3232
private var title: String? = null
@@ -210,7 +210,7 @@ class ScreenStackHeaderConfig(context: Context) : ViewGroup(context) {
210210
toolbar.contentInsetStartWithNavigation = 0
211211
}
212212

213-
val titleTextView = titleTextView
213+
val titleTextView = findTitleTextViewInToolbar(toolbar)
214214
if (titleColor != 0) {
215215
toolbar.setTitleTextColor(titleColor)
216216
}
@@ -309,19 +309,6 @@ class ScreenStackHeaderConfig(context: Context) : ViewGroup(context) {
309309
maybeUpdate()
310310
}
311311

312-
private val titleTextView: TextView?
313-
get() {
314-
for (i in 0 until toolbar.childCount) {
315-
val view = toolbar.getChildAt(i)
316-
if (view is TextView) {
317-
if (view.text == toolbar.title) {
318-
return view
319-
}
320-
}
321-
}
322-
return null
323-
}
324-
325312
fun setTitle(title: String?) {
326313
this.title = title
327314
}
@@ -401,4 +388,18 @@ class ScreenStackHeaderConfig(context: Context) : ViewGroup(context) {
401388
}
402389
toolbar.clipChildren = false
403390
}
391+
392+
companion object {
393+
fun findTitleTextViewInToolbar(toolbar: Toolbar): TextView? {
394+
for (i in 0 until toolbar.childCount) {
395+
val view = toolbar.getChildAt(i)
396+
if (view is TextView) {
397+
if (view.text == toolbar.title) {
398+
return view
399+
}
400+
}
401+
}
402+
return null
403+
}
404+
}
404405
}
Lines changed: 189 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,189 @@
1+
package com.swmansion.rnscreens.utils
2+
3+
import android.app.Activity
4+
import android.util.Log
5+
import android.view.View
6+
import androidx.appcompat.widget.Toolbar
7+
import androidx.coordinatorlayout.widget.CoordinatorLayout
8+
import com.facebook.react.bridge.ReactApplicationContext
9+
import com.facebook.react.uimanager.PixelUtil
10+
import com.google.android.material.appbar.AppBarLayout
11+
import com.swmansion.rnscreens.ScreenStackHeaderConfig
12+
import java.lang.ref.WeakReference
13+
14+
/**
15+
* This class provides methods to create dummy layout (that mimics Screen setup), and to compute
16+
* expected header height. It is meant to be accessed from C++ layer via JNI.
17+
* See https://github.com/software-mansion/react-native-screens/pull/2169
18+
* for more detailed description of the issue this code solves.
19+
*/
20+
internal class ScreenDummyLayoutHelper(reactContext: ReactApplicationContext) {
21+
// The state required to compute header dimensions. We want this on instance rather than on class
22+
// for context access & being tied to instance lifetime.
23+
private lateinit var coordinatorLayout: CoordinatorLayout
24+
private lateinit var appBarLayout: AppBarLayout
25+
private lateinit var dummyContentView: View
26+
private lateinit var toolbar: Toolbar
27+
private var defaultFontSize: Float = 0f
28+
private var defaultContentInsetStartWithNavigation: Int = 0
29+
30+
// LRU with size 1
31+
private var cache: CacheEntry = CacheEntry.EMPTY
32+
33+
// We do not want to be responsible for the context lifecycle. If it's null, we're fine.
34+
// This same context is being passed down to our view components so it is destroyed
35+
// only if our views also are.
36+
private var reactContextRef: WeakReference<ReactApplicationContext> = WeakReference(reactContext)
37+
38+
init {
39+
40+
// We load the library so that we are able to communicate with our C++ code (descriptor & shadow nodes).
41+
// Basically we leak this object to C++, as its lifecycle should span throughout whole application
42+
// lifecycle anyway.
43+
try {
44+
System.loadLibrary(LIBRARY_NAME)
45+
} catch (e: UnsatisfiedLinkError) {
46+
Log.w(TAG, "Failed to load $LIBRARY_NAME")
47+
}
48+
49+
WEAK_INSTANCE = WeakReference(this)
50+
ensureDummyLayoutWithHeader(reactContext)
51+
}
52+
53+
/**
54+
* Initializes dummy view hierarchy with CoordinatorLayout, AppBarLayout and dummy View.
55+
* We utilize this to compute header height (app bar layout height) from C++ layer when its needed.
56+
*/
57+
private fun ensureDummyLayoutWithHeader(reactContext: ReactApplicationContext) {
58+
if (::coordinatorLayout.isInitialized) {
59+
return
60+
}
61+
62+
// We need to use activity here, as react context does not have theme attributes required by
63+
// AppBarLayout attached leading to crash.
64+
val contextWithTheme =
65+
requireNotNull(reactContext.currentActivity) { "[RNScreens] Attempt to use context detached from activity" }
66+
67+
coordinatorLayout = CoordinatorLayout(contextWithTheme)
68+
69+
appBarLayout = AppBarLayout(contextWithTheme).apply {
70+
layoutParams = CoordinatorLayout.LayoutParams(
71+
CoordinatorLayout.LayoutParams.MATCH_PARENT,
72+
CoordinatorLayout.LayoutParams.WRAP_CONTENT,
73+
)
74+
}
75+
76+
toolbar = Toolbar(contextWithTheme).apply {
77+
title = DEFAULT_HEADER_TITLE
78+
layoutParams = AppBarLayout.LayoutParams(
79+
AppBarLayout.LayoutParams.MATCH_PARENT,
80+
AppBarLayout.LayoutParams.WRAP_CONTENT
81+
).apply { scrollFlags = 0 }
82+
}
83+
84+
// We know the title text view will be there, cause we've just set title.
85+
defaultFontSize = ScreenStackHeaderConfig.findTitleTextViewInToolbar(toolbar)!!.textSize
86+
defaultContentInsetStartWithNavigation = toolbar.contentInsetStartWithNavigation
87+
88+
appBarLayout.addView(toolbar)
89+
90+
dummyContentView = View(contextWithTheme).apply {
91+
layoutParams = CoordinatorLayout.LayoutParams(
92+
CoordinatorLayout.LayoutParams.MATCH_PARENT,
93+
CoordinatorLayout.LayoutParams.MATCH_PARENT
94+
)
95+
}
96+
97+
coordinatorLayout.apply {
98+
addView(appBarLayout)
99+
addView(dummyContentView)
100+
}
101+
}
102+
103+
/**
104+
* Triggers layout pass on dummy view hierarchy, taking into consideration selected
105+
* ScreenStackHeaderConfig props that might have impact on final header height.
106+
*
107+
* @param fontSize font size value as passed from JS
108+
* @return header height in dp as consumed by Yoga
109+
*/
110+
private fun computeDummyLayout(fontSize: Int, isTitleEmpty: Boolean): Float {
111+
if (!::coordinatorLayout.isInitialized) {
112+
Log.e(TAG, "[RNScreens] Attempt to access dummy view hierarchy before it is initialized")
113+
return 0.0f
114+
}
115+
116+
if (cache.hasKey(CacheKey(fontSize, isTitleEmpty))) {
117+
return cache.headerHeight
118+
}
119+
120+
val topLevelDecorView = requireActivity().window.decorView
121+
122+
// These dimensions are not accurate, as they do include status bar & navigation bar, however
123+
// it is ok for our purposes.
124+
val decorViewWidth = topLevelDecorView.width
125+
val decorViewHeight = topLevelDecorView.height
126+
127+
val widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(decorViewWidth, View.MeasureSpec.EXACTLY)
128+
val heightMeasureSpec = View.MeasureSpec.makeMeasureSpec(decorViewHeight, View.MeasureSpec.EXACTLY)
129+
130+
if (isTitleEmpty) {
131+
toolbar.title = ""
132+
toolbar.contentInsetStartWithNavigation = 0
133+
} else {
134+
toolbar.title = DEFAULT_HEADER_TITLE
135+
toolbar.contentInsetStartWithNavigation = defaultContentInsetStartWithNavigation
136+
}
137+
138+
val textView = ScreenStackHeaderConfig.findTitleTextViewInToolbar(toolbar)
139+
textView?.textSize = if (fontSize != FONT_SIZE_UNSET) fontSize.toFloat() else defaultFontSize
140+
141+
coordinatorLayout.measure(widthMeasureSpec, heightMeasureSpec)
142+
143+
// It seems that measure pass would be enough, however I'm not certain whether there are no
144+
// scenarios when layout violates measured dimensions.
145+
coordinatorLayout.layout(0, 0, decorViewWidth, decorViewHeight)
146+
147+
val headerHeight = PixelUtil.toDIPFromPixel(appBarLayout.height.toFloat())
148+
cache = CacheEntry(CacheKey(fontSize, isTitleEmpty), headerHeight)
149+
return headerHeight
150+
}
151+
152+
private fun requireReactContext(): ReactApplicationContext {
153+
return requireNotNull(reactContextRef.get()) { "[RNScreens] Attempt to require missing react context" }
154+
}
155+
156+
private fun requireActivity(): Activity {
157+
return requireNotNull(requireReactContext().currentActivity) { "[RNScreens] Attempt to use context detached from activity" }
158+
}
159+
160+
companion object {
161+
const val TAG = "ScreenDummyLayoutHelper"
162+
163+
const val LIBRARY_NAME = "react_codegen_rnscreens"
164+
165+
const val FONT_SIZE_UNSET = -1
166+
167+
private const val DEFAULT_HEADER_TITLE: String = "FontSize123!#$"
168+
169+
// We access this field from C++ layer, through `getInstance` method.
170+
// We don't care what instance we get access to as long as it has initialized
171+
// dummy view hierarchy.
172+
private var WEAK_INSTANCE = WeakReference<ScreenDummyLayoutHelper>(null)
173+
174+
@JvmStatic
175+
fun getInstance(): ScreenDummyLayoutHelper? {
176+
return WEAK_INSTANCE.get()
177+
}
178+
}
179+
}
180+
181+
private data class CacheKey(val fontSize: Int, val isTitleEmpty: Boolean)
182+
183+
private class CacheEntry(val cacheKey: CacheKey, val headerHeight: Float) {
184+
fun hasKey(key: CacheKey) = cacheKey.fontSize != Int.MIN_VALUE && cacheKey == key
185+
186+
companion object {
187+
val EMPTY = CacheEntry(CacheKey(Int.MIN_VALUE, false), 0f)
188+
}
189+
}

0 commit comments

Comments
 (0)