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

Commit ff9f9b23 authored by Jordan Demeulenaere's avatar Jordan Demeulenaere
Browse files

Reapply "Don't place the modal clickable for invisible overlays" 

This reverts commit 956bc2a0.

This CL is a reland of ag/34828325, which was broken given that it was
not applying the `ContentElement` modifier on the scenes and only on the
overlays, and caused b/435361598. This regression is now covered by the
tests added/improved in ag/34917382 and ag/34918781, which pass with
this CL.

I also double checked that b/435361598 was not happening with this
change.

Bug: 433909610
Bug: 435361598
Test: atest SceneTransitionLayoutTest
Flag: com.android.systemui.scene_container
Change-Id: Ic0860d269a77e86c4638890fd48f388f0ce23a6c
parent 2b85e2ef

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/SceneTransitionLayoutImpl.kt

+35 −2
Original line number Diff line number Diff line
@@ -41,7 +41,9 @@ import androidx.compose.ui.layout.ApproachMeasureScope 
import androidx.compose.ui.layout.LookaheadScope

import androidx.compose.ui.layout.Measurable

import androidx.compose.ui.layout.MeasureResult

import androidx.compose.ui.layout.MeasureScope

import androidx.compose.ui.node.LayoutAwareModifierNode

import androidx.compose.ui.node.LayoutModifierNode

import androidx.compose.ui.node.ModifierNodeElement

import androidx.compose.ui.platform.testTag

import androidx.compose.ui.unit.Constraints
@@ -509,7 +511,12 @@ internal class SceneTransitionLayoutImpl( 
    @Composable

    private fun Scenes() {

        scenesToCompose().fastForEach { (scene, isInvisible) ->

            key(scene.key) { scene.Content(isInvisible = isInvisible) }

            key(scene.key) {

                scene.Content(

                    isInvisible = isInvisible,

                    modifier = Modifier.then(ContentElement(scene.zIndex, isInvisible)),

                )

            }

        }

    }
@@ -566,7 +573,7 @@ internal class SceneTransitionLayoutImpl( 
                // We put the overlays inside a Box that is matching the layout size so that they

                // are measured after all scenes and that their max size is the size of the layout

                // without the overlays.

                Box(Modifier.matchParentSize().zIndex(overlay.zIndex)) {

                Box(Modifier.then(ContentElement(overlay.zIndex, isInvisible)).matchParentSize()) {

                    if (overlay.isModal) {

                        // Add a fullscreen clickable to prevent swipes from reaching the scenes and

                        // other overlays behind this overlay. Clicking will close the overlay.
@@ -712,4 +719,30 @@ private class LayoutNode( 
    }

}



private data class ContentElement(val zIndex: Float, val isInvisible: Boolean) :

    ModifierNodeElement<ContentNode>() {

    override fun create(): ContentNode = ContentNode(zIndex, isInvisible)



    override fun update(node: ContentNode) {

        node.zIndex = zIndex

        node.isInvisible = isInvisible

    }

}



private class ContentNode(var zIndex: Float, var isInvisible: Boolean) :

    Modifier.Node(), LayoutModifierNode {

    override fun MeasureScope.measure(

        measurable: Measurable,

        constraints: Constraints,

    ): MeasureResult {

        return measurable.measure(constraints).run {

            layout(width, height) {

                if (!isInvisible) {

                    place(0, 0, zIndex = zIndex)

                }

            }

        }

    }

}



internal const val SceneTransitionLayoutRootContentTag = "SceneTransitionLayoutRootContent"

packages/SystemUI/compose/scene/src/com/android/compose/animation/scene/content/Content.kt

+8 −24
Original line number Diff line number Diff line
@@ -178,7 +178,7 @@ internal sealed class Content( 
        val content =

            @Composable {

                Box(

                    modifier.then(ContentElement(this, isElevationPossible, isInvisible)).thenIf(

                    modifier.then(ContentElement(this, isElevationPossible)).thenIf(

                        layoutImpl.implicitTestTags

                    ) {

                        Modifier.testTag(key.testTag)
@@ -226,20 +226,16 @@ internal sealed class Content( 
private data class ContentElement(

    private val content: Content,

    private val isElevationPossible: Boolean,

    private val isInvisible: Boolean,

) : ModifierNodeElement<ContentNode>() {

    override fun create(): ContentNode = ContentNode(content, isElevationPossible, isInvisible)

    override fun create(): ContentNode = ContentNode(content, isElevationPossible)



    override fun update(node: ContentNode) {

        node.update(content, isElevationPossible, isInvisible)

        node.update(content, isElevationPossible)

    }

}



private class ContentNode(

    private var content: Content,

    private var isElevationPossible: Boolean,

    private var isInvisible: Boolean,

) : DelegatingNode(), ApproachLayoutModifierNode {

private class ContentNode(private var content: Content, private var isElevationPossible: Boolean) :

    DelegatingNode(), ApproachLayoutModifierNode {

    private var containerDelegate = containerDelegate(isElevationPossible)



    private fun containerDelegate(isElevationPossible: Boolean): ContainerNode? {
@@ -250,7 +246,7 @@ private class ContentNode( 
        this.content.targetSize = Element.SizeUnspecified

    }



    fun update(content: Content, isElevationPossible: Boolean, isInvisible: Boolean) {

    fun update(content: Content, isElevationPossible: Boolean) {

        if (content != this.content) {

            this.content.targetSize = Element.SizeUnspecified

            this.content = content
@@ -262,8 +258,6 @@ private class ContentNode( 
            containerDelegate?.let { undelegate(it) }

            containerDelegate = containerDelegate(isElevationPossible)

        }



        this.isInvisible = isInvisible

    }



    override fun isMeasurementApproachInProgress(lookaheadSize: IntSize): Boolean = false
@@ -275,11 +269,7 @@ private class ContentNode( 
        check(isLookingAhead)

        return measurable.measure(constraints).run {

            content.targetSize = IntSize(width, height)

            layout(width, height) {

                if (!isInvisible) {

                    place(0, 0, zIndex = content.zIndex)

                }

            }

            layout(width, height) { place(0, 0) }

        }

    }
@@ -287,13 +277,7 @@ private class ContentNode( 
        measurable: Measurable,

        constraints: Constraints,

    ): MeasureResult {

        return measurable.measure(constraints).run {

            layout(width, height) {

                if (!isInvisible) {

                    place(0, 0, zIndex = content.zIndex)

                }

            }

        }

        return measurable.measure(constraints).run { layout(width, height) { place(0, 0) } }

    }

}

packages/SystemUI/compose/scene/tests/src/com/android/compose/animation/scene/SceneTransitionLayoutTest.kt

+31 −0
Original line number Diff line number Diff line
@@ -22,6 +22,7 @@ import androidx.compose.animation.core.LinearEasing 
import androidx.compose.animation.core.spring

import androidx.compose.animation.core.tween

import androidx.compose.foundation.background

import androidx.compose.foundation.clickable

import androidx.compose.foundation.layout.Box

import androidx.compose.foundation.layout.Spacer

import androidx.compose.foundation.layout.fillMaxSize
@@ -56,6 +57,7 @@ import androidx.compose.ui.test.onChild 
import androidx.compose.ui.test.onNodeWithTag

import androidx.compose.ui.test.onNodeWithText

import androidx.compose.ui.test.onRoot

import androidx.compose.ui.test.performClick

import androidx.compose.ui.test.performTouchInput

import androidx.compose.ui.test.swipeDown

import androidx.compose.ui.unit.Dp
@@ -698,4 +700,33 @@ class SceneTransitionLayoutTest { 
            .containsExactly("scene:SceneA", "scene:SceneB", "overlay:OverlayA", "overlay:OverlayB")

            .inOrder()

    }



    @Test

    fun alwaysComposeModalOverlay_notInterceptingTouchesWhenNotVisible() {

        val state = rule.runOnUiThread { MutableSceneTransitionLayoutStateForTests(SceneA) }

        var fooClicked = false

        val scope =

            rule.setContentAndCreateMainScope {

                SceneTransitionLayoutForTesting(state) {

                    scene(SceneA) {

                        Box(

                            Modifier.element(TestElements.Foo).size(40.dp).clickable {

                                fooClicked = true

                            }

                        )

                    }

                    overlay(OverlayA, isModal = true, alwaysCompose = true) {

                        Box(Modifier.element(TestElements.Bar).size(20.dp))

                    }

                }

            }



        // Overlay hidden: Foo is displayed and Bar exists.

        scope.launch { state.snapTo(state.currentScene, overlays = emptySet()) }

        rule.onNode(isElement(TestElements.Foo)).assertIsDisplayed().assertSizeIsEqualTo(40.dp)

        rule.onNode(isElement(TestElements.Bar)).assertExists().assertIsNotDisplayed()



        rule.onNode(isElement(TestElements.Foo)).performClick()

        assertThat(fooClicked).isTrue()

    }

}