Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Korean text input #406

Merged
merged 2 commits into from
Feb 23, 2023
Merged

Fix Korean text input #406

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
e9c4aaa
Fix Korean text input
igordmn Feb 14, 2023
bd4fcda
Update WindowTypeTest.kt
igordmn Feb 23, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 3 additions & 6 deletions compose/ui/ui/src/desktopMain/kotlin/androidx/compose/ui/awt/ComposeLayer.desktop.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -331,17 +331,14 @@ internal class ComposeLayer(
_component.addInputMethodListener(object : InputMethodListener {
override fun caretPositionChanged(event: InputMethodEvent?) {
if (isDisposed) return
if (event != null) {
catchExceptions {
platform.textInputService.onInputEvent(event)
}
}
// Which OSes and which input method could produce such events? We need to have some
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should ask someone knowledgeable? I bet it's not here for nothing.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It may be only for Swing components. This method isn't triggered on Linux/Windows/macOS

I created a separate issue that can be related to this: JetBrains/compose-multiplatform#2758

// specific cases in mind before implementing this
}

override fun inputMethodTextChanged(event: InputMethodEvent) {
if (isDisposed) return
catchExceptions {
platform.textInputService.onInputEvent(event)
platform.textInputService.inputMethodTextChanged(event)
}
}
})
Expand Down
34 changes: 6 additions & 28 deletions ...ui/ui/src/desktopMain/kotlin/androidx/compose/ui/platform/DesktopPlatformInput.desktop.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,47 +106,25 @@ internal class PlatformInput(private val component: PlatformComponent) :
}
}

fun onInputEvent(event: InputMethodEvent) {
fun inputMethodTextChanged(event: InputMethodEvent) {
if (!event.isConsumed) {
when (event.id) {
InputMethodEvent.INPUT_METHOD_TEXT_CHANGED -> {
replaceInputMethodText(event)
event.consume()
}
InputMethodEvent.CARET_POSITION_CHANGED -> {
inputMethodCaretPositionChanged(event)
event.consume()
}
}
replaceInputMethodText(event)
event.consume()
}
}

private fun inputMethodCaretPositionChanged(
@Suppress("UNUSED_PARAMETER") event: InputMethodEvent
) {
// Which OSes and which input method could produce such events? We need to have some
// specific cases in mind before implementing this
}

private fun replaceInputMethodText(event: InputMethodEvent) {
currentInput?.let { input ->
if (event.text == null) {
return
}
val committed = event.text.toStringUntil(event.committedCharacterCount)
val composing = event.text.toStringFrom(event.committedCharacterCount)
val committed = event.text?.toStringUntil(event.committedCharacterCount).orEmpty()
val composing = event.text?.toStringFrom(event.committedCharacterCount).orEmpty()
val ops = mutableListOf<EditCommand>()

if (needToDeletePreviousChar && isMac && input.value.selection.min > 0 && composing.isEmpty()) {
needToDeletePreviousChar = false
ops.add(DeleteSurroundingTextInCodePointsCommand(1, 0))
}

// newCursorPosition == 1 leads to effectively ignoring of this parameter in EditCommands
// processing. the cursor will be set after the inserted text.
if (committed.isNotEmpty()) {
ops.add(CommitTextCommand(committed, 1))
}
ops.add(CommitTextCommand(committed, 1))
if (composing.isNotEmpty()) {
ops.add(SetComposingTextCommand(composing, 1))
}
Expand Down
48 changes: 28 additions & 20 deletions compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/TestUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,34 +22,21 @@ import androidx.compose.ui.graphics.ImageBitmap
import androidx.compose.ui.graphics.Paint
import androidx.compose.ui.graphics.painter.BitmapPainter
import androidx.compose.ui.graphics.painter.Painter
import java.awt.AWTEvent
import java.awt.Component
import java.awt.Container
import java.awt.EventQueue
import java.awt.Image
import java.awt.Toolkit
import java.awt.Window
import java.awt.event.InvocationEvent
import java.awt.event.InputMethodEvent
import java.awt.event.KeyEvent
import java.awt.event.MouseEvent
import java.awt.event.MouseWheelEvent
import java.awt.font.TextHitInfo
import java.awt.image.BufferedImage
import java.awt.image.MultiResolutionImage
import java.lang.reflect.InvocationTargetException
import java.lang.reflect.Method
import java.util.Objects
import java.util.concurrent.atomic.AtomicReference
import java.text.AttributedString
import javax.swing.Icon
import javax.swing.ImageIcon
import javax.swing.JFrame
import javax.swing.SwingUtilities
import kotlin.coroutines.resume
import kotlinx.coroutines.runBlocking
import kotlinx.coroutines.suspendCancellableCoroutine
import kotlinx.coroutines.yield
import org.jetbrains.annotations.NonNls
import org.jetbrains.annotations.TestOnly
import org.jetbrains.skiko.MainUIDispatcher

fun testImage(color: Color): Painter = run {
val bitmap = ImageBitmap(100, 100)
Expand All @@ -72,19 +59,40 @@ internal val isMacOs = os.startsWith("mac")

fun Window.sendKeyEvent(
code: Int,
char: Char = code.toChar(),
id: Int = KeyEvent.KEY_PRESSED,
location: Int = KeyEvent.KEY_LOCATION_STANDARD,
modifiers: Int = 0
): Boolean {
val event = KeyEvent(
// if we would just use `focusOwner` then it will be null if the window is minimized
mostRecentFocusOwner,
KeyEvent.KEY_PRESSED,
id,
0,
modifiers,
code,
code.toChar(),
KeyEvent.KEY_LOCATION_STANDARD
char,
location
)
mostRecentFocusOwner!!.dispatchEvent(event)
return event.isConsumed
}

fun Window.sendInputEvent(
text: String?,
committedCharacterCount: Int,
): Boolean {
val event = InputMethodEvent(
// if we would just use `focusOwner` then it will be null if the window is minimized
mostRecentFocusOwner,
InputMethodEvent.INPUT_METHOD_TEXT_CHANGED,
0,
text?.let(::AttributedString)?.iterator,
committedCharacterCount,
TextHitInfo.leading(0),
TextHitInfo.leading(0)
)
dispatchEvent(event)
mostRecentFocusOwner!!.dispatchEvent(event)
return event.isConsumed
}

Expand Down
4 changes: 2 additions & 2 deletions ...se/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/platform/DesktopInputComponentTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class DesktopInputComponentTest {

val familyEmoji = "\uD83D\uDC68\u200D\uD83D\uDC69\u200D\uD83D\uDC66\u200D\uD83D\uDC66"

input.onInputEvent(
input.inputMethodTextChanged(
InputMethodEvent(
DummyComponent,
InputMethodEvent.INPUT_METHOD_TEXT_CHANGED,
Expand Down Expand Up @@ -104,7 +104,7 @@ class DesktopInputComponentTest {
component.enabledInput!!.getSelectedText(null)
input.charKeyPressed = false

input.onInputEvent(
input.inputMethodTextChanged(
InputMethodEvent(
DummyComponent,
InputMethodEvent.INPUT_METHOD_TEXT_CHANGED,
Expand Down
21 changes: 16 additions & 5 deletions compose/ui/ui/src/desktopTest/kotlin/androidx/compose/ui/window/TestUtils.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import kotlinx.coroutines.yield
import org.jetbrains.skiko.MainUIDispatcher
import org.junit.Assume.assumeFalse

@OptIn(ExperimentalCoroutinesApi::class)
internal fun runApplicationTest(
/**
* Use delay(500) additionally to `yield` in `await*` functions
Expand All @@ -45,9 +44,15 @@ internal fun runApplicationTest(
* (non-flaky).
*
* We have to use `useDelay` in some Linux Tests, because Linux can behave in
* non-deterministic way when we change position/size very fast (see the snippet below)
* non-deterministic way when we change position/size very fast (see the snippet below).
*/
useDelay: Boolean = false,
// TODO ui-test solved this issue by passing InfiniteAnimationPolicy to CoroutineContext. Do the same way here
/**
* Hint for `awaitIdle` that the content contains animations (ProgressBar, TextField cursor, etc).
* In this case, we use `delay` instead of waiting for state changes to end.
*/
hasAnimations: Boolean = false,
timeoutMillis: Long = 30000,
body: suspend WindowTestScope.() -> Unit
) {
Expand All @@ -57,7 +62,7 @@ internal fun runApplicationTest(
withTimeout(timeoutMillis) {
val exceptionHandler = TestExceptionHandler()
withExceptionHandler(exceptionHandler) {
val scope = WindowTestScope(this, useDelay, exceptionHandler)
val scope = WindowTestScope(this, useDelay, hasAnimations, exceptionHandler)
scope.body()
scope.exitTestApplication()
}
Expand Down Expand Up @@ -100,6 +105,7 @@ internal class TestExceptionHandler : Thread.UncaughtExceptionHandler {
internal class WindowTestScope(
private val scope: CoroutineScope,
private val useDelay: Boolean,
private val hasAnimations: Boolean,
private val exceptionHandler: TestExceptionHandler
) : CoroutineScope by CoroutineScope(scope.coroutineContext + Job()) {
var isOpen by mutableStateOf(true)
Expand Down Expand Up @@ -132,8 +138,13 @@ internal class WindowTestScope(
awaitEDT()

Snapshot.sendApplyNotifications()
for (recomposerInfo in Recomposer.runningRecomposers.value - initialRecomposers) {
recomposerInfo.state.takeWhile { it > Recomposer.State.Idle }.collect()

if (hasAnimations) {
delay(500)
} else {
for (recomposerInfo in Recomposer.runningRecomposers.value - initialRecomposers) {
recomposerInfo.state.takeWhile { it > Recomposer.State.Idle }.collect()
}
}

exceptionHandler.throwIfCaught()
Expand Down
Loading