Skip to content

Conversation

kligarski
Copy link
Contributor

@kligarski kligarski commented May 16, 2025

Description

Status bar insets were not handled properly when opening the formSheet with initial height that was greater than safe content area. The bug is directly connected with how BottomSheetBehavior works. For unknown reasons, onChildLayout sometimes isn't called after BottomSheetBehavior's insets handler runs. In this PR, we override Screen's setOnApplyWindowInsetsListener (used by BottomSheetBehavior to register its handler), to make sure to request the layout after the listener runs.

The problem

On first layout, BottomSheetBehavior registers the inset listener which updates insetsTop field. In onLayoutChild, BottomSheetBehavior uses insetsTop value to calculate childHeight which is then used to calculate fitToContentsOffset. fitToContentsOffset is returned from getExpandedOffset() if isFitToContents == true (which is the case for single and two detents in screens). If isFitToContents == false (i.e. we use 3 detents), getExpandedOffset() uses insetsTop directly. When layout wasn't called after the insets handler, previous value was used which caused the sheet to go under the status bar.

Fixes #2896.

Changes

  • add custom setter for onApplyWindowInsetsListener
  • add Test2896 test screen (unable to create e2e test because content behind formSheet still has visibility = VISIBLE)

Screenshots / GIFs

Before After
before_2896_fix.mp4
after_2896_fix.mp4

Test code and steps to reproduce

Run example app and open Test2896 test screen.

Checklist

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

@kligarski kligarski requested review from kkafar and maciekstosio May 16, 2025 10:26
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.

Beside my wrong suggestion to use ArraySet last time, this one looks good!

@kligarski kligarski force-pushed the @kligarski/form-sheet-insets branch from d8783a0 to 9e383d4 Compare May 19, 2025 09:11
@kligarski kligarski force-pushed the @kligarski/form-sheet-insets branch from 9e383d4 to 3cbc2fb Compare May 19, 2025 10:52
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 have follow up question that just popped in my head

@kligarski kligarski changed the base branch from main to @kligarski/form-sheet-autofocus-2 May 20, 2025 10:10
@kligarski kligarski force-pushed the @kligarski/form-sheet-insets branch from 3cbc2fb to e25bb8d Compare May 20, 2025 11:18
Base automatically changed from @kligarski/form-sheet-autofocus-2 to main May 20, 2025 11:37
@kligarski kligarski requested a review from kkafar May 20, 2025 12:37
@ilteoood
Copy link

Any chance that we get this PR released soon?

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.

[4.10.0] formSheet does not respect safe area
4 participants