-
-
Notifications
You must be signed in to change notification settings - Fork 521
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
fix(Android, Fabric): jumping content with native header (#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
- Loading branch information
Showing
14 changed files
with
697 additions
and
38 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
189 changes: 189 additions & 0 deletions
189
android/src/main/java/com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper.kt
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,189 @@ | ||
| package com.swmansion.rnscreens.utils | ||
|
|
||
| import android.app.Activity | ||
| import android.util.Log | ||
| import android.view.View | ||
| import androidx.appcompat.widget.Toolbar | ||
| import androidx.coordinatorlayout.widget.CoordinatorLayout | ||
| import com.facebook.react.bridge.ReactApplicationContext | ||
| import com.facebook.react.uimanager.PixelUtil | ||
| import com.google.android.material.appbar.AppBarLayout | ||
| import com.swmansion.rnscreens.ScreenStackHeaderConfig | ||
| import java.lang.ref.WeakReference | ||
|
|
||
| /** | ||
| * This class provides methods to create dummy layout (that mimics Screen setup), and to compute | ||
| * expected header height. It is meant to be accessed from C++ layer via JNI. | ||
| * See https://github.com/software-mansion/react-native-screens/pull/2169 | ||
| * for more detailed description of the issue this code solves. | ||
| */ | ||
| internal class ScreenDummyLayoutHelper(reactContext: ReactApplicationContext) { | ||
| // The state required to compute header dimensions. We want this on instance rather than on class | ||
| // for context access & being tied to instance lifetime. | ||
| private lateinit var coordinatorLayout: CoordinatorLayout | ||
| private lateinit var appBarLayout: AppBarLayout | ||
| private lateinit var dummyContentView: View | ||
| private lateinit var toolbar: Toolbar | ||
| private var defaultFontSize: Float = 0f | ||
| private var defaultContentInsetStartWithNavigation: Int = 0 | ||
|
|
||
| // LRU with size 1 | ||
| private var cache: CacheEntry = CacheEntry.EMPTY | ||
|
|
||
| // We do not want to be responsible for the context lifecycle. If it's null, we're fine. | ||
| // This same context is being passed down to our view components so it is destroyed | ||
| // only if our views also are. | ||
| private var reactContextRef: WeakReference<ReactApplicationContext> = WeakReference(reactContext) | ||
|
|
||
| init { | ||
|
|
||
| // We load the library so that we are able to communicate with our C++ code (descriptor & shadow nodes). | ||
| // Basically we leak this object to C++, as its lifecycle should span throughout whole application | ||
| // lifecycle anyway. | ||
| try { | ||
| System.loadLibrary(LIBRARY_NAME) | ||
| } catch (e: UnsatisfiedLinkError) { | ||
| Log.w(TAG, "Failed to load $LIBRARY_NAME") | ||
| } | ||
|
|
||
| WEAK_INSTANCE = WeakReference(this) | ||
| ensureDummyLayoutWithHeader(reactContext) | ||
| } | ||
|
|
||
| /** | ||
| * Initializes dummy view hierarchy with CoordinatorLayout, AppBarLayout and dummy View. | ||
| * We utilize this to compute header height (app bar layout height) from C++ layer when its needed. | ||
| */ | ||
| private fun ensureDummyLayoutWithHeader(reactContext: ReactApplicationContext) { | ||
| if (::coordinatorLayout.isInitialized) { | ||
| return | ||
| } | ||
|
|
||
| // We need to use activity here, as react context does not have theme attributes required by | ||
| // AppBarLayout attached leading to crash. | ||
| val contextWithTheme = | ||
| requireNotNull(reactContext.currentActivity) { "[RNScreens] Attempt to use context detached from activity" } | ||
|
|
||
| coordinatorLayout = CoordinatorLayout(contextWithTheme) | ||
|
|
||
| appBarLayout = AppBarLayout(contextWithTheme).apply { | ||
| layoutParams = CoordinatorLayout.LayoutParams( | ||
| CoordinatorLayout.LayoutParams.MATCH_PARENT, | ||
| CoordinatorLayout.LayoutParams.WRAP_CONTENT, | ||
| ) | ||
| } | ||
|
|
||
| toolbar = Toolbar(contextWithTheme).apply { | ||
| title = DEFAULT_HEADER_TITLE | ||
| layoutParams = AppBarLayout.LayoutParams( | ||
| AppBarLayout.LayoutParams.MATCH_PARENT, | ||
| AppBarLayout.LayoutParams.WRAP_CONTENT | ||
| ).apply { scrollFlags = 0 } | ||
| } | ||
|
|
||
| // We know the title text view will be there, cause we've just set title. | ||
| defaultFontSize = ScreenStackHeaderConfig.findTitleTextViewInToolbar(toolbar)!!.textSize | ||
| defaultContentInsetStartWithNavigation = toolbar.contentInsetStartWithNavigation | ||
|
|
||
| appBarLayout.addView(toolbar) | ||
|
|
||
| dummyContentView = View(contextWithTheme).apply { | ||
| layoutParams = CoordinatorLayout.LayoutParams( | ||
| CoordinatorLayout.LayoutParams.MATCH_PARENT, | ||
| CoordinatorLayout.LayoutParams.MATCH_PARENT | ||
| ) | ||
| } | ||
|
|
||
| coordinatorLayout.apply { | ||
| addView(appBarLayout) | ||
| addView(dummyContentView) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Triggers layout pass on dummy view hierarchy, taking into consideration selected | ||
| * ScreenStackHeaderConfig props that might have impact on final header height. | ||
| * | ||
| * @param fontSize font size value as passed from JS | ||
| * @return header height in dp as consumed by Yoga | ||
| */ | ||
| private fun computeDummyLayout(fontSize: Int, isTitleEmpty: Boolean): Float { | ||
| if (!::coordinatorLayout.isInitialized) { | ||
| Log.e(TAG, "[RNScreens] Attempt to access dummy view hierarchy before it is initialized") | ||
| return 0.0f | ||
| } | ||
|
|
||
| if (cache.hasKey(CacheKey(fontSize, isTitleEmpty))) { | ||
| return cache.headerHeight | ||
| } | ||
|
|
||
| val topLevelDecorView = requireActivity().window.decorView | ||
|
|
||
| // These dimensions are not accurate, as they do include status bar & navigation bar, however | ||
| // it is ok for our purposes. | ||
| val decorViewWidth = topLevelDecorView.width | ||
| val decorViewHeight = topLevelDecorView.height | ||
|
|
||
| val widthMeasureSpec = View.MeasureSpec.makeMeasureSpec(decorViewWidth, View.MeasureSpec.EXACTLY) | ||
| val heightMeasureSpec = View.MeasureSpec.makeMeasureSpec(decorViewHeight, View.MeasureSpec.EXACTLY) | ||
|
|
||
| if (isTitleEmpty) { | ||
| toolbar.title = "" | ||
| toolbar.contentInsetStartWithNavigation = 0 | ||
| } else { | ||
| toolbar.title = DEFAULT_HEADER_TITLE | ||
| toolbar.contentInsetStartWithNavigation = defaultContentInsetStartWithNavigation | ||
| } | ||
|
|
||
| val textView = ScreenStackHeaderConfig.findTitleTextViewInToolbar(toolbar) | ||
| textView?.textSize = if (fontSize != FONT_SIZE_UNSET) fontSize.toFloat() else defaultFontSize | ||
|
|
||
| coordinatorLayout.measure(widthMeasureSpec, heightMeasureSpec) | ||
|
|
||
| // It seems that measure pass would be enough, however I'm not certain whether there are no | ||
| // scenarios when layout violates measured dimensions. | ||
| coordinatorLayout.layout(0, 0, decorViewWidth, decorViewHeight) | ||
|
|
||
| val headerHeight = PixelUtil.toDIPFromPixel(appBarLayout.height.toFloat()) | ||
| cache = CacheEntry(CacheKey(fontSize, isTitleEmpty), headerHeight) | ||
| return headerHeight | ||
| } | ||
|
|
||
| private fun requireReactContext(): ReactApplicationContext { | ||
| return requireNotNull(reactContextRef.get()) { "[RNScreens] Attempt to require missing react context" } | ||
| } | ||
|
|
||
| private fun requireActivity(): Activity { | ||
| return requireNotNull(requireReactContext().currentActivity) { "[RNScreens] Attempt to use context detached from activity" } | ||
| } | ||
|
|
||
| companion object { | ||
| const val TAG = "ScreenDummyLayoutHelper" | ||
|
|
||
| const val LIBRARY_NAME = "react_codegen_rnscreens" | ||
|
|
||
| const val FONT_SIZE_UNSET = -1 | ||
|
|
||
| private const val DEFAULT_HEADER_TITLE: String = "FontSize123!#$" | ||
|
|
||
| // We access this field from C++ layer, through `getInstance` method. | ||
| // We don't care what instance we get access to as long as it has initialized | ||
| // dummy view hierarchy. | ||
| private var WEAK_INSTANCE = WeakReference<ScreenDummyLayoutHelper>(null) | ||
|
|
||
| @JvmStatic | ||
| fun getInstance(): ScreenDummyLayoutHelper? { | ||
| return WEAK_INSTANCE.get() | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private data class CacheKey(val fontSize: Int, val isTitleEmpty: Boolean) | ||
|
|
||
| private class CacheEntry(val cacheKey: CacheKey, val headerHeight: Float) { | ||
| fun hasKey(key: CacheKey) = cacheKey.fontSize != Int.MIN_VALUE && cacheKey == key | ||
|
|
||
| companion object { | ||
| val EMPTY = CacheEntry(CacheKey(Int.MIN_VALUE, false), 0f) | ||
| } | ||
| } |
Oops, something went wrong.