When the button is deactivated, a touch event is delivered to the lower level's Composable function. [183908811]

Fixed

Bug

Status Update

No update yet.

Description

pl...@gmail.com

created issue #1

Mar 29, 2021 05:08PM

Jetpack Compose release version: 1.4.30 Android Studio Build: Android Studio Arctic Fox | 2020.3.1 Canary 12

Steps to Reproduce:

Define Click on Box and inner button
Apply Disabled to button
(Bug?!?! )When clicking the button, the box click operates

The button click does not work, but I don't know if it is correct to propagate to the lower box. If it is an existing ViewGroup, it will not be propagated to the lower part.

@Composable
fun SampleBox() {
    var text by remember { mutableStateOf("Test") }
    Box(
        modifier = Modifier
            .aspectRatio(1f)
            .background(Color.Blue)
            .clickable { text = "Box" }
            .padding(10.dp)
    ) {
        Button(
            modifier = Modifier
                .size(100.dp)
                .align(Alignment.TopCenter),
            colors = ButtonDefaults.buttonColors(Color.Red),
            onClick = { text = "Red" }
        ) { }

        Text(
            text = text,
            fontSize = 40.sp,
            color = Color.White,
            modifier = Modifier.align(Alignment.Center)
        )

        Button(
            modifier = Modifier
                .size(100.dp)
                .align(Alignment.BottomCenter),
            colors = ButtonDefaults.buttonColors(Color.Green),
            enabled = false,
            onClick = { text = "Green" }
        ) { }
    }
}

sample.mov

474 KB

Download

Comments

po...@google.com <po...@google.com> Jun 11, 2021 02:34PM

Assigned to ma...@google.com.

ap...@google.com <ap...@google.com> #2Jun 16, 2021 09:23AM

Marked as fixed.

Project: platform/frameworks/support
Branch: androidx-master-dev

commit c60f33e229e31ab328ef6b59dab63b264954831c
Author: Alexandre Elias <aelias@google.com>
Date: Fri Jul 10 16:23:09 2020

Semantics no-op cleanups

Partly in response to lmr's broad code review, I did a pass of
superficial API/implementation cleanup. The main changes are:

- I changed each Boolean SemanticsProperty where false is equivalent to
not being present to take "Unit" instead. This is conceptually
clearer: it avoids questions like "can I cancel out a semantics from a
merged child by setting it to false?" Because "property = Unit" looks
weird, I also changed the style of these to "property()".

- I moved the Semantics id generator closer to where it's used, in
SemanticsModifierCore. I made it internal and an AtomicInt.
(Note that integer ids are heavily used in the Android
AccessibilityNodeInfo APIs so I can't simply remove them entirely.)

- I deleted dead code. Some examples include SemanticsHintOverrides,
a public API not connected to anything, and SemanticsPropertyKey
merge() open method which is never called. (In both cases I have
a different plan in mind for accessibility.)

Fixes: 145951226
Fixes: 145955412
Test: existing tests
Relnote: "Single-value semantics properties now use a calling style.
For example, 'semantics { hidden = true }' is now written as:
'semantics { hidden() }'."

Change-Id: Ic1afd12ea22c926babc9662f1804d80b33aa0cfc

M ui/integration-tests/benchmark/src/androidTest/java/androidx/ui/benchmark/test/LayoutNodeModifierBenchmark.kt
M ui/ui-core/api/0.1.0-dev15.txt
M ui/ui-core/api/current.txt
M ui/ui-core/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-core/api/public_plus_experimental_current.txt
M ui/ui-core/api/restricted_0.1.0-dev15.txt
M ui/ui-core/api/restricted_current.txt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/graphics/vector/VectorTest.kt
M ui/ui-core/src/androidAndroidTest/kotlin/androidx/ui/semantics/SemanticsTests.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidActuals.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeView.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidComposeViewAccessibilityDelegateCompat.kt
M ui/ui-core/src/androidMain/kotlin/androidx/ui/core/AndroidPopup.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/Expect.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsConfiguration.kt
D ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsHintOverrides.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsModifier.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsNode.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsOwner.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/core/semantics/SemanticsWrapper.kt
M ui/ui-core/src/commonMain/kotlin/androidx/ui/semantics/SemanticsProperties.kt
M ui/ui-foundation/api/0.1.0-dev15.txt
M ui/ui-foundation/api/current.txt
M ui/ui-foundation/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-foundation/api/public_plus_experimental_current.txt
M ui/ui-foundation/api/restricted_0.1.0-dev15.txt
M ui/ui-foundation/api/restricted_current.txt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Clickable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Dialog.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/Scroller.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Selectable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/selection/Toggleable.kt
M ui/ui-foundation/src/main/java/androidx/ui/foundation/semantics/FoundationSemanticsProperties.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ButtonTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CardTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/CheckboxScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/RadioButtonScreenshotTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ScaffoldTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SnackbarTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/SurfaceTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/ripple/RippleIndicationTest.kt
M ui/ui-material/src/androidTest/java/androidx/ui/material/textfield/TextFieldScreenshotTest.kt
M ui/ui-material/src/main/java/androidx/ui/material/AppBar.kt
M ui/ui-material/src/main/java/androidx/ui/material/TextFieldImpl.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/AssertsTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/CallSemanticsActionTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ErrorMessagesTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/FindersTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/PrintToStringTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/ScrollToTest.kt
M ui/ui-test/src/androidTest/java/androidx/ui/test/TextActionsTest.kt
M ui/ui-test/src/main/java/androidx/ui/test/Actions.kt
M ui/ui-test/src/main/java/androidx/ui/test/Filters.kt
M ui/ui-text/api/0.1.0-dev15.txt
M ui/ui-text/api/current.txt
M ui/ui-text/api/public_plus_experimental_0.1.0-dev15.txt
M ui/ui-text/api/public_plus_experimental_current.txt
M ui/ui-text/api/restricted_0.1.0-dev15.txt
M ui/ui-text/api/restricted_current.txt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/CoreTextField.kt
M ui/ui-text/src/commonMain/kotlin/androidx/ui/text/TextSemanticsProperties.kt

https://android-review.googlesource.com/1360099

ma...@google.com <ma...@google.com> #3Jun 16, 2021 10:12AM

Thanks a lot for filing this issue. It was a serious misconception in Modifier.clickable and its users that we were able to fix before hitting stable thanks to you!

[Deleted User] <[Deleted User]> #4Jul 13, 2021 05:02PM

Ok. But if i need to disable buttons click, and still handle box clicks? Now this fix comletely destroys this ability

ma...@google.com <ma...@google.com> #5Jul 13, 2021 05:32PM

Could you share a usecase when it is desirable to proxy click action to the parent card/box?

If the button is disabled - it's unavailable and nothing should happen when the user presses it.

[Deleted User] <[Deleted User]> #6Jul 16, 2021 07:23AM

I have a custom layout. When it in the collapsed state - the parent should be clickable. Click on parrent should expand children. In this state children are clickable. Click on child item opens new screen, click on knob collapses the layout.

In previeous realization i had for children:

MoodCardStack(
                    date = date,
                    state = state,
                    itemPadding = Dimens.standard,
                    modifier = Modifier.padding(vertical = Dimens.small)
                ) {
                    if (moods[date]?.size == 1 && state.expanded) {
                        state.collapse()
                    }
                    moods[date]?.forEach { mood ->
                        key(mood) {
                            MoodCardItem(
                                userMood = mood,
                                modifier = Modifier
                                    .fillMaxWidth()
                                    .height(90.dp)
                                    .clickable(
                                        enabled = state.expanded || moods[date]?.size == 1,
                                        indication = null,
                                        interactionSource = remember { MutableInteractionSource() }
                                    ) {
                                        onMoodClick(mood.id)
                                    }
                            )
                        }
                    }
                }

and for parent

 Layout(
            content = content,
            modifier = Modifier
                .clickable(
                    enabled = state.expanded.not() && collapseProgress.value == 0F,
                    indication = null,
                    interactionSource = remember { MutableInteractionSource() }
                ) { state.expand() }
                .constrainAs(container) {
                    top.linkTo(parent.top)
                    start.linkTo(dateColumn.end, margin = Dimens.small)
                    end.linkTo(parent.end, margin = Dimens.standard)
                    width = Dimension.fillToConstraints
                }
        ) { measurables, constraints ->
```

Now if MoodCardItem .clickable(false), the whole layout is not clickable. I had to dynamically create different modifiers with and without clickables. And this realization looks weird when the previous was more obvious.

layout.mp4

2.6 MB

Download

ma...@google.com <ma...@google.com> #7Jul 19, 2021 09:50AM

It seems like the card shouldn't be clickable at all when the layout is collapsed.

disabled clickable shouldn't allow other click gestures to go through, as it would be unpredictable for users and developers. Basically, the list goes:

Modifier.clickable(enabled = true) == I can react to the click gestures and the action is available now
Modifier.clickable(enabled = false) == I can react to the click gestures, and the action is currently unavailable
no clickable modifier == I cannot react to the click gestures, so anyone else up the hierarchy can handle it for me.

I hope that helps.