[BottomSheet] Bottomsheet appears at top of screen when transitioning on 1.6

[skyler-stripe]

Description: When upgrading to 1.6 from 1.5 our bottom sheet started to appear at the top of the screen when transitioning to a different fragment inside.

https://user-images.githubusercontent.com/89166418/168380762-a03fc111-83df-48b5-a6dd-48f5e0bcff61.mov

Expected behavior: Bottom sheet should stay at the bottom when transitioning.

Source code: Our logic is fairly complicated so I'll try to link the relevant files. Maybe you can give us suggestions on fixes if it's our issue.

xml: https://github.com/stripe/stripe-android/blob/master/paymentsheet/res/layout/activity_payment_sheet.xml#L9

    <LinearLayout
        android:id="@+id/bottom_sheet"
        android:layout_width="match_parent"
        android:layout_height="wrap_content"
        android:layout_gravity="bottom|center_horizontal"
        android:animateLayoutChanges="true"
        android:orientation="vertical"
        android:background="?colorSurface"
        app:layout_behavior="com.google.android.material.bottomsheet.BottomSheetBehavior">

controller class: https://github.com/stripe/stripe-android/blob/master/paymentsheet/src/main/java/com/stripe/android/paymentsheet/BottomSheetController.kt

main bottom sheet activity: https://github.com/stripe/stripe-android/blob/master/paymentsheet/src/main/java/com/stripe/android/paymentsheet/ui/BaseSheetActivity.kt

Minimal sample app repro: Our repository has a sample app called paymentsheet-example, I'm also happy to try and spend some more time creating a minimal sample repo. but I'm not quite sure what would cause this behavior so any guidance would help.

You'll need to change the value to 1.6 here: https://github.com/stripe/stripe-android/blob/master/build.gradle#L81

Once running you can click

1. Playground
2. Check returning customer
3. Reload paymentsheet
4. click the visa logo next to checkout custom
5. click add

observed the sheet is at the top of the screen. Occasionally I see weird states where it's partially showing like so:

Android API version: min api 21, repros on all versions I tried: 28,30,31

Material Library version: 1.6

Device: Pixel 3a, several emulators.

To help us triage faster, please check to make sure you are using the latest version of the library.

We also happily accept pull requests.

[afohrman]

Hi Skyler, this isn't happening in the default catalog case, but I was able to reproduce the issue in our catalog and find what looks like the trigger for the bug - looks like the culprit was setting fitContents to false right here in the BottomSheetController. I'm pretty sure that this is unintended behavior and a bug on our end and I'm trying to find the source of the behavior and when it was introduced; so far I've found the issue as far back as https://github.com/material-components/material-components-android/commit/19af0ac9d98cc3c504ce69ae38fbc00570cc85f5.

In the meantime, you should be able to work around the issue by removing that line to continue using 1.6.0. Please let me know if that works for you.

[skyler-stripe]

Hey there!

Removing that line causes a few other issues for us. I'll follow up with my team, but we need that line because as our sheet resizes (which happens a lot with our usecases) it doesn't get positioned on the screen correctly. Any other suggestions?

Here's what happens when I remove that line and upgrade to 1.6 btw:

[afohrman]

Hey Skyler, I narrowed down the source of the bug to commit https://github.com/material-components/material-components-android/commit/04c483cf3466cd0a7bf4835cadccc2372ba0da2b. It looks like it should be a relatively clean rollback, and assuming a rollback is feasible, we plan to include a fix for the issue in an upcoming release.

Thanks for reporting this issue and for the detailed bug description!

[skyler-stripe]

Great!

It looks like we'll have to wait on this release for the fix, which is no problem. Let us know when you have it ready for us to test.

[afohrman]

Looking at this some more, it looks like there actually are issues on both ends. Your app's usage of fitToContents appears to be masking some layout issues with the bottom sheet configuration. The fitToContents=false setting is actually intended to be sort of like match_parent, as opposed to the default behavior, fitToContents=true, which is basically wrap_content. In this case, it looks like the bottom sheet was not behaving properly beforehand, and setting fitToContents to false was hiding the issue -- although that flag itself wasn't working properly beforehand; if it was, it would have set the bottom sheet to "fullscreen mode", since it looks like the parent CoordinatorLayout's height is fullscreen. In fact, https://github.com/material-components/material-components-android/commit/04c483cf3466cd0a7bf4835cadccc2372ba0da2b may have fixed whatever bug prevented fitToContents from working properly initially, thereby allowing the bottom sheet to expand to the top of the parent.

Separately, there is an issue with bottom sheet that may be on our end (although I haven't quite gotten to the bottom of it yet), which is that empty space appears at the bottom of the bottom sheet, to create the appearance of "flying up". I'm going to do some more digging and try to find out what's causing that, or if it might be a bug introduced on our end in 1.6.0 (but based on what I've found so far, it's not newly introduced in 1.6.0).

Another issue in BottomSheetController.kt is that fitToContents is set inside the BottomSheetCallback every time the bottom sheet hits STATE_EXPANDED. In our catalog, doing this creates the exact same bug, even further back then https://github.com/material-components/material-components-android/commit/04c483cf3466cd0a7bf4835cadccc2372ba0da2b. Although we don't explicitly state that we don't support changing layout flags during layout state transitions, it is a non-typical use case and we'd have to evaluate whether can support that and if we want to. If you do choose to continue using the flag, I recommend setting it outside of the BottomSheetCallback, along with the other layout flags in BottomSheetController.kt.

I was also wondering whether you could set isDraggable to true, in order to allow for user scrolling? Seems like this might prevent the sizing/layout issue you mentioned here.

One more thing: it looks like your app is using persistent bottom sheets (BottomSheetBehavior) along with activities to achieve what is essentially a BottomSheetDialog. Is there a reason why that architecture is necessary? Moving to BottomSheetDialog should simplify the code and reduce bugs.

[Kiwijane3]

Do you have any additional information or an ETA on a fix? I'm experiencing a similar issue.

[RostyslavKloos]

@skyler-stripe just remove android:animateLayoutChanges="true"