material-components / material-components-android

Modular and customizable Material Design UI components for Android
Apache License 2.0
16.38k stars 3.07k forks source link

[BottomSheetBehavior] `shouldSkipHalfExpandedStateWhenDragging` is ignored by some nested scrolling events #2874

Closed OxygenCobalt closed 2 years ago

OxygenCobalt commented 2 years ago

Description: My bottom sheets are fitToContents = false, thus, they have a half-expanded state I try to disable on my end. The existing flags that I use work well for all touch events except for nested scrolling. There, the half expanded state will still occur, even when I try to disable it.

Expected behavior: When completing a nested scrolling event that swipes the panel up or down, the behavior should check shouldSkipHalfExpandedStateWhenDragging and skip the half expanded state when enabled, like the other dragging code.

More specifically, these lines in onStopNestedScroll need to be updated to have a check: https://github.com/material-components/material-components-android/blob/23b7157a648411cf01ca22eb7071fc4f4ad5ac05/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java#L771-L775

https://github.com/material-components/material-components-android/blob/23b7157a648411cf01ca22eb7071fc4f4ad5ac05/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java#L799-L803

https://github.com/material-components/material-components-android/blob/23b7157a648411cf01ca22eb7071fc4f4ad5ac05/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java#L812-L816 Source code:

I disable the half-expanded state by overriding the following methods:

override fun shouldSkipHalfExpandedStateWhenDragging() = true
override fun shouldExpandOnUpwardDrag(
    dragDurationMillis: Long,
    yPositionPercentage: Float
) = true

I understand that these are for internal use, but since #1807 is seemingly dead, I had to use whatever workarounds I could find.

Android API version: API 31

Material Library version: 1.7.0-alpha03

Device: Pixel 5

drchen commented 2 years ago

Hi thanks for posting this and providing your workaround. Basically #1807 is about opening shouldSkipHalfExpandedStateWhenDragging to public and applying whatever fixes we need to make it act correctly (like your suggestion.)

The only thing is that I don't think we currently have people who can work on that. For now, to move this forward, I think the best first step will be creating a PR to include the fixes you need to make shouldSkipHalfExpandedStateWhenDragging work correctly (but don't expose shouldSkipHalfExpandedStateWhenDragging to public), so it will unblock you from using it, and in the meantime we can skip the tedious process of adding a new public API like adding tests, etc., for now. If you can make a PR I'll review it.

I'll close the issue as duplicate to #1807 so we can better track the request.

OxygenCobalt commented 2 years ago

Okay, cool. I've already patched it on my end so I'll go make a PR.

OxygenCobalt commented 2 years ago

Okay, I filed a PR at #2876 @drchen.