Skip to content

Conversation

@t0maboro
Copy link
Contributor

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

  • Added logic for preferring top inset from DecorView.

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

  • Included code example that can be used to test this change
  • Ensured that CI passes

Copy link
Member

@kkafar kkafar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

No top insets for Android in 4.19.0

3 participants