Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 502a5b56 authored by Ale Nijamkin's avatar Ale Nijamkin
Browse files

[Media] Removes placeRelatively parameter from OffsetOverscrollEffect 

The thinking is that placing relatively is _never_ the right behavior
for this effect since drag deltas don't change their sign based on
LayoutDirection.

Bug: 403558944
Test: manually verified that media in compose still works correctly both
in the swipe to dismiss and swipe to reveal cases in the compose gallery
app
Flag: EXEMPT code not in use in production

Change-Id: Ie276704c6c3d7673a99a52da1d8b8c6429538e06
parent 3f6fd4ab

packages/SystemUI/compose/core/src/com/android/compose/gesture/effect/OffsetOverscrollEffect.kt

+9 −30
Original line number Diff line number Diff line
@@ -36,22 +36,15 @@ import androidx.compose.ui.unit.dp 
import kotlin.math.roundToInt

import kotlinx.coroutines.CoroutineScope



/**

 * Returns a [remember]ed [OffsetOverscrollEffect].

 *

 * @param placeRelatively If `true`, the overscrolled content will be placed relatively, meaning

 *   that its placement will be flipped in right-to-left layouts. If `false`, the content will be

 *   placed absolutely - same placement for RTL and LTR.

 */

/** Returns a [remember]ed [OffsetOverscrollEffect]. */

@Composable

@OptIn(ExperimentalMaterial3ExpressiveApi::class)

fun rememberOffsetOverscrollEffect(

    animationSpec: AnimationSpec<Float> = MaterialTheme.motionScheme.slowSpatialSpec(),

    placeRelatively: Boolean = true,

    animationSpec: AnimationSpec<Float> = MaterialTheme.motionScheme.slowSpatialSpec()

): OffsetOverscrollEffect {

    val animationScope = rememberCoroutineScope()

    return remember(animationScope, animationSpec) {

        OffsetOverscrollEffect(animationScope, animationSpec, placeRelatively)

        OffsetOverscrollEffect(animationScope, animationSpec)

    }

}
@@ -74,17 +67,13 @@ data class OffsetOverscrollEffectFactory( 
        return OffsetOverscrollEffect(

            animationScope = animationScope,

            animationSpec = animationSpec,

            placeRelatively = true,

        )

    }

}



/** An [OverscrollEffect] that offsets the content by the overscroll value. */

class OffsetOverscrollEffect(

    animationScope: CoroutineScope,

    animationSpec: AnimationSpec<Float>,

    placeRelatively: Boolean,

) : BaseContentOverscrollEffect(animationScope, animationSpec) {

class OffsetOverscrollEffect(animationScope: CoroutineScope, animationSpec: AnimationSpec<Float>) :

    BaseContentOverscrollEffect(animationScope, animationSpec) {

    override val node: DelegatableNode =

        object : Modifier.Node(), LayoutModifierNode {

            override fun MeasureScope.measure(
@@ -95,25 +84,15 @@ class OffsetOverscrollEffect( 
                return layout(placeable.width, placeable.height) {

                    val offsetPx = computeOffset(density = this@measure, overscrollDistance)

                    if (offsetPx != 0) {

                        if (placeRelatively) {

                            placeable.placeRelativeWithLayer(

                                with(requireConverter()) { offsetPx.toIntOffset() }

                            )

                        } else {

                        placeable.placeWithLayer(

                            with(requireConverter()) { offsetPx.toIntOffset() }

                        )

                        }

                    } else {

                        if (placeRelatively) {

                            placeable.placeRelative(0, 0)

                    } else {

                        placeable.place(0, 0)

                    }

                }

            }

        }

        }



    companion object {

        private val MaxDistance = 400.dp

packages/SystemUI/src/com/android/systemui/media/remedia/ui/compose/Media.kt

+2 −3
Original line number Diff line number Diff line
@@ -244,8 +244,7 @@ private fun CardCarouselContent( 
                    userScrollEnabled = isSwipingEnabled,

                    pageSpacing = 8.dp,

                    key = { index: Int -> viewModel.cards[index].key },

                    overscrollEffect =

                        overscrollEffect ?: rememberOffsetOverscrollEffect(placeRelatively = false),

                    overscrollEffect = overscrollEffect ?: rememberOffsetOverscrollEffect(),

                ) { pageIndex: Int ->

                    Card(

                        viewModel = viewModel.cards[pageIndex],
@@ -274,7 +273,7 @@ private fun CardCarouselContent( 
                onDismissed = onDismissed,

            )

        } else {

            val overscrollEffect = rememberOffsetOverscrollEffect(placeRelatively = false)

            val overscrollEffect = rememberOffsetOverscrollEffect()

            SwipeToReveal(

                foregroundContent = { PagerContent(overscrollEffect) },

                foregroundContentEffect = overscrollEffect,

packages/SystemUI/src/com/android/systemui/media/remedia/ui/compose/SwipeToDismiss.kt

+1 −1
Original line number Diff line number Diff line
@@ -48,7 +48,7 @@ fun SwipeToDismiss( 
    onDismissed: () -> Unit,

    modifier: Modifier = Modifier,

) {

    val overscrollEffect = rememberOffsetOverscrollEffect(placeRelatively = false)

    val overscrollEffect = rememberOffsetOverscrollEffect()



    // This is the width of the content UI box. It's not a state because it's not observed in any

    // composition and is an object with a value to avoid the extra cost associated with boxing and

packages/SystemUI/src/com/android/systemui/media/remedia/ui/compose/SwipeToReveal.kt

+1 −1
Original line number Diff line number Diff line
@@ -65,7 +65,7 @@ fun SwipeToReveal( 
    // This composable supports an overscroll effect, to make it possible for the user to

    // "stretch" the UI when the side is fully revealed but the user keeps trying to reveal it

    // further.

    val revealedContentEffect = rememberOffsetOverscrollEffect(placeRelatively = false)

    val revealedContentEffect = rememberOffsetOverscrollEffect()



    // This is the width of the revealed content UI box. It's not a state because it's not

    // observed in any composition and is an object with a value to avoid the extra cost