From 354e48a5421222b938272961c21a7fd562a5ffa8 Mon Sep 17 00:00:00 2001 From: Oleksandr Karpovich Date: Thu, 21 Oct 2021 19:02:11 +0200 Subject: [PATCH] Web: fix jumping cursor of controlled inputs (#1287) * web: fix cursor position for controlled inputs (wip) * web: fix cursor position for controlled inputs (wip) Co-authored-by: Oleksandr Karpovich --- .../jetbrains/compose/web/attributes/Attrs.kt | 4 +- .../web/attributes/PredefinedAttrValues.kt | 28 +++++++ .../builders/InternalControlledInputUtils.kt | 4 +- .../compose/web/elements/Elements.kt | 47 ++++++++---- .../compose/web/sample/tests/Common.kt | 7 +- .../ControlledInputsCursorsPositionTests.kt | 51 +++++++++++++ .../ControlledInputsCursorsPositionTests.kt | 75 +++++++++++++++++++ .../integration/ControlledInputsTests.kt | 21 ------ .../common/BaseIntegrationTests.kt | 9 +++ 9 files changed, 203 insertions(+), 43 deletions(-) create mode 100644 web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/ControlledInputsCursorsPositionTests.kt create mode 100644 web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsCursorsPositionTests.kt diff --git a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/Attrs.kt b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/Attrs.kt index c41ed3a1ff..138efdad44 100644 --- a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/Attrs.kt +++ b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/Attrs.kt @@ -299,7 +299,9 @@ fun AttrsBuilder.alt(value: String): AttrsBuilder Unit = { e, v -> - e.value = v + if (v != e.value) { + e.value = v + } saveControlledInputState(e, v) } diff --git a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/PredefinedAttrValues.kt b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/PredefinedAttrValues.kt index f8baddf11a..52fcd3b0e7 100644 --- a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/PredefinedAttrValues.kt +++ b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/PredefinedAttrValues.kt @@ -50,6 +50,34 @@ sealed class InputType(val typeStr: String) { protected fun valueAsString(event: Event): String { return event.target?.asDynamic()?.value?.unsafeCast() ?: "" } + + companion object { + internal fun fromString(type: String): InputType<*> { + return when (type) { + "button" -> Button + "checkbox" -> Checkbox + "color" -> Color + "date" -> Date + "datetime-local" -> DateTimeLocal + "email" -> Email + "file" -> File + "hidden" -> Hidden + "month" -> Month + "number" -> Number + "password" -> Password + "radio" -> Radio + "range" -> Range + "search" -> Search + "submit" -> Submit + "tel" -> Tel + "text" -> Text + "time" -> Time + "url" -> Url + "week" -> Week + else -> error("fromString got unknown type - $type") + } + } + } } sealed class DirType(val dirStr: String) { diff --git a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/builders/InternalControlledInputUtils.kt b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/builders/InternalControlledInputUtils.kt index 88bbc4af5e..5b84c18f05 100644 --- a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/builders/InternalControlledInputUtils.kt +++ b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/attributes/builders/InternalControlledInputUtils.kt @@ -16,7 +16,9 @@ import org.w3c.dom.HTMLTextAreaElement private val controlledInputsValuesWeakMap: JsWeakMap = js("new WeakMap();").unsafeCast() -internal fun restoreControlledInputState(type: InputType<*>, inputElement: HTMLInputElement) { +internal fun restoreControlledInputState(inputElement: HTMLInputElement) { + val type = InputType.fromString(inputElement.type) + if (controlledInputsValuesWeakMap.has(inputElement)) { if (type == InputType.Radio) { controlledRadioGroups[inputElement.name]?.forEach { radio -> diff --git a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/elements/Elements.kt b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/elements/Elements.kt index 66c3049e08..6ebec334b4 100644 --- a/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/elements/Elements.kt +++ b/web/core/src/jsMain/kotlin/org/jetbrains/compose/web/elements/Elements.kt @@ -1,8 +1,6 @@ package org.jetbrains.compose.web.dom -import androidx.compose.runtime.Composable -import androidx.compose.runtime.ComposeNode -import androidx.compose.runtime.remember +import androidx.compose.runtime.* import androidx.compose.web.attributes.SelectAttrsBuilder import kotlinx.browser.document import org.jetbrains.compose.web.attributes.* @@ -678,27 +676,36 @@ fun TextArea( // if firstProvidedValueWasNotNull then TextArea behaves as controlled input val firstProvidedValueWasNotNull = remember { value != null } + // changes to this key trigger [textAreaRestoreControlledStateEffect] + val keyForRestoringControlledState: MutableState = remember { mutableStateOf(0) } + TagElement( elementBuilder = TextArea, applyAttrs = { - val taab = TextAreaAttrsBuilder() + val textAreaAttrsBuilder = TextAreaAttrsBuilder() + textAreaAttrsBuilder.onInput { + // controlled state needs to be restored after every input + keyForRestoringControlledState.value = keyForRestoringControlledState.value + 1 + } if (attrs != null) { - taab.attrs() + textAreaAttrsBuilder.attrs() } if (firstProvidedValueWasNotNull) { - taab.value(value ?: "") + textAreaAttrsBuilder.value(value ?: "") } - taab.onInput { - restoreControlledTextAreaState(it.target) - } - - this.copyFrom(taab) + this.copyFrom(textAreaAttrsBuilder) }, - content = null + content = { + DomSideEffect(keyForRestoringControlledState.value, textAreaRestoreControlledStateEffect) + } ) } +private val textAreaRestoreControlledStateEffect: DomEffectScope.(HTMLTextAreaElement) -> Unit = { + restoreControlledTextAreaState(element = it) +} + @Composable fun Nav( attrs: AttrBuilderContext? = null, @@ -983,32 +990,42 @@ inline fun Style( * } * ``` */ +@OptIn(ComposeWebInternalApi::class) @Composable fun Input( type: InputType, attrs: InputAttrsBuilder.() -> Unit ) { + // changes to this key trigger [inputRestoreControlledStateEffect] + val keyForRestoringControlledState: MutableState = remember { mutableStateOf(0) } + TagElement( elementBuilder = Input, applyAttrs = { val inputAttrsBuilder = InputAttrsBuilder(type) inputAttrsBuilder.type(type) - inputAttrsBuilder.attrs() - inputAttrsBuilder.onInput { - restoreControlledInputState(type = type, inputElement = it.target) + // controlled state needs to be restored after every input + keyForRestoringControlledState.value = keyForRestoringControlledState.value + 1 } + inputAttrsBuilder.attrs() + this.copyFrom(inputAttrsBuilder) }, content = { if (type == InputType.Radio) { DisposeRadioGroupEffect() } + DomSideEffect(keyForRestoringControlledState.value, inputRestoreControlledStateEffect) } ) } +private val inputRestoreControlledStateEffect: DomEffectScope.(HTMLInputElement) -> Unit = { + restoreControlledInputState(inputElement = it) +} + @Composable fun Input(type: InputType) { Input(type) {} diff --git a/web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/Common.kt b/web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/Common.kt index 2e2b7dd80f..474b53df5b 100644 --- a/web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/Common.kt +++ b/web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/Common.kt @@ -1,10 +1,7 @@ package org.jetbrains.compose.web.sample.tests import androidx.compose.runtime.Composable -import androidx.compose.web.sample.tests.ControlledInputsTests -import androidx.compose.web.sample.tests.RadioGroupTestCases -import androidx.compose.web.sample.tests.SelectElementTests -import androidx.compose.web.sample.tests.UncontrolledInputsTests +import androidx.compose.web.sample.tests.* import org.jetbrains.compose.web.dom.Span import org.jetbrains.compose.web.dom.Text import org.jetbrains.compose.web.renderComposableInBody @@ -39,7 +36,7 @@ fun launchTestCase(testCaseId: String) { listOf( TestCases1(), InputsTests(), EventsTests(), SelectElementTests(), ControlledInputsTests(), UncontrolledInputsTests(), - RadioGroupTestCases() + RadioGroupTestCases(), ControlledInputsCursorsPositionTests() ) if (testCaseId !in testCases) error("Test Case '$testCaseId' not found") diff --git a/web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/ControlledInputsCursorsPositionTests.kt b/web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/ControlledInputsCursorsPositionTests.kt new file mode 100644 index 0000000000..847bd74900 --- /dev/null +++ b/web/integration-core/src/jsMain/kotlin/androidx/compose/web/sample/tests/ControlledInputsCursorsPositionTests.kt @@ -0,0 +1,51 @@ +/* + * Copyright 2020-2021 JetBrains s.r.o. and respective authors and developers. + * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file. + */ + +package androidx.compose.web.sample.tests + +import androidx.compose.runtime.getValue +import androidx.compose.runtime.mutableStateOf +import androidx.compose.runtime.remember +import androidx.compose.runtime.setValue +import org.jetbrains.compose.web.dom.* +import org.jetbrains.compose.web.sample.tests.TestText +import org.jetbrains.compose.web.sample.tests.testCase + +class ControlledInputsCursorsPositionTests { + + val textInputTypingIntoMiddle by testCase { + var onInputText by remember { mutableStateOf("None") } + var textValue by remember { mutableStateOf("") } + + P { TestText(onInputText) } + + Div { + TextInput(value = textValue, attrs = { + id("textInput") + onInput { + onInputText = it.value + textValue = it.value + } + }) + } + } + + val textAreaTypingIntoMiddle by testCase { + var onInputText by remember { mutableStateOf("None") } + var textValue by remember { mutableStateOf("") } + + P { TestText(onInputText) } + + Div { + TextArea(value = textValue, attrs = { + id("textArea") + onInput { + onInputText = it.value + textValue = it.value + } + }) + } + } +} diff --git a/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsCursorsPositionTests.kt b/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsCursorsPositionTests.kt new file mode 100644 index 0000000000..2c8a54f505 --- /dev/null +++ b/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsCursorsPositionTests.kt @@ -0,0 +1,75 @@ +/* + * Copyright 2020-2021 JetBrains s.r.o. and respective authors and developers. + * Use of this source code is governed by the Apache 2.0 license that can be found in the LICENSE.txt file. + */ + +package org.jetbrains.compose.web.tests.integration + +import org.jetbrains.compose.web.tests.integration.common.BaseIntegrationTests +import org.jetbrains.compose.web.tests.integration.common.ResolveDrivers +import org.jetbrains.compose.web.tests.integration.common.openTestPage +import org.jetbrains.compose.web.tests.integration.common.waitTextToBe +import org.openqa.selenium.By +import org.openqa.selenium.Keys +import org.openqa.selenium.WebDriver + +class ControlledInputsCursorsPositionTests : BaseIntegrationTests() { + + @ResolveDrivers + fun textInputTypingIntoMiddle(driver: WebDriver) { + driver.openTestPage("textInputTypingIntoMiddle") + driver.waitTextToBe(value = "None") + + val controlledTextInput = driver.findElement(By.id("textInput")) + + controlledTextInput.sendKeys("A") + driver.waitTextToBe(value = "A") + controlledTextInput.sendKeys("B") + driver.waitTextToBe(value = "AB") + controlledTextInput.sendKeys("C") + driver.waitTextToBe(value = "ABC") + + controlledTextInput.sendKeys(Keys.ARROW_LEFT) + controlledTextInput.sendKeys(Keys.ARROW_LEFT) + assert(driver.cursorPosition("textInput") == 1) + + controlledTextInput.sendKeys("1") + driver.waitTextToBe(value = "A1BC") + + assert(driver.cursorPosition("textInput") == 2) + + controlledTextInput.sendKeys("2") + driver.waitTextToBe(value = "A12BC") + + assert(driver.cursorPosition("textInput") == 3) + } + + @ResolveDrivers + fun textAreaTypingIntoMiddle(driver: WebDriver) { + driver.openTestPage("textAreaTypingIntoMiddle") + driver.waitTextToBe(value = "None") + + val controlledTextInput = driver.findElement(By.id("textArea")) + + controlledTextInput.sendKeys("A") + driver.waitTextToBe(value = "A") + controlledTextInput.sendKeys("B") + driver.waitTextToBe(value = "AB") + controlledTextInput.sendKeys("C") + driver.waitTextToBe(value = "ABC") + + controlledTextInput.sendKeys(Keys.ARROW_LEFT) + controlledTextInput.sendKeys(Keys.ARROW_LEFT) + assert(driver.cursorPosition("textArea") == 1) + + controlledTextInput.sendKeys("1") + driver.waitTextToBe(value = "A1BC") + + assert(driver.cursorPosition("textArea") == 2) + + controlledTextInput.sendKeys("2") + driver.waitTextToBe(value = "A12BC") + + assert(driver.cursorPosition("textArea") == 3) + } +} diff --git a/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsTests.kt b/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsTests.kt index ce185b8d31..071a4ec3c6 100644 --- a/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsTests.kt +++ b/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/ControlledInputsTests.kt @@ -472,12 +472,6 @@ class ControlledInputsTests : BaseIntegrationTests() { @ResolveDrivers fun mutableTimeChanges(driver: WebDriver) { - // We skip chrome, since for some reason `sendKeys` doesn't work as expected when used for Controlled Input in Chrome - Assumptions.assumeTrue( - driver !is ChromeDriver, - "chrome driver doesn't work properly when using sendKeys on Controlled Input" - ) - driver.openTestPage("mutableTimeChanges") driver.waitTextToBe(value = "") @@ -489,19 +483,4 @@ class ControlledInputsTests : BaseIntegrationTests() { driver.waitTextToBe(value = "18:31") check(timeInput.getAttribute("value") == "18:31") } - - @ResolveDrivers - fun timeInputSendKeysOnChromeFailingTest(driver: WebDriver) { - Assumptions.assumeTrue( - driver is ChromeDriver, - "this a `failing test for Chrome only` to catch when issue with sendKeys is resolved" - ) - driver.openTestPage("mutableTimeChanges") - driver.waitTextToBe(value = "") - - val timeInput = driver.findElement(By.id("time")) - - timeInput.sendKeys("18:31") - driver.waitTextToBe(value = "18:03") // it should be 18:31, but this is a failing test, so wrong value is expected - } } diff --git a/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/common/BaseIntegrationTests.kt b/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/common/BaseIntegrationTests.kt index 4ab2df9775..107113c956 100644 --- a/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/common/BaseIntegrationTests.kt +++ b/web/integration-core/src/jvmTest/kotlin/org/jetbrains/compose/web/tests/integration/common/BaseIntegrationTests.kt @@ -95,6 +95,15 @@ abstract class BaseIntegrationTests() { return (this as JavascriptExecutor).executeAsyncScript(script).toString() } + fun WebDriver.cursorPosition(id: String): Int { + val script = """ + var callback = arguments[arguments.length - 1]; + callback(document.getElementById("$id").selectionStart); + """.trimIndent() + + return (this as JavascriptExecutor).executeAsyncScript(script).toString().toInt() + } + fun WebDriver.sendKeysForDateInput(input: WebElement, year: Int, month: Int, day: Int) { val keys = when (this) { is ChromeDriver -> "${day}${month}${year}"