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

Commit 2589889b authored by Fabian Kozynski's avatar Fabian Kozynski Committed by Android (Google) Code Review
Browse files

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

parents d8c41f00 956bc2a0
Loading
Loading
Loading
Loading
+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)
                }
            }
        }
    }
}
+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)
                }
            }
        }
    }
}

+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()
    }
}