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

Commit 956bc2a0 authored by Neha Jain's avatar Neha Jain Committed by Android (Google) Code Review
Browse files

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

This reverts commit 15336522.

Reason for revert: b/435361598

Bug: 433909610
Change-Id: Ibe02a73e0029a8f8a01ff0ce4e1fb7f52632f57d
parent 15336522

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

+1 −29
Original line number Diff line number Diff line
@@ -41,9 +41,7 @@ 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.unit.Constraints

import androidx.compose.ui.unit.Density
@@ -565,7 +563,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.then(ContentElement(overlay.zIndex, isInvisible)).matchParentSize()) {

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

                    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.
@@ -710,29 +708,3 @@ private class LayoutNode( 
        return layout(width, height) { placeable.place(0, 0) }

    }

}
 


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)

                }

            }

        }

    }

}

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

+24 −8
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)).thenIf(

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

                        layoutImpl.implicitTestTags

                    ) {

                        Modifier.testTag(key.testTag)
@@ -226,16 +226,20 @@ 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)

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



    override fun update(node: ContentNode) {

        node.update(content, isElevationPossible)

        node.update(content, isElevationPossible, isInvisible)

    }

}



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

    DelegatingNode(), ApproachLayoutModifierNode {

private class ContentNode(

    private var content: Content,

    private var isElevationPossible: Boolean,

    private var isInvisible: Boolean,

) : DelegatingNode(), ApproachLayoutModifierNode {

    private var containerDelegate = containerDelegate(isElevationPossible)



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

    }



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

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

        if (content != this.content) {

            this.content.targetSize = Element.SizeUnspecified

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

            containerDelegate = containerDelegate(isElevationPossible)

        }



        this.isInvisible = isInvisible

    }



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

        return measurable.measure(constraints).run {

            content.targetSize = IntSize(width, height)

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

            layout(width, height) {

                if (!isInvisible) {

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

                }

            }

        }

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

        constraints: Constraints,

    ): MeasureResult {

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

        return measurable.measure(constraints).run {

            layout(width, height) {

                if (!isInvisible) {

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

                }

            }

        }

    }

}

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

+0 −31
Original line number Diff line number Diff line
@@ -22,7 +22,6 @@ 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
@@ -54,7 +53,6 @@ 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
@@ -654,33 +652,4 @@ class SceneTransitionLayoutTest { 
        rule.onNode(isElement(TestElements.Foo)).assertIsDisplayed().assertSizeIsEqualTo(40.dp)

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

    }



    @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()

    }

}