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

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

Don't place the modal clickable for invisible overlays

This CL moves the isInvisible behavior for Content in
SceneTransitionLayoutImpl so that the modal clickable is also not placed
when the associated overlay is invisible.

Bug: 433909610
Test: atest SceneTransitionLayoutTest
Flag: com.android.systemui.scene_container
Change-Id: If04811deb09ba5c8d13740586e49beec7195789a
parent 602ecafb
Loading
Loading
Loading
Loading
+29 −1
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.unit.Constraints
import androidx.compose.ui.unit.Density
@@ -563,7 +565,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.
@@ -708,3 +710,29 @@ 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)
                }
            }
        }
    }
}
+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) } }
    }
}

+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
@@ -53,6 +54,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
@@ -652,4 +654,33 @@ 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()
    }
}