Material3 Bottom Sheets should support hiding via external gestures like system back [304850357]

Fixed

Feature Request

Status Update

No update yet.

Description

il...@google.com

created issue #1

Oct 11, 2023 11:48PM

With Android 14, apps can opt in for predictive back where they show a preview of what is happening as the system back gesture is happening (rather than only after the user confirms the system back gesture).

The Activity 1.8.0 release adds a new PredictiveBackHandler that allows apps to capture the progress of the system back gesture and use it to drive the state of their own components.

I wanted to use this to start hiding a Material3 Bottom Sheet by writing code like:

PredictiveBackHandler(enabled = showBottomSheet) { progress ->
    try {
        progress.collectLatest {  backEvent ->
            // Progress is 0 (not hidden) to 1 (fully hidden)
            val progress = backEvent.progress
            // TODO: Be able to pass the progress to hide to animate the
            // bottom sheet to a particular gesture controlled progress
            // towards completely hidden
            sheetState.hide(progress)
        }
        // The user has fully committed the system back gesture and the
        // bottom sheet should be completely hidden
        sheetState.hide()
    } catch (e: CancellationException) {
        // The user has cancelled the system back gesture so we should
        // reset its position back to shown
        // TODO: ensure this works even though this suspending scope is cancelled
        sheetState.show()
    }
}

But as per the two TODO messages there, I ran into two issues:

There's no way to drive the progress towards the hidden state from outside the bottom sheet - I see that ModalBottomSheet does this internally to support dragging the bottom sheet down, but this isn't exposed anywhere.
The use of a suspend show method here doesn't actually work well in PredictiveBackHandler since the scope that is provided to the trailing lambda is cancelled by the time we need to call show (similar to other suspending lambdas, it uses coroutine's cancellation as its indicator for cancellation). It would be helpful to have a non-suspending way to set the targetValue and let ModalBottomSheet take care of moving to that state. I think we could possibly work around this by using rememberCoroutineScope and launching the show into that scope, but that seems even harder to get right.

Comments

il...@google.com <il...@google.com> #2Oct 11, 2023 11:56PM

Project: platform/frameworks/support
Branch: androidx-main

commit a330c0d3bcdd41326f37968a60e6084ad4a2e32c
Author: Chet Haase <chet@google.com>
Date: Wed Jul 05 07:26:46 2023

Convert APIs using PointF to use Float instead

PointF is a convenient mechanism for passing around x.y values
representing 2D points. But there are downsides, including:
- Converting to PointF: You may not have the data in PointF form
to begin with, so using an API which takes PointF requires converting
the data to that form (including allocating a PointF object every time)
- Mutability: Point structures can be mutated internally, causing
unpredictability in what that mutation means. Should the library
react to those changes? Ignore them? Do defensive copies (requiring
even more allocations)? Using primitive types like Float make the
behavior more obvious (by making the data inherently immutable).
- Allocations: Whenever we use object types, there are necessarily
allocations on the Java heap for them. This puts pressure on the GC
at both allocation and collection time. Given the amount of points
being passed around (especially at morph creation time, when curves
are being split and created), this causes a lot of PointF objects to
be allocated (even temporarily). Using Float avoids that problem.

Also fixed bug with unclosed paths causing discontinuity at the
start/end point.

Bug: 276466399
Bug: 290254314
Test: integration and unit tests pass
Relnote: PointF parameters changed to Float pairs
Change-Id: Id4705d27c7be31b26ade8186b99fffe2e2f8450e

M graphics/graphics-shapes/api/current.txt
M graphics/graphics-shapes/api/restricted_current.txt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicShapeTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/CubicTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonMeasureTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/PolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/RoundedPolygonTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/ShapesTest.kt
M graphics/graphics-shapes/src/androidTest/java/androidx/graphics/shapes/TestUtils.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Cubic.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/CubicShape.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FeatureMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/FloatMapping.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Morph.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/PolygonMeasure.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/RoundedPolygon.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Shapes.kt
M graphics/graphics-shapes/src/main/java/androidx/graphics/shapes/Utils.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/DebugDraw.kt
M graphics/integration-tests/testapp-compose/src/main/java/androidx/graphics/shapes/testcompose/ShapeEditor.kt
M graphics/integration-tests/testapp/src/main/java/androidx/graphics/shapes/test/MaterialShapes.kt

https://android-review.googlesource.com/2649119

jo...@google.com <jo...@google.com> #3Oct 12, 2023 09:40AM

PointF was the main (possibly only) mutability issue, marking this as fixed

jo...@google.com <jo...@google.com> #4Oct 12, 2023 09:43AM

Reassigned to bu...@google.com.

The following release(s) address this bug.It is possible this bug has only been partially addressed:

androidx.graphics:graphics-shapes:1.0.0-alpha04

bu...@google.com <bu...@google.com> #5Oct 12, 2023 09:43AM

Accepted by bu...@google.com.

Hi. I've received your bug and will wait for

b/304936017

to be resolved and then mark as fixed by jossiwolf@google.com, or give up at 2023-11-23 09:43 +0000 GMT.

bu...@google.com <bu...@google.com> #6Nov 23, 2023 09:44AM

Assigned to jo...@google.com.

The specified deadline has been exceeded.

jo...@google.com <jo...@google.com> #7Nov 27, 2023 07:41PM

Reassigned to se...@google.com.

Copying comment from M2 issue:

Looking at the Material guidelines and MDC views, it looks like the gesture only influences how the sheet is drawn. AnchoredDraggable's APIs support this, although we might want to improve the contract of the anchoredDrag method so users do not have to call settle manually.

The implementation of this lies with the Material team, reassigning for prioritization:

Here is some example code:

    /**
     * This receiver allows manipulating the sheet's offset.
     */
    interface SheetAnimationScope {
        fun seekTo(fraction: Float)
    }

    /**
     * Seek the [ModalBottomSheetState] to fractional values between [from] and [to]. This is useful
     * to integrate seeking interactions such as predictive back.
     */
    suspend fun seek(
        from: ModalBottomSheetValue = currentValue,
        to: ModalBottomSheetValue = if (from == HalfExpanded || from == Expanded) Hidden
        else if (hasHalfExpandedState) HalfExpanded
        else Expanded,
        block: suspend SheetAnimationScope.() -> Unit
    ) {
        anchoredDraggableState.anchoredDrag { anchors ->
            val startOffset = anchors.positionOf(from)
            val targetOffset = anchors.positionOf(to)
            val distance = abs(startOffset - targetOffset)
            val scope = object : SheetAnimationScope {
                override fun seekTo(fraction: Float) {
                    val newOffset = distance * fraction
                    dragTo(startOffset + newOffset)
                }
            }
            scope.block()
        }
        anchoredDraggableState.settle(0f)
    }

     PredictiveBackHandler { progress ->
          bottomSheetState.seek(to = ModalBottomSheetValue.Hidden) {
               progress.collect { backEvent ->
                    seekTo(backEvent.progress)
               }
          }
     }

se...@google.com <se...@google.com> #8Jan 22, 2024 08:54PM

Reassigned to dn...@google.com.

CC: Dan, is this within the scope of your predictive back work in compose?

dn...@google.com <dn...@google.com> #9Jan 25, 2024 06:01PM

Yes! Working on this in https://android-review.googlesource.com/c/platform/frameworks/support/+/2863667.

With the approach in the CL above, ModalBottomSheet will automatically animate itself during the predictive back swipe gesture on U+.

dn...@google.com <dn...@google.com> Feb 2, 2024 05:57PM

Accepted by dn...@google.com.

ap...@google.com <ap...@google.com> #10Feb 13, 2024 07:29PM

Project: platform/frameworks/support
Branch: androidx-main

commit 699ee4fa3f47da2ccd52fb6b4628fc2d61cd5039
Author: Dan Nizri <dniz@google.com>
Date: Wed Dec 06 20:50:22 2023

Update Compose M3 ModalBottomSheet to support Predictive Back on U+

Bug: 281967264
Bug: 307969902
Bug: 304850357
Test: added screenshot tests
Relnote: "Update Compose M3 ModalBottomSheet to support Predictive Back on U+"
Change-Id: Iccf324cb6dfc7f4ea1fe413b69e035658282360d

A compose/material3/material3/src/androidInstrumentedTest/kotlin/androidx/compose/material3/ModalBottomSheetScreenshotTest.kt
M compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/ModalBottomSheet.android.kt
M compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/SearchBar.android.kt
A compose/material3/material3/src/androidMain/kotlin/androidx/compose/material3/internal/PredictiveBack.android.kt

https://android-review.googlesource.com/2863667

dn...@google.com <dn...@google.com> Feb 14, 2024 02:15PM

Marked as fixed.

pr...@google.com <pr...@google.com> #11Feb 21, 2024 07:33PM