fix(Android, Stack): Prefer consuming the top inset from DecorView#3500
fix(Android, Stack): Prefer consuming the top inset from DecorView#3500
Conversation
kkafar
left a comment
There was a problem hiding this comment.
I don't approve this change right now for at least few reasons.
First - we don't have a valid reproduction, so it's really hard to tell what is the root cause of the issue -> blindly working around it introduces a hard to maintain code into our codebase. We should insist on having reproduction here.
Second - we should avoid using insets directly from window / decor, exactly for this reason -> if someone consumes the insets & sets the padding above us, we will apply the padding erroneously for the second time. This will most likely also break nested stacks scenarios. So I believe now that this is not really a fix, rather a workaround for this particular issue.
Third - likely the bug comes, because somebody (some component, maybe even one of ours) consumes the insets and does not apply correct padding and this is where we should look for the fix.
|
Just to help here, I used claude code to help find the culprit that may help you identify the issue. Find the details below: Root Cause Analysis: The regression was introduced by PR #3240 which changed how CustomToolbar retrieves window insets. The Breaking Change: In CustomToolbar.kt:116-129, the toolbar now uses unhandledInsets from super.onApplyWindowInsets(insets) instead of rootWindowInsets. Why It Fails: In edge-to-edge mode (enforced on Android SDK 35+), parent views in the hierarchy consume window insets before they reach the toolbar. By the time CustomToolbar.onApplyWindowInsets() is called, the top system bar inset has already been consumed, resulting in zero top padding. Affected Scenarios: Hope this helps. |
Description
The top inset from DecorView is currently used in both Screen and ScreenDummyLayoutHelper to manually apply layout adjustments. To ensure consistent behavior between Screen and Toolbar, we should preferably handle the top inset here. Relying on unhandledInsets may be unreliable, as they could already have been consumed earlier in the layout process.
Fixes: #3499
Changes
Test code and steps to reproduce
I'm not able to reproduce this issue on our side, so I'm basing on the information from: #3499 (comment)
Additionally, I ensured that it causes no regression in Test3006, for which these changes were initially made.
Checklist