TextField with singleLine=true in dialog does not consume Enter event [305518328]

Bug

Status Update

No update yet.

Description

da...@mullvad.net

created issue #1

Oct 16, 2023 08:46AM

TextField with singleLine = true does not consume the enter key. It passed imeOptions(singleLine = true) but IME never consumes the click. This becomes a problem when using a TextField in a dialog since focus is still on a list item in the back. Pressing enter while focusing the TextField will then cause the button in the list view in the background to be pressed again.

Here is a snippet with the issue at hand:

            MyApplicationTheme {
                val selectedRow = remember { mutableStateOf<Int?>(null) }
                val textFieldContent = remember { mutableStateOf("") }

                if (selectedRow.value != null) {
                    AlertDialog(onDismissRequest = { selectedRow.value = null }) {
                        Surface(
                            Modifier
                                .fillMaxWidth(0.8f)
                                .clip(MaterialTheme.shapes.medium)
                                .background(MaterialTheme.colorScheme.background)
                                .padding(24.dp)
                                .wrapContentHeight()
                        ) {
                            Column {
                                TextField(
                                    value = textFieldContent.value,
                                    onValueChange = { textFieldContent.value = it },
                                    keyboardActions = KeyboardActions(onDone = {
                                        selectedRow.value = null
                                    }),
                                    keyboardOptions = KeyboardOptions(
                                        imeAction = ImeAction.Done
                                    ),
                                    singleLine = true
                                )

                                Button(onClick = { selectedRow.value = null }) {
                                    Text(text = "Submit")
                                }
                            }
                        }
                    }
                }

                Surface(
                    modifier = Modifier.fillMaxSize(),
                    color = MaterialTheme.colorScheme.background
                ) {
                    LazyColumn {
                        for (i in 0..100) {
                            item {
                                Button(onClick = {
                                    // I get called when user pressed enter in the text field in the dialog :(
                                    Log.d("MainActivity", "Clicked row $i")
                                    selectedRow.value = i
                                }) {
                                    Text(text = "Row $i")
                                }
                            }
                        }
                    }
                }
            }

I've also attached a recording of the issue, I first navigate with keyboard enter the dialog and focus the TextField, enter some text and then press enter to finish my modification. This causes the dialog to close but also trigger a onClick on the button (named "Row 2") in the background thus causing the dialog to open again. On the second dialog I go to the "Confirm" button and press enter instead, this correctly consumes the press and simply closes the dialog.

singleline_enter_event.webm

8 MB

Download

Comments

da...@mullvad.net <da...@mullvad.net> #2Oct 16, 2023 08:56AM

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

da...@mullvad.net <da...@mullvad.net> #3Oct 16, 2023 09:22AM

So further clarification on this. I tried several ways of handling this without `singleLine=true` e.g consuming the event in `onPreviewKeyEvent`, `onKeyEvent`, `onPreInterceptKeyBeforeSoftKeyboard` even though I consume the `enter event` in these functions it will still also trigger the click event outside the Dialog and open it again.

da...@mullvad.net <da...@mullvad.net> #4Oct 16, 2023 12:11PM

Only way to avoid the view in the back to get an onclick seems to simulate and interaction with a button or delay the removal of the dialog:

```
// Submit button's interactionSource
val interactionSource: MutableInteractionSource = remember { MutableInteractionSource() }

// TextField
...
onDone = {
val press = PressInteraction.Press(Offset.Zero)
interactionSource.tryEmit(press) interactionSource.tryEmit(PressInteraction.Release(press))
},
...
```
or
```
val scope = rememberCoroutineScope()

// TextField
...
onDone = {
scope.launch {
delay(100)
selectedRow.value = null
}
},
...
```

Haven't found any nice solution to this issue so far.

pj...@gmail.com <pj...@gmail.com> #5Oct 17, 2023 03:55AM

Para fijar un clip, manténlo presionado. Después de una hora, se borrarán todos los clips que no estén fijados.

kl...@google.com <kl...@google.com> #6Oct 17, 2023 10:38PM

Added some logging, it looks like the dialog gets the ACTION_DOWN event, but the main window gets the ACTION_UP event. If I replace the list with Android views, this does not happen.

I think probably Compose's Material Button should not treat an UP without a DOWN as a click.

Or better yet, according to this thread, Compose's hardware key system should probably filter out orphan up events like this.

Message last modified on Oct 17, 2023 11:11PM

da...@mullvad.net <da...@mullvad.net> #7Oct 3, 2024 01:20PM

I tested this on the latest version of compose, seems to work correctly now.

Issue 305518328