TextField doesn't allow backspace removal of last character [165956313]

Fixed

Bug

Status Update

No update yet.

Description

se...@google.com

created issue #1

Aug 22, 2020 04:45AM

dev 17
kotlin 1.4

1-function repro :)

@Preview
@Composable
fun PreviewTextField() {
    val (text, setText) = remember { mutableStateOf("") }
    TextField(value = text, onValueChange = setText, label = { })
}

TextField works as expected until user presses backspace to remove last character. setText is not called, but the cursor position is modified.

Selecting the text and using cut does cause setText to be cleared.

Thanks!

Comments

ma...@google.com <ma...@google.com> #2Aug 24, 2020 10:43AM

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

so...@google.com <so...@google.com> #3Aug 24, 2020 10:48AM

Seems to be a duplicate of b/163834907

ma...@google.com <ma...@google.com> #4Aug 24, 2020 10:51AM

It works fine with Material TextFields (both filled and outline) if you use TextFieldValue based overloads.

It doesn't work when you use String based overloads however (both Outline and Filled). I believe this is related to the one-frame lag as we host TextFieldValue internally and proxy only string.

ma...@google.com <ma...@google.com> #5Aug 24, 2020 12:08PM

Chatted with Anastasia.

We have this code

// "value: String" comes as a param in TextField
TextFieldImpl(
        type = TextFieldType.Outlined,
        value = textFieldValue,
        onValueChange = {
            selection = it.selection
            composition = it.composition
            if (value != it.text) {
                onValueChange(it.text)
            }
        },

and this value is always "" for me, as it's captured my lambda when lambda was created and this lambda is saved and reused by CoreTextField.

Anyway, two things we can do:

Remove this if and invoke onValueChange everytime, even on the selection change (which we don't care about). This will allow us to workaround quickly and not to let it slip through.
Find and implement the proper fix in CoreTextField

Let me know what you think

so...@google.com <so...@google.com> #6Aug 24, 2020 01:25PM

Updated the details here b/163834907.

so...@google.com <so...@google.com> #7Aug 24, 2020 01:31PM

Just a note that this issue affects String overrides of both material text fields as well as any custom String override text fields that developer might create on top of CoreTextField/BaseTextField.

Message last modified on Aug 24, 2020 01:31PM

si...@google.com <si...@google.com> Aug 24, 2020 05:43PM

Reassigned to so...@google.com.

ap...@google.com <ap...@google.com> #8Aug 26, 2020 08:58AM

Marked as fixed.

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

commit 1717097cfa889c564450626af6d5dcc405581d79
Author: Anastasia Soboleva <soboleva@google.com>
Date: Mon Aug 24 16:38:59 2020

Fix lambda value being captured for string override text fields

Fixes: 163834907
Fixes: 163808219
Fixes: 165956313
Test: new test in TextFieldTest#stringOverrideTextField_canDeleteLastSymbol
Test: foundation and material tests passed
Change-Id: I9932a2bddc3859a2a35a54e0b6d7cb525853d3ac

M compose/foundation/foundation-text/src/commonMain/kotlin/androidx/compose/foundation/text/CoreTextField.kt
M compose/foundation/foundation/src/androidAndroidTest/kotlin/androidx/compose/foundation/TextFieldTest.kt

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

vi...@airbnb.com <vi...@airbnb.com> #9Aug 27, 2020 05:45AM

Which release is this fix going to be a part of? Also, what's the best way to know which version a change will be a part of in general?

so...@google.com <so...@google.com> #10Aug 27, 2020 09:48AM

This fix will be available in the upcoming release.

As for the second question, I'm not sure I've got a good answer for that one. I'd say normally if it's marked as fixed, it'll be available within two next releases.