From 3cc723e2bd332ab8fce0e43989248783d9e85709 Mon Sep 17 00:00:00 2001 From: Kacper Kafara Date: Fri, 12 Jun 2026 16:12:10 +0200 Subject: [PATCH] Fix issue with BottomNavigationView content jump on enter animation This is a small commit, however it introduces profound changes and is based on delicate interactions between our components and the framework. The intention here is to fix an issue, where the contents of a `BottomNavigationView`, when nested inside a `ScreenStack` (stack v4) would jump on enter animation, the activity indicator view would appear mid transition etc. This very issue is fixed by swapping usage of `postFrameCallback` with `post` in `TabsHost`. Let's explain first what is the difference between the two. `ReactChoreographer` is a `Choreographer.FrameCallback` and registers itself as the callback on the actual `Choreographer` instance. `Choreographer`'s frame callbacks are triggered on VSync signal from the platform's renderer, by posting an asynchronous message to the main thread's message queue. This means, that when we add a callback to the `ReactChoreographer` we have a guarantee to be executed before the next frame is drawn, because the view traversals (measure, layout, draw) are executed after user-added frame callbacks. This has one big drawback - the layout we schedule with `postFrameCallback` will be executed BEFORE system dispatches insets, therefore the result of the layout calculation won't take them into consideration and effectively won't be correct. If we draw a frame before we recompute the layout again with insets, we will observe a `BottomNavigationView` height jump. The solution here is usage of the `post` to simply defer the layout computation until the insets are dispatched. Do we have a guarantee that the insets will be dispatched before our `post` callback is executed? Yes, provided that before we call `post` we (or anybody else) request inset dispatch from the hierarchy root (`BottomNavigationView` does this for us). When insets are requested, the `ViewRootImpl` adds a *barrier message* to the message queue, which naturally will be processed before our `post` callback, and will block processing of subsequent *regular messages* until vsync comes and traversals are executed. Okay, so the problem of jumping tab bar height is fixed, but there is still another problem with *activity indicator* (a round view that emphasizes the currently selected item icon). THe problem is that the `NavigationBarItemView` instances are not created and added to `NavigationBarMenuView` until we create & add items. This is done in `TabsContainer.performContainerUpdate`, just before we trigger a fragment transaction adding the selected tab, in turn, this happens synchronously after (sometimes before) we schedule the layout with `post`. That would be fine if the *activity indicator* had been laid out synchronously with the rest of the `BottomNavigationView` contents, but its not the case. The `NavigationBarItemView` schedules with `post` a callback that updates the *activity indicator* **layout params** (not the frame!), which triggers `parent.requestLayout()` and effectively schedules yet another `post` callback from `TabsHost` with the layout callback. Therefore it seems, that the activity indicator would appear after a couple frames of the transition have already been drawn, but it is not the case. **I'm not sure why**, but the activity indicator appears right now immediately on the very first animation frame in the right place and has good dimensions. I'm gonna let this be just because it works, but it could definitely be investigated further (maybe it takes a couple of frames to start the transition? Maybe it's `SafeAreaView` `ViewTreeObserver.OnPreDrawListener` that blocks the first frame from being drawn until the layout is correct? Not sure). Okay, so the tab bar now seems to be working correctly, but we have created yet another problem. This time we've broken the exit transition of the screen we're pushed on top of. Stack v4 uses the *Animation API* to animate the screens. When, in result of a *fragment transaction* fragment's view will be removed from the view hierarchy, the framework will mark the view as disappearing by calling `screenStack.startViewTransition(child)` and later observing that the `child` is detached from the hierarchy. This allows the disappearing view to be still drawn during the transition. It has to call `endViewTransition` at some point. To ensure it's called "at the right moment", [just after it starts the view transition it also schedules an accompanying `Animation` on the view](https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:fragment/fragment/src/main/java/androidx/fragment/app/DefaultSpecialEffectsController.kt;l=562-591;drc=c53e65302e39ba824260a5b9e0831d872aa786d1). This is a subclass of `Animation`: `FragmentAnim.EndViewTransitionAnimation`. It ends the transition on animation end OR after [it detects that it has to been queried for transformation between two *post callbacks*.](https://cs.android.com/androidx/platform/frameworks/support/+/androidx-main:fragment/fragment/src/main/java/androidx/fragment/app/FragmentAnim.java;l=226-268;drc=c53e65302e39ba824260a5b9e0831d872aa786d1). This way it determines that the animation is "no longer active" and calls the `endViewTranstion`. This "heuristic" is completely wrong though, in case ANY `ViewTreeObserver.OnPreDrawListener` blocks the draw process by returning `false` from `onPreDraw`. In particular our `SafeAreaView` does exactly that, because it needs to wait until the layout is correct and insets are applied before it allows the first frame to be drawn. This means that the transition will be ended prematurely, right after the first frame is drawn, which causes the screen to simply disappear and therefore a flash. To workaround the interaction described above I've added a hack in `ScreensCoordinatorLayout` & `ScreenStack` that blocks the premature calls to `endViewTransition` and moves the responsibility of ending the transition to the `ScreenStack`. We've considered an alternative approach, but this one, despite being a hack, seemed like the least disruptive one. Considered alternative: * Simply return `true` from `SafeAreaView`'s `onPreDraw`. This would allow the transition to be ended at the right moment, but it might cause a flash of un-inset content during the first frame of the transition, which is even worse than the current issue. I haven't been able to trigger that effect though. The `SafeAreaView` seemed to be working correctly despite that change, but decided it's a real risk. It seems that this is not an end of the problems. We've noticed that when we set a large font size for the tab bar item label, the safe area view receives few incorrect layouts. The tab bar is thankfully (:D) stable, however the safe area jump is visible. --- .../com/swmansion/rnscreens/ScreenStack.kt | 16 +++++++++++++++- .../rnscreens/ScreenStackFragment.kt | 14 ++++++-------- .../rnscreens/gamma/tabs/host/TabsHost.kt | 19 +++++++++++-------- .../stack/views/ScreensCoordinatorLayout.kt | 18 ++++++++++++++++++ 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt b/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt index bffd9c7c72..9f9f8647a5 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenStack.kt @@ -76,6 +76,13 @@ class ScreenStack( } override fun endViewTransition(view: View) { + require(view is ScreensCoordinatorLayout) { + "[RNScreens] Expected children of type: ${ScreensCoordinatorLayout::class.java.simpleName}" + } + if (view.blockFrameworkTransitionFinalization) { + return + } + super.endViewTransition(view) disappearingTransitioningChildren.remove(view) @@ -89,10 +96,17 @@ class ScreenStack( } } - fun onViewAppearTransitionEnd() { + internal fun onViewTransitionEnd(view: ScreensCoordinatorLayout) { if (!removalTransitionStarted) { dispatchOnFinishTransitioning() } + + if (view.needsTransitionFinalization) { + check(!view.blockFrameworkTransitionFinalization) { + "[RNScreens] Attempt to finalize transition on view that blocks the action!" + } + endViewTransition(view) + } } /** diff --git a/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt b/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt index dd82c6c75e..b9e6e2e908 100644 --- a/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt +++ b/android/src/main/java/com/swmansion/rnscreens/ScreenStackFragment.kt @@ -148,19 +148,17 @@ class ScreenStackFragment : override fun onViewAnimationEnd() { super.onViewAnimationEnd() - - // Rely on guards inside the callee to detect whether this was indeed appear transition. - notifyViewAppearTransitionEnd() - - // Rely on guards inside the callee to detect whether this was indeed removal transition. - screen.endRemovalTransition() + notifyViewTransitionEnd() } - private fun notifyViewAppearTransitionEnd() { + private fun notifyViewTransitionEnd() { val screenStack = view?.parent if (screenStack is ScreenStack) { - screenStack.onViewAppearTransitionEnd() + screenStack.onViewTransitionEnd(coordinatorLayout) } + + // Rely on guards inside the callee to detect whether this was indeed removal transition. + screen.endRemovalTransition() } /** diff --git a/android/src/main/java/com/swmansion/rnscreens/gamma/tabs/host/TabsHost.kt b/android/src/main/java/com/swmansion/rnscreens/gamma/tabs/host/TabsHost.kt index 8da0489f1b..b8fd8e6b19 100644 --- a/android/src/main/java/com/swmansion/rnscreens/gamma/tabs/host/TabsHost.kt +++ b/android/src/main/java/com/swmansion/rnscreens/gamma/tabs/host/TabsHost.kt @@ -128,14 +128,17 @@ class TabsHost( @Suppress("SENSELESS_COMPARISON") // layoutCallback can be null here since this method can be called in init if (!isLayoutEnqueued && layoutCallback != null) { isLayoutEnqueued = true - // we use NATIVE_ANIMATED_MODULE choreographer queue because it allows us to catch the current - // looper loop instead of enqueueing the update in the next loop causing a one frame delay. - ReactChoreographer - .getInstance() - .postFrameCallback( - ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE, - layoutCallback, - ) + post { + layoutCallback.doFrame(0) + } +// // we use NATIVE_ANIMATED_MODULE choreographer queue because it allows us to catch the current +// // looper loop instead of enqueueing the update in the next loop causing a one frame delay. +// ReactChoreographer +// .getInstance() +// .postFrameCallback( +// ReactChoreographer.CallbackType.NATIVE_ANIMATED_MODULE, +// layoutCallback, +// ) } } diff --git a/android/src/main/java/com/swmansion/rnscreens/stack/views/ScreensCoordinatorLayout.kt b/android/src/main/java/com/swmansion/rnscreens/stack/views/ScreensCoordinatorLayout.kt index ff1409d9b2..487fa9d93b 100644 --- a/android/src/main/java/com/swmansion/rnscreens/stack/views/ScreensCoordinatorLayout.kt +++ b/android/src/main/java/com/swmansion/rnscreens/stack/views/ScreensCoordinatorLayout.kt @@ -23,6 +23,9 @@ internal class ScreensCoordinatorLayout( PointerEventsBoxNoneImpl(), ) + internal var blockFrameworkTransitionFinalization = false + internal var needsTransitionFinalization = false + override fun onApplyWindowInsets(insets: WindowInsets?): WindowInsets = super.onApplyWindowInsets(insets) private val animationListener: Animation.AnimationListener = @@ -32,7 +35,12 @@ internal class ScreensCoordinatorLayout( } override fun onAnimationEnd(animation: Animation) { + if (blockFrameworkTransitionFinalization) { + blockFrameworkTransitionFinalization = false + needsTransitionFinalization = true + } fragment.onViewAnimationEnd() + needsTransitionFinalization = false } override fun onAnimationRepeat(animation: Animation) {} @@ -50,6 +58,16 @@ internal class ScreensCoordinatorLayout( // We also add fakeAnimation to the set of animations, which sends the progress of animation val fakeAnimation = ScreensAnimation(fragment).apply { duration = animation.duration } + if (fragment.isRemoving) { + // This is a hack. + // We'll finalize the transition via our listener. + // We do it to prevent the framework from ending the transition prematurely, + // due to interaction between `FragmentAnim.EndViewTransitionAnimation` and + // `SafeAreaView` drawing blocking logic. For detailed description see: + // TODO LINK THE PR HERE + blockFrameworkTransitionFinalization = true + } + if (animation is AnimationSet && !fragment.isRemoving) { animation .apply {