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

Commit f06ff25d authored by Jordan Demeulenaere's avatar Jordan Demeulenaere Committed by Android (Google) Code Review
Browse files

Merge "Fix the overscroll placement/composition logic" into main

parents 584b3fe1 526b787c
Loading
Loading
Loading
Loading
+11 −8
Original line number Diff line number Diff line
@@ -596,12 +596,9 @@ private fun shouldPlaceElement(
        return false
    }

    // Place the element if it is not shared or if the current scene is the one that is currently
    // overscrolling with [OverscrollSpec].
    // Place the element if it is not shared.
    if (
        transition.fromScene !in element.sceneStates ||
            transition.toScene !in element.sceneStates ||
            transition.currentOverscrollSpec?.scene == scene.key
        transition.fromScene !in element.sceneStates || transition.toScene !in element.sceneStates
    ) {
        return true
    }
@@ -611,7 +608,7 @@ private fun shouldPlaceElement(
        return true
    }

    return shouldDrawOrComposeSharedElement(
    return shouldPlaceOrComposeSharedElement(
        layoutImpl,
        scene.key,
        element.key,
@@ -619,12 +616,18 @@ private fun shouldPlaceElement(
    )
}

internal fun shouldDrawOrComposeSharedElement(
internal fun shouldPlaceOrComposeSharedElement(
    layoutImpl: SceneTransitionLayoutImpl,
    scene: SceneKey,
    element: ElementKey,
    transition: TransitionState.Transition,
): Boolean {
    // If we are overscrolling, only place/compose the element in the overscrolling scene.
    val overscrollScene = transition.currentOverscrollSpec?.scene
    if (overscrollScene != null) {
        return scene == overscrollScene
    }

    val scenePicker = element.scenePicker
    val fromScene = transition.fromScene
    val toScene = transition.toScene
@@ -637,7 +640,7 @@ internal fun shouldDrawOrComposeSharedElement(
            toSceneZIndex = layoutImpl.scenes.getValue(toScene).zIndex,
        ) ?: return false

    return pickedScene == scene || transition.currentOverscrollSpec?.scene == scene
    return pickedScene == scene
}

private fun isSharedElementEnabled(
+1 −1
Original line number Diff line number Diff line
@@ -187,7 +187,7 @@ private fun shouldComposeMovableElement(
        } ?: return false

    // Always compose movable elements in the scene picked by their scene picker.
    return shouldDrawOrComposeSharedElement(
    return shouldPlaceOrComposeSharedElement(
        layoutImpl,
        scene,
        element,
+103 −0
Original line number Diff line number Diff line
@@ -32,6 +32,7 @@ import androidx.compose.foundation.layout.offset
import androidx.compose.foundation.layout.size
import androidx.compose.foundation.pager.HorizontalPager
import androidx.compose.foundation.pager.PagerState
import androidx.compose.material3.Text
import androidx.compose.runtime.Composable
import androidx.compose.runtime.LaunchedEffect
import androidx.compose.runtime.SideEffect
@@ -46,9 +47,12 @@ import androidx.compose.ui.Modifier
import androidx.compose.ui.geometry.Offset
import androidx.compose.ui.layout.approachLayout
import androidx.compose.ui.platform.LocalViewConfiguration
import androidx.compose.ui.platform.testTag
import androidx.compose.ui.test.assertIsDisplayed
import androidx.compose.ui.test.assertIsNotDisplayed
import androidx.compose.ui.test.assertPositionInRootIsEqualTo
import androidx.compose.ui.test.assertTopPositionInRootIsEqualTo
import androidx.compose.ui.test.hasText
import androidx.compose.ui.test.junit4.createComposeRule
import androidx.compose.ui.test.onNodeWithTag
import androidx.compose.ui.test.onRoot
@@ -1529,4 +1533,103 @@ class ElementTest {
        rule.waitForIdle()
        assertThat(fooInB.lastAlpha).isEqualTo(0.3f)
    }

    @Test
    fun sharedElementIsOnlyPlacedInOverscrollingScene() {
        val state =
            rule.runOnUiThread {
                MutableSceneTransitionLayoutStateImpl(
                    SceneA,
                    transitions {
                        overscroll(SceneA, Orientation.Horizontal)
                        overscroll(SceneB, Orientation.Horizontal)
                    }
                )
            }

        @Composable
        fun SceneScope.Foo() {
            Box(Modifier.element(TestElements.Foo).size(10.dp))
        }

        rule.setContent {
            SceneTransitionLayout(state) {
                scene(SceneA) { Foo() }
                scene(SceneB) { Foo() }
            }
        }

        rule.onNode(isElement(TestElements.Foo, SceneA)).assertIsDisplayed()
        rule.onNode(isElement(TestElements.Foo, SceneB)).assertDoesNotExist()

        // A => B while overscrolling at scene B.
        var progress by mutableStateOf(2f)
        rule.runOnUiThread {
            state.startTransition(transition(from = SceneA, to = SceneB, progress = { progress }))
        }
        rule.waitForIdle()

        // Foo should only be placed in scene B.
        rule.onNode(isElement(TestElements.Foo, SceneA)).assertExists().assertIsNotDisplayed()
        rule.onNode(isElement(TestElements.Foo, SceneB)).assertIsDisplayed()

        // Overscroll at scene A.
        progress = -1f
        rule.waitForIdle()

        // Foo should only be placed in scene A.
        rule.onNode(isElement(TestElements.Foo, SceneA)).assertIsDisplayed()
        rule.onNode(isElement(TestElements.Foo, SceneB)).assertExists().assertIsNotDisplayed()
    }

    @Test
    fun sharedMovableElementIsOnlyComposedInOverscrollingScene() {
        val state =
            rule.runOnUiThread {
                MutableSceneTransitionLayoutStateImpl(
                    SceneA,
                    transitions {
                        overscroll(SceneA, Orientation.Horizontal)
                        overscroll(SceneB, Orientation.Horizontal)
                    }
                )
            }

        val fooInA = "fooInA"
        val fooInB = "fooInB"

        @Composable
        fun SceneScope.MovableFoo(text: String, modifier: Modifier = Modifier) {
            MovableElement(TestElements.Foo, modifier) { content { Text(text) } }
        }

        rule.setContent {
            SceneTransitionLayout(state) {
                scene(SceneA) { MovableFoo(text = fooInA) }
                scene(SceneB) { MovableFoo(text = fooInB) }
            }
        }

        rule.onNode(hasText(fooInA)).assertIsDisplayed()
        rule.onNode(hasText(fooInB)).assertDoesNotExist()

        // A => B while overscrolling at scene B.
        var progress by mutableStateOf(2f)
        rule.runOnUiThread {
            state.startTransition(transition(from = SceneA, to = SceneB, progress = { progress }))
        }
        rule.waitForIdle()

        // Foo content should only be composed in scene B.
        rule.onNode(hasText(fooInA)).assertDoesNotExist()
        rule.onNode(hasText(fooInB)).assertIsDisplayed()

        // Overscroll at scene A.
        progress = -1f
        rule.waitForIdle()

        // Foo content should only be composed in scene A.
        rule.onNode(hasText(fooInA)).assertIsDisplayed()
        rule.onNode(hasText(fooInB)).assertDoesNotExist()
    }
}