Skip to content

Commit debecae

Browse files
m-sasham-sasha
authored
Use TextRange.min/max instead of start/end in TextEditingScope (JetBrains#2982)
1 parent b3ce9c9 commit debecae

4 files changed

Lines changed: 157 additions & 82 deletions

File tree

‎compose/foundation/foundation/src/skikoMain/kotlin/androidx/compose/foundation/text/input/internal/TextInputSession.skiko.kt‎

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -155,33 +155,36 @@ private fun TextEditingScope(buffer: TextFieldBuffer) = object : TextEditingScop
155155
setSelectionCoerced(value, value)
156156
}
157157

158+
// Be careful about using TextRange.start/end, as the selection can be reversed (start > end).
159+
// Prefer to use TextRange.min/max.
160+
158161
override fun deleteSurroundingTextInCodePoints(
159162
lengthBeforeCursor: Int,
160163
lengthAfterCursor: Int
161164
) {
162165
val charSequence = buffer.asCharSequence()
163166
val selection = buffer.selection
164167
buffer.delete(
165-
start = selection.end,
168+
start = selection.max,
166169
end = charSequence.offsetByCodePoints(
167-
index = selection.end,
170+
index = selection.max,
168171
offset = lengthAfterCursor
169172
)
170173
)
171174
buffer.delete(
172175
start = charSequence.offsetByCodePoints(
173-
index = selection.start,
176+
index = selection.min,
174177
offset = -lengthBeforeCursor
175178
),
176-
end = selection.start
179+
end = selection.min
177180
)
178181
}
179182

180183
override fun commitText(text: CharSequence, newCursorPosition: Int) {
181184
// API description says replace ongoing composition text if there. Then, if there is no
182-
// composition text, insert text into cursor position or replace selection.
185+
// composition text, insert text into the cursor position or replace selection.
183186
val replacementRange = buffer.composition ?: buffer.selection
184-
buffer.replace(replacementRange.start, replacementRange.end, text)
187+
buffer.replace(replacementRange.min, replacementRange.max, text)
185188

186189
// After replace function is called, the editing buffer places the cursor at the end of the
187190
// modified range.
@@ -200,11 +203,11 @@ private fun TextEditingScope(buffer: TextFieldBuffer) = object : TextEditingScop
200203
override fun setComposingText(text: CharSequence, newCursorPosition: Int) {
201204
val replacementRange = buffer.composition ?: buffer.selection
202205
// API doc says, if there is ongoing composing text, replace it with new text.
203-
// If there is no composing text, insert composing text into cursor position with
206+
// If there is no composing text, insert composing text into the cursor position with
204207
// removing selected text if any.
205-
buffer.replace(replacementRange.start, replacementRange.end, text)
208+
buffer.replace(replacementRange.min, replacementRange.max, text)
206209
if (text.isNotEmpty()) {
207-
buffer.setComposition(replacementRange.start, replacementRange.start + text.length)
210+
buffer.setComposition(replacementRange.min, replacementRange.max + text.length)
208211
}
209212

210213
// After replace function is called, the editing buffer places the cursor at the end of the

‎compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/TestUtils.kt‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,16 @@ fun Window.sendKeyEvent(
9191
return event.isConsumed
9292
}
9393

94+
fun Window.sendPressAndReleaseKeyEvents(
95+
code: Int,
96+
char: Char = code.toChar(),
97+
location: Int = KeyEvent.KEY_LOCATION_STANDARD,
98+
modifiers: Int = 0
99+
) {
100+
sendKeyEvent(code, char, id = KeyEvent.KEY_PRESSED, location, modifiers)
101+
sendKeyEvent(code, char, id = KeyEvent.KEY_RELEASED, location, modifiers)
102+
}
103+
94104
fun Window.focusedInputMethodRequests(): InputMethodRequests? =
95105
mostRecentFocusOwner!!.inputMethodRequests
96106

‎compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/BaseWindowTextFieldTest.kt‎

Lines changed: 108 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ open class BaseWindowTextFieldTest {
6363
internal fun <S: TextFieldTestScope> runTextFieldTest(
6464
textFieldKind: TextFieldKind<S>,
6565
name: String,
66+
initialText: String = "",
67+
initialSelection: TextRange = TextRange.Zero,
6668
body: suspend S.() -> Unit
6769
) = runApplicationTest {
6870
var scope: S? = null
@@ -75,7 +77,12 @@ open class BaseWindowTextFieldTest {
7577
modifier = Modifier.fillMaxSize()
7678
) {
7779
if (scope == null) {
78-
scope = textFieldKind.createScope(this@runApplicationTest, window)
80+
scope = textFieldKind.createScope(
81+
windowTestScope = this@runApplicationTest,
82+
window = window,
83+
initialText = initialText,
84+
initialSelection = initialSelection
85+
)
7986
}
8087
Column(horizontalAlignment = Alignment.CenterHorizontally) {
8188
Text("$name ($scope)")
@@ -153,10 +160,17 @@ open class BaseWindowTextFieldTest {
153160

154161
internal abstract class TextField1Scope(
155162
windowTestScope: WindowTestScope,
156-
window: ComposeWindow
163+
window: ComposeWindow,
164+
initialText: String,
165+
initialSelection: TextRange,
157166
): TextFieldTestScope(windowTestScope, window) {
158167

159-
protected var textFieldValue by mutableStateOf(TextFieldValue())
168+
protected var textFieldValue by mutableStateOf(
169+
TextFieldValue(
170+
text = initialText,
171+
selection = initialSelection,
172+
)
173+
)
160174

161175
override val text: String
162176
get() = textFieldValue.text
@@ -175,9 +189,14 @@ open class BaseWindowTextFieldTest {
175189

176190
internal abstract class TextField2Scope(
177191
windowTestScope: WindowTestScope,
178-
window: ComposeWindow
192+
window: ComposeWindow,
193+
initialText: String,
194+
initialSelection: TextRange,
179195
): TextFieldTestScope(windowTestScope, window) {
180-
protected val textFieldState = TextFieldState()
196+
protected val textFieldState = TextFieldState(
197+
initialText = initialText,
198+
initialSelection = initialSelection,
199+
)
181200
var inputTransformation: InputTransformation? by mutableStateOf(null)
182201

183202
override val text: String
@@ -198,99 +217,115 @@ open class BaseWindowTextFieldTest {
198217
windowTestScope: WindowTestScope,
199218
window: ComposeWindow,
200219
textObfuscationMode: TextObfuscationMode,
201-
): TextField2Scope(windowTestScope, window) {
220+
initialText: String = "",
221+
initialSelection: TextRange = TextRange.Zero,
222+
): TextField2Scope(windowTestScope, window, initialText, initialSelection) {
202223

203224
var textObfuscationMode by mutableStateOf(textObfuscationMode)
204225

205226
}
206227

207228
internal fun interface TextFieldKind<S: TextFieldTestScope> {
208-
fun createScope(windowTestScope: WindowTestScope, window: ComposeWindow): S
229+
fun createScope(
230+
windowTestScope: WindowTestScope,
231+
window: ComposeWindow,
232+
initialText: String,
233+
initialSelection: TextRange,
234+
): S
209235
}
210236

211237
companion object {
212238
@JvmField
213239
@DataPoint
214-
internal val TextField1 = TextFieldKind<TextField1Scope> { windowTestScope, window ->
215-
object : TextField1Scope(windowTestScope, window) {
216-
@Composable
217-
override fun TextField() {
218-
val focusRequester = remember { FocusRequester() }
219-
BasicTextField(
220-
value = textFieldValue,
221-
onValueChange = {
222-
textFieldValue = it
223-
},
224-
onTextLayout = { textLayoutResult = it },
225-
modifier = Modifier
226-
.focusRequester(focusRequester)
227-
.onPlaced {
228-
textBoundingBox = it.boundsInWindow()
229-
}
230-
)
231-
232-
LaunchedEffect(focusRequester) {
233-
focusRequester.requestFocus()
240+
internal val TextField1 = TextFieldKind<TextField1Scope> {
241+
windowTestScope, window, initialText, initialSelection ->
242+
object : TextField1Scope(windowTestScope, window, initialText, initialSelection) {
243+
@Composable
244+
override fun TextField() {
245+
val focusRequester = remember { FocusRequester() }
246+
BasicTextField(
247+
value = textFieldValue,
248+
onValueChange = {
249+
textFieldValue = it
250+
},
251+
onTextLayout = { textLayoutResult = it },
252+
modifier = Modifier
253+
.focusRequester(focusRequester)
254+
.onPlaced {
255+
textBoundingBox = it.boundsInWindow()
256+
}
257+
)
258+
259+
LaunchedEffect(focusRequester) {
260+
focusRequester.requestFocus()
261+
}
234262
}
235-
}
236263

237-
override fun toString() = "TextField1"
238-
}
264+
override fun toString() = "TextField1"
265+
}
239266
}
240267

241268
@JvmField
242269
@DataPoint
243-
internal val TextField2 = TextFieldKind<TextField2Scope> { windowTestScope, window ->
244-
object : TextField2Scope(windowTestScope, window) {
245-
@Composable
246-
override fun TextField() {
247-
val focusRequester = remember { FocusRequester() }
248-
BasicTextField(
249-
state = textFieldState,
250-
inputTransformation = inputTransformation,
251-
onTextLayout = { textLayoutResultGetter = it },
252-
modifier = Modifier
253-
.focusRequester(focusRequester)
254-
.onPlaced {
255-
textBoundingBox = it.boundsInWindow()
256-
}
257-
)
258-
259-
LaunchedEffect(focusRequester) {
260-
focusRequester.requestFocus()
270+
internal val TextField2 = TextFieldKind<TextField2Scope> {
271+
windowTestScope, window, initialText, initialSelection ->
272+
object : TextField2Scope(windowTestScope, window, initialText, initialSelection) {
273+
@Composable
274+
override fun TextField() {
275+
val focusRequester = remember { FocusRequester() }
276+
BasicTextField(
277+
state = textFieldState,
278+
inputTransformation = inputTransformation,
279+
onTextLayout = { textLayoutResultGetter = it },
280+
modifier = Modifier
281+
.focusRequester(focusRequester)
282+
.onPlaced {
283+
textBoundingBox = it.boundsInWindow()
284+
}
285+
)
286+
287+
LaunchedEffect(focusRequester) {
288+
focusRequester.requestFocus()
289+
}
261290
}
262-
}
263291

264-
override fun toString() = "TextField2"
265-
}
292+
override fun toString() = "TextField2"
293+
}
266294
}
267295

268296
@JvmField
269297
@DataPoint
270-
internal val SecureTextField = TextFieldKind<SecureTextFieldScope> { windowTestScope, window ->
271-
object : SecureTextFieldScope(windowTestScope, window, TextObfuscationMode.Hidden) {
272-
@Composable
273-
override fun TextField() {
274-
val focusRequester = remember { FocusRequester() }
275-
276-
BasicSecureTextField(
277-
state = textFieldState,
278-
textObfuscationMode = textObfuscationMode,
279-
onTextLayout = { textLayoutResultGetter = it },
280-
modifier = Modifier
281-
.focusRequester(focusRequester)
282-
.onPlaced {
283-
textBoundingBox = it.boundsInWindow()
284-
}
285-
)
286-
287-
LaunchedEffect(focusRequester) {
288-
focusRequester.requestFocus()
298+
internal val SecureTextField = TextFieldKind<SecureTextFieldScope> {
299+
windowTestScope, window, initialText, initialSelection ->
300+
object : SecureTextFieldScope(
301+
windowTestScope,
302+
window,
303+
TextObfuscationMode.Hidden,
304+
initialText,
305+
initialSelection
306+
) {
307+
@Composable
308+
override fun TextField() {
309+
val focusRequester = remember { FocusRequester() }
310+
311+
BasicSecureTextField(
312+
state = textFieldState,
313+
textObfuscationMode = textObfuscationMode,
314+
onTextLayout = { textLayoutResultGetter = it },
315+
modifier = Modifier
316+
.focusRequester(focusRequester)
317+
.onPlaced {
318+
textBoundingBox = it.boundsInWindow()
319+
}
320+
)
321+
322+
LaunchedEffect(focusRequester) {
323+
focusRequester.requestFocus()
324+
}
289325
}
290-
}
291326

292-
override fun toString() = "SecureTextField"
293-
}
327+
override fun toString() = "SecureTextField"
328+
}
294329
}
295330
}
296331
}

‎compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/window/WindowTypeTest.kt‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,9 @@ import androidx.compose.ui.sendCharTypedEvents
2424
import androidx.compose.ui.sendInputMethodEvent
2525
import androidx.compose.ui.sendKeyEvent
2626
import androidx.compose.ui.sendKeyTypedEvent
27+
import androidx.compose.ui.sendPressAndReleaseKeyEvents
2728
import androidx.compose.ui.text.TextRange
29+
import java.awt.event.KeyEvent
2830
import java.awt.event.KeyEvent.KEY_PRESSED
2931
import java.awt.event.KeyEvent.KEY_RELEASED
3032
import kotlin.test.Test
@@ -903,4 +905,29 @@ class WindowTypeTest : BaseWindowTextFieldTest() {
903905
window.sendInputMethodEvent("·", committedCharacterCount = 1)
904906
assertStateEquals("·", selection = TextRange(1), composition = null)
905907
}
908+
909+
@Theory
910+
internal fun `select text backwards, then input via IME`(
911+
textFieldKind: TextFieldKind<*>
912+
) = runTextFieldTest(
913+
textFieldKind = textFieldKind,
914+
name = "Select text backwards, then input via IME",
915+
initialText = "abcdef",
916+
initialSelection = TextRange(6)
917+
) {
918+
// Select "def"
919+
window.sendKeyEvent(KeyEvent.VK_SHIFT, id = KEY_PRESSED)
920+
repeat(3) {
921+
window.sendPressAndReleaseKeyEvents(KeyEvent.VK_LEFT, modifiers = KeyEvent.SHIFT_DOWN_MASK)
922+
awaitIdle()
923+
}
924+
925+
assertStateEquals("abcdef", selection = TextRange(6, 3), composition = null)
926+
927+
// Insert character via IME
928+
window.sendInputMethodEvent("ㅂ", 0)
929+
window.sendKeyEvent(81, 'q', KEY_RELEASED)
930+
931+
assertStateEquals("abcㅂ", selection = TextRange(4, 4), composition = TextRange(3, 4))
932+
}
906933
}

0 commit comments

Comments
 (0)