fix(Android, Stack, Fabric): Fix jumping content in nested stack for Fabric #3442
Conversation
t0maboro
changed the title
t0maboro
changed the title
android/src/main/java/com/swmansion/rnscreens/utils/DecorViewInsetsUtils.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper.kt
Outdated
Show resolved
Hide resolved
Good callout about 3405 I tested with and without SAV for nested stacks, results look legit Screen.Recording.2025-12-03.at.10.20.40.movScreen.Recording.2025-12-03.at.10.21.46.mov |
kligarski
left a comment
kligarski
left a comment
There was a problem hiding this comment.
Looks good & scary at the same time.
|
This resolved all my jumping issues on a rather complex split-view tablet Android application. I have applied these changes as a patch for now. Thanks for this!! |
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
Great job here. Looks as hacky as I expected after our initial discussions 😄
Asked few questions & added remarks. Please answer them.
android/src/main/java/com/swmansion/rnscreens/utils/DecorViewInsetsUtils.kt
Show resolved
Hide resolved
android/src/main/java/com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper.kt
Outdated
Show resolved
Hide resolved
android/src/main/java/com/swmansion/rnscreens/utils/ScreenDummyLayoutHelper.kt
Show resolved
Hide resolved
| val maybeAppBarLayout = parent as? CustomAppBarLayout | ||
| maybeAppBarLayout?.let { | ||
| if (shouldApplyLayoutCorrectionForTopInset && !it.isInLayout) { | ||
| it.onToolbarLayout(paddingTop) |
There was a problem hiding this comment.
Reading this I have a problem with naming here. We call onToolbarLayout in requestLayout. This clearly contradicts.
Why do we call this callback in requestLayout? Shouldn't it be called in onLayout?
There was a problem hiding this comment.
By doing it this way, we call onToolbarLayout before the layout triggered by this requestLayout call happens. Is this intentional?
There was a problem hiding this comment.
By doing it this way, we call onToolbarLayout before the layout triggered by this requestLayout call happens. Is this intentional?
yes, in CustomAppBarLayout I'm calling layout explicitly to make a correction asap, without any overhead regarding calling requestLayout
There was a problem hiding this comment.
Okay. Can we add comment documenting this? That we call layout here, to ensure immediate frame update & prevent potentially invalid state being displayed before requested layout is executed.
This reverts commit c7895b1.
kkafar
left a comment
kkafar
left a comment
There was a problem hiding this comment.
I followed up with request on the last review.
Description
This PR introduces a set of fixes and manual layout adjustments to address layout/content jumps in Fabric. I listed them under
Changessection with some motivations.Caution
This PR aims to resolve the issue only on Fabric; on Paper some additional effort is needed and (potentially) would require adapting
ScreenDummyLayoutHelperto old arch. The work will be continued in a separate PR.Changes
CustomAppBarLayout: This custom implementation triggers a layout refresh on theAppBarLayoutwhen we apply padding adjustments to theCustomToolbar.AppBarLayoutjust dispatches insets toToolbar; therefore, I'm still handling them at theToolbarlevel.ActionMenuViewin theToolbar: Initializing theActionMenuViewearly in the layout process prevents inconsistencies and avoids fallback to the system default toolbar height inonMeasure, ensuring a consistent layout.Screen: I'm applying some temporary padding corrections at theScreenlevel until insets are applied.ScreenDummyLayoutHelper: A similar early correction approach is used forScreenDummyLayoutHelperto keep the consistency for the header size before full layout pass (this one with insets applied) is complete.DecorViewInsetHelper: React's layout arrives before insets have been applied, leading to content jumps. To counter this, we rely on insets available from theDecorView(child ofRootView). A dedicated helper class extracts these inset values. Additionally, it ensures that the insets are applied only on the topmost Screen with Toolbar, preventing the unnecessary padding on the nested Toolbars.Screenshots / GIFs
Here you can add screenshots / GIFs documenting your change.
You can add before / after section if you're changing some behavior.
Fabric
Before
Screen.Recording.2025-12-02.at.09.48.27.mov
Screen.Recording.2025-12-02.at.09.49.06.mov
After
Screen.Recording.2025-12-02.at.09.47.01.mov
Screen.Recording.2025-12-02.at.09.46.23.mov
Test code and steps to reproduce
Added Test3006, tested with API levels above and below 30, with both Status Bar and Display Cutout. As mentioned earlier only testing on FabricExample is in the scope of this PR.
Checklist