material-components / material-components-android

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

[BottomSheetBehavior] Collapse/Expand threshold is different for NestedScrollView #1055

Open nate-wallace-9952 opened 4 years ago

nate-wallace-9952 commented 4 years ago

Description: When dragging a bottom sheet (without flinging it) and then letting go, the expand/collapse behavior is different based on if the user started the drag from within a NestedScrollView versus any other type of View.

When starting a drag from outside of a NestedScrollView the BottomSheetBehavior appears to take the velocity of the pointer into account (both horizontal and vertical) when determining to expand or collapse. If there is no vertical velocity or if there is more horizontal velocity than vertical then the bottom sheet will move to whichever of the expand or collapse positions is closest.

When starting a drag from within a NestedScrollView the BottomSheetBehavior doesn't appear to take the pointer velocity into account. All movement is treated as if the user was flinging the view so any slight movement in the upward direction will expand the bottom sheet and any movement in the downward direction will collapse the bottom sheet.

Expected behavior: I was expecting the expand/collapse behavior to be the same between NestedScrollView Views and other types of Views. The current behavior makes user interaction within a NestedScrollView overly sensitive. If a user swipes horizontally on a control within a NestedScrollView it may expand/collapse the bottom sheet unexpectedly if there is any amount of upward or downward motion.

Source code: I posted about this issue on StackOverflow. Sample code that illustrates this issue can be found there: https://stackoverflow.com/questions/60284004/bottomsheet-has-no-drag-threshold-when-user-initiates-drag-from-within-nestedscr

It looks like it's this check in the onViewReleased method that results in the desired behavior which is not present in the onStopNestedScroll method: https://github.com/material-components/material-components-android/blob/8e8d20c94f3573f7f7418f3971fbbc41a4fa83bd/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java#L1345-L1347

Android API version: 28

Material Library version: com.google.android.material:material:1.2.0-alpha04

Device: Pixel

hugosvent commented 4 years ago

any update about this? how do you handle this? I got this issue to when user tap and accidentaly slide a little the sheet change state expanded/collapsed

Tried checking for the offset and force the state to be collapse if offset < 0.05 and expand if offset > 0.95. But sometimes it will be jumpy (before collapse it will goes to expand for a bit)

hugosvent commented 4 years ago

I tried to copy all the BottomSheetBehavior.java and change this line

https://github.com/material-components/material-components-android/blob/88ca5c9eeb64bacaac8589419f3a1838b230631a/lib/java/com/google/android/material/bottomsheet/BottomSheetBehavior.java#L609

instead of 0, I try to use 3 or more so it doesn't trigger if the scroll is not that much

kaneoriley commented 3 years ago
/** Convenience function to fix https://github.com/material-components/material-components-android/issues/1055 */
private fun NestedScrollView.fixNestedScrolling(dialog: BottomSheetDialog) {
    fun updateScrollView(scrollY: Int) {
        val wasNestedScrollingEnabled = isNestedScrollingEnabled
        isNestedScrollingEnabled = scrollY > 0
        if (wasNestedScrollingEnabled != isNestedScrollingEnabled) {
            // If property has changed, we need to requestLayout for it to apply to swipe gestures.
            dialog.findViewById<View>(R.id.design_bottom_sheet)?.requestLayout()
        }
    }

    setOnScrollChangeListener { _, _, scrollY, _, _ -> updateScrollView(scrollY) }

    // Fire off initial update
    updateScrollView(0)
}

This works for me personally. The nested scrollview works correctly, and once scrollY == 0 (i.e. we're at the top), nested scrolling is disabled so the BottomSheetBehavior uses the (much more natural) calculations before initiating a dismiss.

kaizie commented 2 years ago

Any update on this?

romankulykov commented 1 year ago

@kaneoriley solution works for me